diff options
| -rw-r--r-- | Bugzilla/Attachment.pm | 116 | ||||
| -rwxr-xr-x | attachment.cgi | 1 | ||||
| -rw-r--r-- | docs/en/xml/using.xml | 18 | ||||
| -rw-r--r-- | js/attachment.js | 5 | ||||
| -rwxr-xr-x | post_bug.cgi | 1 | ||||
| -rw-r--r-- | template/en/default/admin/params/attachment.html.tmpl | 19 | ||||
| -rw-r--r-- | template/en/default/attachment/createformcontents.html.tmpl | 12 | ||||
| -rw-r--r-- | template/en/default/global/code-error.html.tmpl | 6 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 20 |
9 files changed, 57 insertions, 141 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index ddce1f593..39afe5df1 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -59,6 +59,9 @@ use Bugzilla::Util; use Bugzilla::Field; use Bugzilla::Hook; +use File::Copy; +use List::Util qw(max); + use base qw(Bugzilla::Object); ############################### @@ -108,7 +111,6 @@ use constant VALIDATORS => { isprivate => \&_check_is_private, isurl => \&_check_is_url, mimetype => \&_check_content_type, - store_in_file => \&_check_store_in_file, }; use constant UPDATE_VALIDATORS => { @@ -538,52 +540,29 @@ sub _check_content_type { sub _check_data { my ($invocant, $params) = @_; - my $data; + my $data = $params->{data}; if ($params->{isurl}) { - $data = $params->{data}; ($data && $data =~ m#^(http|https|ftp)://\S+#) || ThrowUserError('attachment_illegal_url', { url => $data }); $params->{mimetype} = 'text/plain'; $params->{ispatch} = 0; - $params->{store_in_file} = 0; + $params->{filesize} = length($data); } else { - if ($params->{store_in_file} || !ref $params->{data}) { - # If it's a filehandle, just store it, not the content of the file - # itself as the file may be quite large. If it's not a filehandle, - # it already contains the content of the file. - $data = $params->{data}; - } - else { - # The file will be stored in the DB. We need the content of the file. - local $/; - my $fh = $params->{data}; - $data = <$fh>; - } + $params->{filesize} = ref $data ? -s $data : length($data); } Bugzilla::Hook::process('attachment_process_data', { data => \$data, attributes => $params }); - # Do not validate the size if we have a filehandle. It will be checked later. - return $data if ref $data; - - $data || ThrowUserError('zero_length_file'); + $params->{filesize} || ThrowUserError('zero_length_file'); # Make sure the attachment does not exceed the maximum permitted size. - my $len = length($data); - my $max_size = $params->{store_in_file} ? Bugzilla->params->{'maxlocalattachment'} * 1048576 - : Bugzilla->params->{'maxattachmentsize'} * 1024; - if ($len > $max_size) { - my $vars = { filesize => sprintf("%.0f", $len/1024) }; - if ($params->{ispatch}) { - ThrowUserError('patch_too_large', $vars); - } - elsif ($params->{store_in_file}) { - ThrowUserError('local_file_too_large'); - } - else { - ThrowUserError('file_too_large', $vars); - } + my $max_size = max(Bugzilla->params->{'maxlocalattachment'} * 1048576, + Bugzilla->params->{'maxattachmentsize'} * 1024); + + if ($params->{filesize} > $max_size) { + my $vars = { filesize => sprintf("%.0f", $params->{filesize}/1024) }; + ThrowUserError('file_too_large', $vars); } return $data; } @@ -642,15 +621,6 @@ sub _check_is_url { return $is_url ? 1 : 0; } -sub _check_store_in_file { - my ($invocant, $store_in_file) = @_; - - if ($store_in_file && !Bugzilla->params->{'maxlocalattachment'}) { - ThrowCodeError('attachment_local_storage_disabled'); - } - return $store_in_file ? 1 : 0; -} - =pod =head2 Class Methods @@ -815,9 +785,6 @@ Params: takes a hashref with the following keys: the attachment is private. C<isurl> - boolean (optional, default false) - true if the attachment is a URL pointing to some external ressource. - C<store_in_file> - boolean (optional, default false) - true - if the attachment must be stored in data/attachments/ instead - of in the DB. Returns: The new attachment object. @@ -833,53 +800,44 @@ sub create { # Extract everything which is not a valid column name. my $bug = delete $params->{bug}; $params->{bug_id} = $bug->id; - my $fh = delete $params->{data}; - my $store_in_file = delete $params->{store_in_file}; + my $data = delete $params->{data}; + my $size = delete $params->{filesize}; my $attachment = $class->insert_create_data($params); my $attachid = $attachment->id; - # We only use $data here in this INSERT with a placeholder, - # so it's safe. - my $sth = $dbh->prepare("INSERT INTO attach_data - (id, thedata) VALUES ($attachid, ?)"); - - my $data = $store_in_file ? "" : $fh; - trick_taint($data); - $sth->bind_param(1, $data, $dbh->BLOB_TYPE); - $sth->execute(); - - # If the file is to be stored locally, stream the file from the web server - # to the local file without reading it into a local variable. - if ($store_in_file) { + # The file is too large to be stored in the DB, so we store it locally. + if ($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"; - open(AH, '>', "$attachdir/$hash/attachment.$attachid"); - binmode AH; - if (ref $fh) { - my $limit = Bugzilla->params->{"maxlocalattachment"} * 1048576; - my $sizecount = 0; - while (<$fh>) { - print AH $_; - $sizecount += length($_); - if ($sizecount > $limit) { - close AH; - close $fh; - unlink "$attachdir/$hash/attachment.$attachid"; - ThrowUserError("local_file_too_large"); - } - } - close $fh; + if (ref $data) { + copy($data, "$attachdir/$hash/attachment.$attachid"); + close $data; } else { - print AH $fh; + open(AH, '>', "$attachdir/$hash/attachment.$attachid"); + binmode AH; + print AH $data; + close AH; } - 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) { + local $/; + $data = <$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(); + # Return the new attachment object. return $attachment; } diff --git a/attachment.cgi b/attachment.cgi index cdfcc6bf7..ca82bc463 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -485,7 +485,6 @@ sub insert { isprivate => scalar $cgi->param('isprivate'), isurl => scalar $cgi->param('attachurl'), mimetype => $content_type, - store_in_file => scalar $cgi->param('bigfile'), }); foreach my $obsolete_attachment (@obsolete_attachments) { diff --git a/docs/en/xml/using.xml b/docs/en/xml/using.xml index 101a9d131..daa0720fa 100644 --- a/docs/en/xml/using.xml +++ b/docs/en/xml/using.xml @@ -332,9 +332,7 @@ <para> <emphasis>Attachments:</emphasis> You can attach files (e.g. testcases or patches) to bugs. If there - are any attachments, they are listed in this section. Attachments are - normally stored in the Bugzilla database, unless they are marked as - Big Files, which are stored directly on disk. + are any attachments, they are listed in this section. </para> </listitem> @@ -862,20 +860,6 @@ </para> <para> - If you have a really large attachment, something that does not need to - be recorded forever (as most attachments are), or something that is too - big for your database, you can mark your attachment as a - <quote>Big File</quote>, assuming the administrator of the installation - has enabled this feature. Big Files are stored directly on disk instead - of in the database. The maximum size of a <quote>Big File</quote> is - normally larger than the maximum size of a regular attachment. Independently - of the storage system used, an administrator can delete these attachments - at any time. Nevertheless, if these files are stored in the database, the - <quote>allow_attachment_deletion</quote> parameter (which is turned off - by default) must be enabled in order to delete them. - </para> - - <para> Also, if the administrator turned on the <quote>allow_attach_url</quote> parameter, you can enter the URL pointing to the attachment instead of uploading the attachment itself. For example, this is useful if you want to diff --git a/js/attachment.js b/js/attachment.js index d759248cd..314e4acb1 100644 --- a/js/attachment.js +++ b/js/attachment.js @@ -56,8 +56,7 @@ function setContentTypeDisabledState(form) function URLFieldHandler() { var field_attachurl = document.getElementById("attachurl"); var greyfields = new Array("data", "ispatch", "autodetect", - "list", "manual", "bigfile", - "contenttypeselection", + "list", "manual", "contenttypeselection", "contenttypeentry"); var i, thisfield; if (field_attachurl.value.match(/^\s*$/)) { @@ -103,8 +102,6 @@ function clearAttachmentFields() { document.getElementById('data').value = ''; DataFieldHandler(); - if ((element = document.getElementById('bigfile'))) - element.checked = ''; if ((element = document.getElementById('attachurl'))) { element.value = ''; URLFieldHandler(); diff --git a/post_bug.cgi b/post_bug.cgi index c097a96ce..956856b12 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -205,7 +205,6 @@ if (defined($cgi->upload('data')) || $cgi->param('attachurl')) { isprivate => scalar $cgi->param('isprivate'), isurl => scalar $cgi->param('attachurl'), mimetype => $content_type, - store_in_file => scalar $cgi->param('bigfile'), }); }; Bugzilla->error_mode($error_mode_cache); diff --git a/template/en/default/admin/params/attachment.html.tmpl b/template/en/default/admin/params/attachment.html.tmpl index 12fd491fc..159588625 100644 --- a/template/en/default/admin/params/attachment.html.tmpl +++ b/template/en/default/admin/params/attachment.html.tmpl @@ -64,13 +64,16 @@ "specify a URL when creating an attachment and " _ "treat the URL itself as if it were an attachment.", - maxattachmentsize => "The maximum size (in kilobytes) of attachments. " _ - "$terms.Bugzilla will not accept attachments greater than this number " _ - "of kilobytes in size. Setting this parameter to 0 will prevent " _ - "attaching files to ${terms.bugs}.", + 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}.", - maxlocalattachment => "The maximum size (in megabytes) of attachments identified by " _ - "the user as 'Big Files' to be stored locally on the webserver. " _ - "If set to zero, attachments will never be kept on the local " _ - "filesystem." } + 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." } %] diff --git a/template/en/default/attachment/createformcontents.html.tmpl b/template/en/default/attachment/createformcontents.html.tmpl index 2cef632d1..ad3a25bc6 100644 --- a/template/en/default/attachment/createformcontents.html.tmpl +++ b/template/en/default/attachment/createformcontents.html.tmpl @@ -32,18 +32,6 @@ > </td> </tr> -[% IF Param("maxlocalattachment") %] -<tr class="expert_fields"> - <th>BigFile:</th> - <td> - <input type="checkbox" id="bigfile" - name="bigfile" value="bigfile"> - <label for="bigfile"> - Big File - Stored locally and may be purged - </label> - </td> -</tr> -[% END %] [% IF Param("allow_attach_url") %] <tr class="expert_fields"> <th><label for="attachurl">AttachURL</label>:</th> diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index 177d47621..43644f703 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -37,11 +37,7 @@ [% DEFAULT title = "Internal Error" %] [% error_message = BLOCK %] - [% IF error == "attachment_local_storage_disabled" %] - [% title = "Local Storage Disabled" %] - You cannot store attachments locally. This feature is disabled. - - [% ELSIF error == "attachment_url_disabled" %] + [% IF error == "attachment_url_disabled" %] [% title = "Attachment URL Disabled" %] You cannot attach a URL. This feature is currently disabled. diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index b57792fd2..490941807 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -574,9 +574,13 @@ [% 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. Non-patch attachments cannot be more than - [%+ Param('maxattachmentsize') %] KB. <br> + 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> We recommend that you store your attachment elsewhere [% IF Param("allow_attach_url") %] and then specify the URL to this file on the attachment @@ -1023,11 +1027,6 @@ [% title = "Invalid Keyword Name" %] You may not use commas or whitespace in a keyword name. - [% ELSIF error == "local_file_too_large" %] - [% title = "Local File Too Large" %] - Local file uploads must not exceed - [% Param('maxlocalattachment') %] MB in size. - [% ELSIF error == "login_needed_for_password_change" %] [% title = "Login Name Required" %] You must enter a login name when requesting to change your password. @@ -1312,13 +1311,6 @@ The password must be at least [%+ constants.USER_PASSWORD_MIN_LENGTH FILTER html %] characters long. - [% ELSIF error == "patch_too_large" %] - [% title = "File Too Large" %] - The file you are trying to attach is [% filesize FILTER html %] - kilobytes (KB) in size. - Patches cannot be more than [% Param('maxattachmentsize') %] KB in size. - Try splitting your patch into several pieces. - [% ELSIF error == "product_access_denied" %] Either the product [%+ IF id.defined %] |