diff options
| -rw-r--r-- | Bugzilla/Attachment.pm | 155 | ||||
| -rw-r--r-- | Bugzilla/Attachment/Database.pm | 59 | ||||
| -rw-r--r-- | Bugzilla/Attachment/FileSystem.pm | 61 | ||||
| -rw-r--r-- | Bugzilla/Config/Attachment.pm | 17 | ||||
| -rwxr-xr-x | scripts/migrate-attachments.pl | 111 | ||||
| -rw-r--r-- | template/en/default/admin/params/attachment.html.tmpl | 24 | ||||
| -rw-r--r-- | template/en/default/attachment/list.html.tmpl | 8 | ||||
| -rw-r--r-- | template/en/default/bug/create/create.html.tmpl | 2 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 6 |
9 files changed, 289 insertions, 154 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index 95e6b2977..07fafb11e 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -333,27 +333,7 @@ the content of the attachment sub data { my $self = shift; - return $self->{data} if exists $self->{data}; - - # First try to get the attachment data from the database. - ($self->{data}) = Bugzilla->dbh->selectrow_array("SELECT thedata - FROM attach_data - WHERE id = ?", - undef, - $self->id); - - # If there's no attachment data in the database, the attachment is stored - # in a local file, so retrieve it from there. - if (length($self->{data}) == 0) { - if (open(AH, '<', $self->_get_local_filename())) { - local $/; - binmode AH; - $self->{data} = <AH>; - close(AH); - } - } - - return $self->{data}; + return $self->{data} //= current_storage()->retrieve($self->id); } =over @@ -372,60 +352,6 @@ sub datasize { =over -=item C<linecount> - -the number of lines of the attachment content - -=back - -=cut - -# linecount allows for getting the number of lines of an attachment -# from the database directly if the data has not yet been loaded for -# performance reasons. - -sub linecount { - my ($self) = @_; - - return $self->{linecount} if exists $self->{linecount}; - - # Limit this to just text/* attachments as this could - # cause strange results for binary attachments. - return if $self->contenttype !~ /^text\//; - - # If the data has already been loaded, we can just determine - # line count from the data directly. - if ($self->{data}) { - $self->{linecount} = $self->{data} =~ tr/\n/\n/; - } - else { - $self->{linecount} = - int(Bugzilla->dbh->selectrow_array(" - SELECT LENGTH(attach_data.thedata)-LENGTH(REPLACE(attach_data.thedata,'\n',''))/LENGTH('\n') - FROM attach_data WHERE id = ?", undef, $self->id)); - - } - - # If we still do not have a linecount either the attachment - # is stored in a local file or has been deleted. If the former, - # we call self->data to force a load from the filesystem and - # then do a split on newlines and count again. - unless ($self->{linecount}) { - $self->{linecount} = $self->data =~ tr/\n/\n/; - } - - return $self->{linecount}; -} - -sub _get_local_filename { - my $self = shift; - my $hash = ($self->id % 100) + 100; - $hash =~ s/.*(\d\d)$/group.$1/; - return bz_locations()->{'attachdir'} . "/$hash/attachment." . $self->id; -} - -=over - =item C<flags> flags that have been set on the attachment @@ -543,13 +469,10 @@ sub _check_data { $params->{attach_size} || ThrowUserError('zero_length_file'); # Make sure the attachment does not exceed the maximum permitted size. - my $max_size = max(Bugzilla->params->{'maxlocalattachment'} * 1048576, - Bugzilla->params->{'maxattachmentsize'} * 1024); - - if ($params->{attach_size} > $max_size) { - my $vars = { filesize => sprintf("%.0f", $params->{attach_size}/1024) }; - ThrowUserError('file_too_large', $vars); + if ($params->{attach_size} > Bugzilla->params->{'maxattachmentsize'} * 1024) { + ThrowUserError('file_too_large', { filesize => sprintf("%.0f", $params->{attach_size}/1024) }); } + return $data; } @@ -804,46 +727,18 @@ sub create { my $data = delete $params->{data}; my $attachment = $class->insert_create_data($params); - my $attachid = $attachment->id; + $attachment->{bug} = $bug; - # The file is too large to be stored in the DB, so we store it locally. - if ($params->{attach_size} > Bugzilla->params->{'maxattachmentsize'} * 1024) { - my $attachdir = bz_locations()->{'attachdir'}; - my $hash = ($attachid % 100) + 100; - $hash =~ s/.*(\d\d)$/group.$1/; - mkdir "$attachdir/$hash", 0770; - chmod 0770, "$attachdir/$hash"; - if (ref $data) { - copy($data, "$attachdir/$hash/attachment.$attachid"); - close $data; - } - else { - open(AH, '>', "$attachdir/$hash/attachment.$attachid"); - binmode AH; - print AH $data; - close AH; - } - $data = ''; # Will be stored in the DB. - } - # If we have a filehandle, we need its content to store it in the DB. - elsif (ref $data) { + # store attachment data + if (ref($data)) { local $/; - # Store the content in a temp variable while we close the FH. my $tmp = <$data>; - close $data; + close($data); $data = $tmp; } + current_storage()->store($attachment->id, $data); - my $sth = $dbh->prepare("INSERT INTO attach_data - (id, thedata) VALUES ($attachid, ?)"); - - trick_taint($data); - $sth->bind_param(1, $data, $dbh->BLOB_TYPE); - $sth->execute(); - - $attachment->{bug} = $bug; - - # Return the new attachment object. + # Return the new attachment object return $attachment; } @@ -924,10 +819,10 @@ sub remove_from_db { 'SELECT id FROM flags WHERE attach_id = ?', undef, $self->id); $dbh->do('DELETE FROM flags WHERE ' . $dbh->sql_in('id', $flag_ids)) if @$flag_ids; - $dbh->do('DELETE FROM attach_data WHERE id = ?', undef, $self->id); $dbh->do('UPDATE attachments SET mimetype = ?, ispatch = ?, isobsolete = ?, attach_size = ? WHERE attach_id = ?', undef, ('text/plain', 0, 1, 0, $self->id)); $dbh->bz_commit_transaction(); + current_storage->remove($self->id); # As we don't call SUPER->remove_from_db we need to manually clear # memcached here. @@ -989,5 +884,33 @@ sub get_content_type { return $content_type; } +sub current_storage { + return state $storage //= get_storage_by_name(Bugzilla->params->{attachment_storage}); +} + +sub get_storage_names { + require Bugzilla::Config::Attachment; + foreach my $param (Bugzilla::Config::Attachment->get_param_list) { + next unless $param->{name} eq 'attachment_storage'; + return @{ $param->{choices} }; + } + return []; +} + +sub get_storage_by_name { + my ($name) = @_; + # all options for attachment_storage need to be handled here + if ($name eq 'database') { + require Bugzilla::Attachment::Database; + return Bugzilla::Attachment::Database->new(); + } + elsif ($name eq 'filesystem') { + require Bugzilla::Attachment::FileSystem; + return Bugzilla::Attachment::FileSystem->new(); + } + else { + return undef; + } +} 1; diff --git a/Bugzilla/Attachment/Database.pm b/Bugzilla/Attachment/Database.pm new file mode 100644 index 000000000..24617cacc --- /dev/null +++ b/Bugzilla/Attachment/Database.pm @@ -0,0 +1,59 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Attachment::Database; +use strict; +use warnings; + +use Bugzilla::Util qw(trick_taint); + +sub new { + return bless({}, shift); +} + +sub store { + my ($self, $attach_id, $data) = @_; + my $dbh = Bugzilla->dbh; + my $sth = $dbh->prepare("INSERT INTO attach_data (id, thedata) VALUES ($attach_id, ?)"); + trick_taint($data); + $sth->bind_param(1, $data, $dbh->BLOB_TYPE); + $sth->execute(); +} + +sub retrieve { + my ($self, $attach_id) = @_; + my $dbh = Bugzilla->dbh; + my ($data) = $dbh->selectrow_array( + "SELECT thedata FROM attach_data WHERE id = ?", + undef, + $attach_id + ); + return $data; +} + +sub remove { + my ($self, $attach_id) = @_; + my $dbh = Bugzilla->dbh; + $dbh->do( + "DELETE FROM attach_data WHERE id = ?", + undef, + $attach_id + ); +} + +sub exists { + my ($self, $attach_id) = @_; + my $dbh = Bugzilla->dbh; + my ($exists) = $dbh->selectrow_array( + "SELECT 1 FROM attach_data WHERE id = ?", + undef, + $attach_id + ); + return !!$exists; +} + +1; diff --git a/Bugzilla/Attachment/FileSystem.pm b/Bugzilla/Attachment/FileSystem.pm new file mode 100644 index 000000000..675184c49 --- /dev/null +++ b/Bugzilla/Attachment/FileSystem.pm @@ -0,0 +1,61 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Attachment::FileSystem; +use strict; +use warnings; + +use Bugzilla::Constants qw(bz_locations); + +sub new { + return bless({}, shift); +} + +sub store { + my ($self, $attach_id, $data) = @_; + my $path = _local_path($attach_id); + mkdir($path, 0770) unless -d $path; + open(my $fh, '>', _local_file($attach_id)); + binmode($fh); + print $fh $data; + close($fh); +} + +sub retrieve { + my ($self, $attach_id) = @_; + if (open(my $fh, '<', _local_file($attach_id))) { + local $/; + binmode($fh); + my $data = <$fh>; + close($fh); + return $data; + } + return undef; +} + +sub remove { + my ($self, $attach_id) = @_; + unlink(_local_file($attach_id)); +} + +sub exists { + my ($self, $attach_id) = @_; + return -e _local_file($attach_id); +} + +sub _local_path { + my ($attach_id) = @_; + my $hash = sprintf('group.%03d', $attach_id % 1000); + return bz_locations()->{attachdir} . '/' . $hash; +} + +sub _local_file { + my ($attach_id) = @_; + return _local_path($attach_id) . '/attachment.' . $attach_id; +} + +1; diff --git a/Bugzilla/Config/Attachment.pm b/Bugzilla/Config/Attachment.pm index e6e3b7f3d..cf87f4c89 100644 --- a/Bugzilla/Config/Attachment.pm +++ b/Bugzilla/Config/Attachment.pm @@ -66,19 +66,12 @@ sub get_param_list { checker => \&check_maxattachmentsize }, - # The maximum size (in bytes) for patches and non-patch attachments. - # The default limit is 1000KB, which is 24KB less than mysql's default - # maximum packet size (which determines how much data can be sent in a - # single mysql packet and thus how much data can be inserted into the - # database) to provide breathing space for the data in other fields of - # the attachment record as well as any mysql packet overhead (I don't - # know of any, but I suspect there may be some.) - { - name => 'maxlocalattachment', - type => 't', - default => '0', - checker => \&check_numeric + name => 'attachment_storage', + type => 's', + choices => ['database', 'filesystem'], + default => 'database', + checker => \&check_multi } ); return @param_list; } diff --git a/scripts/migrate-attachments.pl b/scripts/migrate-attachments.pl new file mode 100755 index 000000000..40342c179 --- /dev/null +++ b/scripts/migrate-attachments.pl @@ -0,0 +1,111 @@ +#!/usr/bin/perl + +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +use strict; +use warnings; + +use FindBin qw($RealBin); +use lib "$RealBin/..", "$RealBin/../lib"; + +use Bugzilla; +use Bugzilla::Attachment; +use Bugzilla::Install::Util qw(indicate_progress); +use Getopt::Long qw(GetOptions); + +my @storage_names = Bugzilla::Attachment->get_storage_names(); + +my %options; +GetOptions(\%options, 'mirror=s@{2}', 'delete=s') or exit(1); +unless ($options{mirror} || $options{delete}) { + die <<EOF; +Syntax: + migrate-attachments.pl --mirror source destination + migrate-attachments.pl --delete source + +'mirror' + Copies all attachments from the specified source to the destination. + Attachments which already exist at the destination will not be copied + again. Attachments deleted on the source will be deleted from the + destination. + +'delete' + Deletes all attachments in the specified location. This operation cannot + be undone. + +Valid locations: + @storage_names + +EOF +} + +my $dbh = Bugzilla->dbh; +my ($total) = $dbh->selectrow_array("SELECT COUNT(*) FROM attachments"); + +if ($options{mirror}) { + if ($options{mirror}->[0] eq $options{mirror}->[1]) { + die "Source and destination must be different\n"; + } + my ($source, $dest) = map { storage($_) } @{ $options{mirror} }; + confirm(sprintf('Mirror %s attachments from %s to %s?', $total, @{ $options{mirror} })); + + my $sth = $dbh->prepare("SELECT attach_id, attach_size FROM attachments ORDER BY attach_id"); + $sth->execute(); + my ($count, $deleted, $stored) = (0, 0, 0); + while (my ($attach_id, $attach_size) = $sth->fetchrow_array()) { + indicate_progress({ total => $total, current => ++$count }); + + # remove deleted attachments + if ($attach_size == 0 && $dest->exists($attach_id)) { + $dest->remove($attach_id); + $deleted++; + } + + # store attachments that don't already exist + elsif ($attach_size != 0 && !$dest->exists($attach_id)) { + if (my $data = $source->retrieve($attach_id)) { + $dest->store($attach_id, $data); + $stored++; + } + } + } + print "\n"; + print "Attachments stored: $stored\n"; + print "Attachments deleted: $deleted\n" if $deleted; +} + +elsif ($options{delete}) { + my $storage = storage($options{delete}); + confirm(sprintf('DELETE %s attachments from %s?', $total, $options{delete})); + + my $sth = $dbh->prepare("SELECT attach_id FROM attachments ORDER BY attach_id"); + $sth->execute(); + my ($count, $deleted) = (0, 0); + while (my ($attach_id) = $sth->fetchrow_array()) { + indicate_progress({ total => $total, current => ++$count }); + if ($storage->exists($attach_id)) { + $storage->remove($attach_id); + $deleted++; + } + } + print "\n"; + print "Attachments deleted: $deleted\n"; +} + +sub storage { + my ($name) = @_; + my $storage = Bugzilla::Attachment::get_storage_by_name($name) + or die "Invalid attachment location: $name\n"; + return $storage; +} + +sub confirm { + my ($prompt) = @_; + print $prompt, "\n\nPress <Ctrl-C> to stop or <Enter> to continue..\n"; + getc(); +} diff --git a/template/en/default/admin/params/attachment.html.tmpl b/template/en/default/admin/params/attachment.html.tmpl index 4075374bc..bdd20c676 100644 --- a/template/en/default/admin/params/attachment.html.tmpl +++ b/template/en/default/admin/params/attachment.html.tmpl @@ -57,19 +57,17 @@ _ " (such as <tt>1234.your.domain.com</tt>) must point to this same" _ " $terms.Bugzilla instance.", - allow_attachment_deletion => "If this option is on, administrators will be able to delete " _ - "the content of attachments.", + allow_attachment_deletion => + "If this option is on, administrators will be able to delete " _ + "the content of attachments.", - maxattachmentsize => "The maximum size (in kilobytes) of attachments to be stored " _ - "in the database. If a file larger than this size is attached " _ - "to ${terms.abug}, $terms.Bugzilla will look at the " _ - "<a href=\"#maxlocalattachment\"><tt>maxlocalattachment</tt> parameter</a> " _ - "to determine if the file can be stored locally on the web server. " _ - "If the file size exceeds both limits, then the attachment is rejected. " _ - "Settings both parameters to 0 will prevent attaching files to ${terms.bugs}.", + maxattachmentsize => + "The maximum size (in kilobytes) of attachments.", - maxlocalattachment => "The maximum size (in megabytes) of attachments to be stored " _ - "locally on the web server. If set to a value lower than the " _ - "<a href=\"#maxattachmentsize\"><tt>maxattachmentsize</tt> parameter</a>, " _ - "attachments will never be kept on the local filesystem." } + attachment_storage => + "Where attachment data should be stored. If this value is changed you " _ + "must use <tt>scripts/mmigrate-attachments</tt> to migrate existing " _ + "attachments.", + + } %] diff --git a/template/en/default/attachment/list.html.tmpl b/template/en/default/attachment/list.html.tmpl index 2b0b31584..4aae3abef 100644 --- a/template/en/default/attachment/list.html.tmpl +++ b/template/en/default/attachment/list.html.tmpl @@ -19,8 +19,6 @@ # Frédéric Buclin <LpSolit@gmail.com> #%] -[% RETURN UNLESS attachments.size || Param("maxattachmentsize") || Param("maxlocalattachment") %] - <script type="text/javascript"> <!-- function toggle_display(link) { @@ -168,10 +166,8 @@ function toggle_display(link) { [% END %] </span> [% END %] - [% IF Param("maxattachmentsize") || Param("maxlocalattachment") %] - <a href="attachment.cgi?bugid=[% bugid %]&action=enter">Add an attachment</a> - (proposed patch, testcase, etc.) - [% END %] + <a href="attachment.cgi?bugid=[% bugid %]&action=enter">Add an attachment</a> + (proposed patch, testcase, etc.) </td> </tr> diff --git a/template/en/default/bug/create/create.html.tmpl b/template/en/default/bug/create/create.html.tmpl index 98290f1c2..ba5d08273 100644 --- a/template/en/default/bug/create/create.html.tmpl +++ b/template/en/default/bug/create/create.html.tmpl @@ -559,7 +559,6 @@ TUI_hide_default('attachment_text_field'); </tr> </tbody> - [% IF Param("maxattachmentsize") || Param("maxlocalattachment") %] <tr> <th>Attachment:</th> <td colspan="3"> @@ -592,7 +591,6 @@ TUI_hide_default('attachment_text_field'); </div> </td> </tr> - [% END %] </tbody> <tbody class="expert_fields"> diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index f8d8985f9..e8510b134 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -688,13 +688,9 @@ [% ELSIF error == "file_too_large" %] [% title = "File Too Large" %] - [%# Convert maxlocalattachment from Mb to Kb %] - [% max_local = Param('maxlocalattachment') * 1024 %] - [% max_limit = [Param('maxattachmentsize'), max_local] %] The file you are trying to attach is [% filesize FILTER html %] kilobytes (KB) in size. Attachments cannot be more than - [%# Hack to get the max value between both limits %] - [%+ max_limit.nsort.last FILTER html %] KB. <br> + [%+ Param('maxattachmentsize') FILTER html %] KB. <br> We recommend that you store your attachment elsewhere and then paste the URL to this file on the attachment creation page in the appropriate text field. |