aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDamien Nozay <damien.nozay@gmail.com>2014-10-24 09:15:26 +0100
committerGervase Markham <gerv@gerv.net>2014-10-24 09:15:26 +0100
commit55e8faeed19ff618483cb5803847bdba6c80c752 (patch)
treef464b10e91ee12e60f0b28a3b6f86a59d6a337e5
parentd4146c594381f3303bb06b02dbe94be69c1da538 (diff)
downloadbugs-55e8faeed19ff618483cb5803847bdba6c80c752.tar
bugs-55e8faeed19ff618483cb5803847bdba6c80c752.tar.gz
bugs-55e8faeed19ff618483cb5803847bdba6c80c752.tar.bz2
bugs-55e8faeed19ff618483cb5803847bdba6c80c752.tar.xz
bugs-55e8faeed19ff618483cb5803847bdba6c80c752.zip
Bug 1073264 - allow attachment download to be offloaded to the webserver using X-SendFile or equivalent. r=gerv, a=glob.
-rw-r--r--Bugzilla/Attachment.pm44
-rw-r--r--Bugzilla/Config/Attachment.pm8
-rwxr-xr-xattachment.cgi18
-rw-r--r--template/en/default/admin/params/attachment.html.tmpl28
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."
+
+ }
+
%]