]> git.ipfire.org Git - thirdparty/bugzilla.git/commitdiff
Bug 490930: Always store attachments locally if they are over X size (and below some...
authorFrédéric Buclin <LpSolit@gmail.com>
Thu, 8 Jul 2010 16:58:33 +0000 (18:58 +0200)
committerFrédéric Buclin <LpSolit@gmail.com>
Thu, 8 Jul 2010 16:58:33 +0000 (18:58 +0200)
r=mkanat a=LpSolit

Bugzilla/Attachment.pm
attachment.cgi
docs/en/xml/using.xml
js/attachment.js
post_bug.cgi
template/en/default/admin/params/attachment.html.tmpl
template/en/default/attachment/createformcontents.html.tmpl
template/en/default/global/code-error.html.tmpl
template/en/default/global/user-error.html.tmpl

index ddce1f59382aca7666de47a108a647b68e724ee5..39afe5df1fd3dab93f33456c388f9705ddd79c28 100644 (file)
@@ -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;
 }
index cdfcc6bf7418d540ec9c15fc5e65f1c572e749b5..ca82bc4638b2bcc45395ec34b5ac78e5628d7b41 100755 (executable)
@@ -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) {
index 101a9d131ad060b144863504cee1f479e4f5a74a..daa0720fadd7d930d95047ff83243e9b70bc88a3 100644 (file)
         <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>
 
       <filename>&amp;content_type=text/plain</filename>.
     </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
index d759248cdfef5743477f1a82f961b99e2c468bde..314e4acb12d50b1272c85703ae469dd881a487c7 100644 (file)
@@ -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();
index c097a96ce259cbd5ed5c8a218ca6890e16dd861e..956856b1288ccbad9a7a2904788dc62148148497 100755 (executable)
@@ -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);
index 12fd491fceb9c987cb0e849a4468e3b72b9702e3..1595886251c95bd1c7631874a0229cb41b9ab9bd 100644 (file)
                       "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." }
 %]
index 2cef632d1851ffe26112c956d9dbda5a77b289ec..ad3a25bc62f4be0ef80b134d665e961c0196a770 100644 (file)
     >
   </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>
index 177d47621de51e892f9f3ad17dc3c35b7409d095..43644f7035c092c5f05c28a8b9bfc7f73f783d8b 100644 (file)
 [% 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.
 
index b57792fd2996040f6cfcb26a74e83a1f1e84de11..490941807a7d74a3cf31c9f9aae051be2d40412b 100644 (file)
 
   [% 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
     [% 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.
     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 %]