Skip to content

bugzilla
bugzilla
Commit 3c61e66f authored by Frédéric Buclin's avatar Frédéric Buclin
Browse files
Options

Bug 490930: Always store attachments locally if they are over X size (and below…

Bug 490930: Always store attachments locally if they are over X size (and below some threshold!), don't ever display "Big File" checkbox r=mkanat a=LpSolit
parent 3c64c526
Hide whitespace changes
Inline Side-by-side
Showing with 57 additions and 141 deletions
Bugzilla/Attachment.pm
View file @ 3c61e66f
......@@ -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;
}
......
attachment.cgi
View file @ 3c61e66f
......@@ -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) {
......
docs/en/xml/using.xml
View file @ 3c61e66f
......@@ -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
......
js/attachment.js
View file @ 3c61e66f
......@@ -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();
......
post_bug.cgi
View file @ 3c61e66f
......@@ -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);
......
template/en/default/admin/params/attachment.html.tmpl
View file @ 3c61e66f
......@@ -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." }
%]
template/en/default/attachment/createformcontents.html.tmpl
View file @ 3c61e66f
......@@ -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>
......
template/en/default/global/code-error.html.tmpl
View file @ 3c61e66f
......@@ -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.
......
template/en/default/global/user-error.html.tmpl
View file @ 3c61e66f
......@@ -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 %]
......
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