diff options
| -rw-r--r-- | Bugzilla/Attachment.pm | 44 | ||||
| -rw-r--r-- | Bugzilla/Config/Attachment.pm | 8 | ||||
| -rwxr-xr-x | attachment.cgi | 18 | ||||
| -rw-r--r-- | template/en/default/admin/params/attachment.html.tmpl | 28 |
4 files changed, 94 insertions, 4 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index b44c94cd0..e165b139e 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -308,7 +308,8 @@ sub is_viewable { =item C<data> -the content of the attachment +Returns the content of the attachment. +As a side-effect, sets $self->is_on_filesystem. =back @@ -325,10 +326,16 @@ sub data { undef, $self->id); + # Setting the property here is cheap, as opposed to making an extra + # query later, and hitting the filesystem to see if the file is + # still there. + $self->{is_on_filesystem} = 0; # 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())) { + # file is actually on disk. + $self->{is_on_filesystem} = 1; local $/; binmode AH; $self->{data} = <AH>; @@ -341,9 +348,36 @@ sub data { =over +=item C<is_on_filesystem> + +Returns true if the attachment is stored on disk (via maxlocalattachment +parameter), as opposed to in the database. + +=back + +=cut + +# When the attachment is on the filesystem, you can let the backend +# (nginx, apache, lighttpd) serve it for you if it supports the X-Sendfile +# feature. This means that the attachment CGI script may have a reduced +# footprint. e.g. bug 906010 and bug 1073241. + +sub is_on_filesystem { + my $self = shift; + return $self->{is_on_filesystem} if exists $self->{is_on_filesystem}; + # In order to serve an attachment, you also send the datasize in the + # content-length header. Making additional queries which are exactly + # the same as found in the datasize code path is just wasteful. + my $datasize = $self->datasize; + return $self->{is_on_filesystem}; +} + +=over + =item C<datasize> -the length (in bytes) of the attachment content +Returns the length (in bytes) of the attachment content. +As a side-effect, sets $self->is_on_filesystem. =back @@ -370,11 +404,17 @@ sub datasize { WHERE id = ?", undef, $self->id) || 0; + # Setting the property here is cheap, as opposed to making an extra + # query later, and hitting the filesystem to see if the file is + # still there. + $self->{is_on_filesystem} = 0; # If there's no attachment data in the database, either the attachment # is stored in a local file, and so retrieve its size from the file, # or the attachment has been deleted. unless ($self->{datasize}) { if (open(AH, $self->_get_local_filename())) { + # file is actually on disk. + $self->{is_on_filesystem} = 1; binmode AH; $self->{datasize} = (stat(AH))[7]; close(AH); diff --git a/Bugzilla/Config/Attachment.pm b/Bugzilla/Config/Attachment.pm index 580ec46d9..5bf854293 100644 --- a/Bugzilla/Config/Attachment.pm +++ b/Bugzilla/Config/Attachment.pm @@ -38,6 +38,14 @@ sub get_param_list { }, { + name => 'xsendfile_header', + type => 's', + choices => ['off', 'X-Sendfile', 'X-Accel-Redirect', 'X-LIGHTTPD-send-file'], + default => 'off', + checker => \&check_multi + }, + + { name => 'maxattachmentsize', type => 't', default => '1000', diff --git a/attachment.cgi b/attachment.cgi index 5db8f5909..61e6f58d8 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -26,6 +26,7 @@ use Bugzilla::Attachment::PatchReader; use Bugzilla::Token; use Encode qw(encode find_encoding); +use Cwd qw(realpath); # For most scripts we don't make $cgi and $template global variables. But # when preparing Bugzilla for mod_perl, this script used these @@ -361,9 +362,24 @@ sub view { } } } + my $sendfile_header = {}; + my $sendfile_param = Bugzilla->params->{'xsendfile_header'}; + if ($attachment->is_on_filesystem && $sendfile_param ne 'off') { + # attachment is on filesystem and Admin turned on feature. + # This means we can let webserver handle the request and stream the file + # for us. This is called the X-Sendfile feature. see bug 1073264. + my $attachment_path = realpath($attachment->_get_local_filename()); + $sendfile_header->{$sendfile_param} = $attachment_path; + } print $cgi->header(-type=>"$contenttype; name=\"$filename\"", -content_disposition=> "$disposition; filename=\"$filename\"", - -content_length => $attachment->datasize); + -content_length => $attachment->datasize, + %$sendfile_header); + if ($attachment->is_on_filesystem && $sendfile_param ne 'off') { + # in case of X-Sendfile, we do not print the data. + # that is handled directly by the webserver. + return; + } disable_utf8(); print $attachment->data; } diff --git a/template/en/default/admin/params/attachment.html.tmpl b/template/en/default/admin/params/attachment.html.tmpl index 5efcc1106..c850802ab 100644 --- a/template/en/default/admin/params/attachment.html.tmpl +++ b/template/en/default/admin/params/attachment.html.tmpl @@ -58,5 +58,31 @@ 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\"><var>maxattachmentsize</var> parameter</a>, " _ - "attachments will never be kept on the local filesystem." } + "attachments will never be kept on the local filesystem. " _ + "If you want to store all attachments on disk rather than in the " _ + "database, then set <a href=\"#maxattachmentsize\">" _ + "<var>maxattachmentsize</var> parameter</a> to 0. ", + + + xsendfile_header => + "By default, attachments are served by the CGI script. " _ + "If you enable filesystem file storage for large files using the " _ + "<a href=\"#maxlocalattachment\"><var>maxlocalattachment</var> parameter</a> " _ + "then you can have those files served directly by the webserver, which " _ + "avoids copying them entirely into memory, and this may result in a " _ + "performance improvement. To do this, configure your webserver appropriately " _ + "and then set the correct header, as follows:" _ + "<ul>" _ + "<li>Apache: <code>X-Sendfile</code> header; see " _ + "<code><a href=\"https://tn123.org/mod_xsendfile/\">mod_xsendfile</a></code> module</li>" _ + "<li>nginx: <code>X-Accel-Redirect</code> header; see "_ + "<a href=\"http://wiki.nginx.org/X-accel\">webserver documentation</a> for additional configuration</li>" _ + "<li>lighttpd: <code>X-LIGHTTPD-send-file</code> header; see " _ + "<a href=\"http://redmine.lighttpd.net/projects/1/wiki/X-LIGHTTPD-send-file\">webserver documentation</a> for additional configuration</li>" _ + "</ul><br>" _ + "Please note that attachments stored in the database cannot be offloaded " _ + "to apache/nginx/lighttpd and are always handled by the CGI script." + + } + %] |