Skip to content

bugzilla
bugzilla
Commit bd9062bb authored by Gervase Markham's avatar Gervase Markham
Browse files
Options

Revert "Bug 1073264 - allow attachment download to be offloaded to the webserver…

Revert "Bug 1073264 - allow attachment download to be offloaded to the webserver using X-SendFile or equivalent. r=gerv, a=glob." Morning brain thought this bug was approved for 5.0. This reverts commit 55e8faee.
parent 55e8faee
Hide whitespace changes
Inline Side-by-side
Showing with 4 additions and 94 deletions
Bugzilla/Attachment.pm
View file @ bd9062bb
......@@ -308,8 +308,7 @@ sub is_viewable {
=item C<data>
Returns the content of the attachment.
As a side-effect, sets $self->is_on_filesystem.
the content of the attachment
=back
......@@ -326,16 +325,10 @@ 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>;
......@@ -348,36 +341,9 @@ 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>
Returns the length (in bytes) of the attachment content.
As a side-effect, sets $self->is_on_filesystem.
the length (in bytes) of the attachment content
=back
......@@ -404,17 +370,11 @@ 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);
......
Bugzilla/Config/Attachment.pm
View file @ bd9062bb
......@@ -38,14 +38,6 @@ 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',
......
attachment.cgi
View file @ bd9062bb
......@@ -26,7 +26,6 @@ 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
......@@ -362,24 +361,9 @@ 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,
%$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;
}
-content_length => $attachment->datasize);
disable_utf8();
print $attachment->data;
}
......
template/en/default/admin/params/attachment.html.tmpl
View file @ bd9062bb
......@@ -58,31 +58,5 @@
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. " _
"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."
}
"attachments will never be kept on the local filesystem." }
%]
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment