Commit 1afb5b3b authored by lpsolit%gmail.com's avatar lpsolit%gmail.com

Bug 376497: validateID() should return an attachment object - Patch by…

Bug 376497: validateID() should return an attachment object - Patch by Fré©ric Buclin <LpSolit@gmail.com> a=LpSolit
parent 52c8d09d
...@@ -728,8 +728,8 @@ sub insert_attachment_for_bug { ...@@ -728,8 +728,8 @@ sub insert_attachment_for_bug {
my $filename; my $filename;
my $contenttype; my $contenttype;
my $isurl; my $isurl;
$class->validate_is_patch($throw_error) || return 0; $class->validate_is_patch($throw_error) || return;
$class->validate_description($throw_error) || return 0; $class->validate_description($throw_error) || return;
if (Bugzilla->params->{'allow_attach_url'} if (Bugzilla->params->{'allow_attach_url'}
&& ($attachurl =~ /^(http|https|ftp):\/\/\S+/) && ($attachurl =~ /^(http|https|ftp):\/\/\S+/)
...@@ -743,16 +743,16 @@ sub insert_attachment_for_bug { ...@@ -743,16 +743,16 @@ sub insert_attachment_for_bug {
$cgi->delete('bigfile'); $cgi->delete('bigfile');
} }
else { else {
$filename = _validate_filename($throw_error) || return 0; $filename = _validate_filename($throw_error) || return;
# need to validate content type before data as # need to validate content type before data as
# we now check the content type for image/bmp in _validate_data() # we now check the content type for image/bmp in _validate_data()
unless ($cgi->param('ispatch')) { unless ($cgi->param('ispatch')) {
$class->validate_content_type($throw_error) || return 0; $class->validate_content_type($throw_error) || return;
} }
$data = _validate_data($throw_error, $hr_vars); $data = _validate_data($throw_error, $hr_vars);
# If the attachment is stored locally, $data eq ''. # If the attachment is stored locally, $data eq ''.
# If an error is thrown, $data eq '0'. # If an error is thrown, $data eq '0'.
($data ne '0') || return 0; ($data ne '0') || return;
$contenttype = $cgi->param('contenttype'); $contenttype = $cgi->param('contenttype');
# These are inserted using placeholders so no need to panic # These are inserted using placeholders so no need to panic
...@@ -826,7 +826,7 @@ sub insert_attachment_for_bug { ...@@ -826,7 +826,7 @@ sub insert_attachment_for_bug {
close AH; close AH;
close $fh; close $fh;
unlink "$attachdir/$hash/attachment.$attachid"; unlink "$attachdir/$hash/attachment.$attachid";
$throw_error ? ThrowUserError("local_file_too_large") : return 0; $throw_error ? ThrowUserError("local_file_too_large") : return;
} }
} }
close AH; close AH;
...@@ -874,8 +874,8 @@ sub insert_attachment_for_bug { ...@@ -874,8 +874,8 @@ sub insert_attachment_for_bug {
$$hr_vars->{'flag_creation_error'} = $@; $$hr_vars->{'flag_creation_error'} = $@;
} }
# Return the ID of the new attachment. # Return the new attachment object.
return $attachid; return $attachment;
} }
1; 1;
...@@ -129,21 +129,16 @@ exit; ...@@ -129,21 +129,16 @@ exit;
# 2) attachment does not exist, or 3) user isn't allowed to access the # 2) attachment does not exist, or 3) user isn't allowed to access the
# attachment. # attachment.
# #
# Returns a list, where the first item is the validated, detainted # Returns an attachment object.
# attachment id, and the 2nd item is the bug id corresponding to the
# attachment. sub validateID {
#
sub validateID
{
my $param = @_ ? $_[0] : 'id'; my $param = @_ ? $_[0] : 'id';
my $dbh = Bugzilla->dbh;
my $user = Bugzilla->user; my $user = Bugzilla->user;
# If we're not doing interdiffs, check if id wasn't specified and # If we're not doing interdiffs, check if id wasn't specified and
# prompt them with a page that allows them to choose an attachment. # prompt them with a page that allows them to choose an attachment.
# Happens when calling plain attachment.cgi from the urlbar directly # Happens when calling plain attachment.cgi from the urlbar directly
if ($param eq 'id' && !$cgi->param('id')) { if ($param eq 'id' && !$cgi->param('id')) {
print $cgi->header(); print $cgi->header();
$template->process("attachment/choose.html.tmpl", $vars) || $template->process("attachment/choose.html.tmpl", $vars) ||
ThrowTemplateError($template->error()); ThrowTemplateError($template->error());
...@@ -159,22 +154,16 @@ sub validateID ...@@ -159,22 +154,16 @@ sub validateID
|| ThrowUserError("invalid_attach_id", { attach_id => $cgi->param($param) }); || ThrowUserError("invalid_attach_id", { attach_id => $cgi->param($param) });
# Make sure the attachment exists in the database. # Make sure the attachment exists in the database.
my ($bugid, $isprivate, $submitter_id) = $dbh->selectrow_array( my $attachment = Bugzilla::Attachment->get($attach_id)
"SELECT bug_id, isprivate, submitter_id || ThrowUserError("invalid_attach_id", { attach_id => $attach_id });
FROM attachments
WHERE attach_id = ?",
undef, $attach_id);
ThrowUserError("invalid_attach_id", { attach_id => $attach_id })
unless $bugid;
# Make sure the user is authorized to access this attachment's bug. # Make sure the user is authorized to access this attachment's bug.
ValidateBugID($bugid); ValidateBugID($attachment->bug_id);
if ($isprivate && $user->id != $submitter_id && !$user->is_insider) { if ($attachment->isprivate && $user->id != $attachment->attacher->id && !$user->is_insider) {
ThrowUserError('auth_failure', {action => 'access', ThrowUserError('auth_failure', {action => 'access',
object => 'attachment'}); object => 'attachment'});
} }
return $attachment;
return ($attach_id, $bugid);
} }
# Validates format of a diff/interdiff. Takes a list as an parameter, which # Validates format of a diff/interdiff. Takes a list as an parameter, which
...@@ -272,18 +261,12 @@ sub isViewable ...@@ -272,18 +261,12 @@ sub isViewable
################################################################################ ################################################################################
# Display an attachment. # Display an attachment.
sub view sub view {
{
# Retrieve and validate parameters # Retrieve and validate parameters
my ($attach_id) = validateID(); my $attachment = validateID();
my $dbh = Bugzilla->dbh; my $contenttype = $attachment->contenttype;
my $filename = $attachment->filename;
# Retrieve the attachment content and its content type from the database.
my ($contenttype, $filename, $thedata) = $dbh->selectrow_array(
"SELECT mimetype, filename, thedata FROM attachments " .
"INNER JOIN attach_data ON id = attach_id " .
"WHERE attach_id = ?", undef, $attach_id);
# Bug 111522: allow overriding content-type manually in the posted form # Bug 111522: allow overriding content-type manually in the posted form
# params. # params.
if (defined $cgi->param('content_type')) if (defined $cgi->param('content_type'))
...@@ -295,69 +278,36 @@ sub view ...@@ -295,69 +278,36 @@ sub view
} }
# Return the appropriate HTTP response headers. # Return the appropriate HTTP response headers.
$filename =~ s/^.*[\/\\]//; $attachment->datasize || ThrowUserError("attachment_removed");
my $filesize = length($thedata);
# A zero length attachment in the database means the attachment is
# stored in a local file
if ($filesize == 0)
{
my $hash = ($attach_id % 100) + 100;
$hash =~ s/.*(\d\d)$/group.$1/;
if (open(AH, bz_locations()->{'attachdir'} . "/$hash/attachment.$attach_id")) {
binmode AH;
$filesize = (stat(AH))[7];
}
}
if ($filesize == 0)
{
ThrowUserError("attachment_removed");
}
$filename =~ s/^.*[\/\\]//;
# escape quotes and backslashes in the filename, per RFCs 2045/822 # escape quotes and backslashes in the filename, per RFCs 2045/822
$filename =~ s/\\/\\\\/g; # escape backslashes $filename =~ s/\\/\\\\/g; # escape backslashes
$filename =~ s/"/\\"/g; # escape quotes $filename =~ s/"/\\"/g; # escape quotes
print $cgi->header(-type=>"$contenttype; name=\"$filename\"", print $cgi->header(-type=>"$contenttype; name=\"$filename\"",
-content_disposition=> "inline; filename=\"$filename\"", -content_disposition=> "inline; filename=\"$filename\"",
-content_length => $filesize); -content_length => $attachment->datasize);
print $attachment->data;
if ($thedata) {
print $thedata;
} else {
while (<AH>) {
print $_;
}
close(AH);
}
} }
sub interdiff { sub interdiff {
# Retrieve and validate parameters # Retrieve and validate parameters
my ($old_id) = validateID('oldid'); my $old_attachment = validateID('oldid');
my ($new_id) = validateID('newid'); my $new_attachment = validateID('newid');
my $format = validateFormat('html', 'raw'); my $format = validateFormat('html', 'raw');
my $context = validateContext(); my $context = validateContext();
# XXX - validateID should be replaced by Attachment::check_attachment()
# and should return an attachment object. This would save us a lot of
# trouble.
my $old_attachment = Bugzilla::Attachment->get($old_id);
my $new_attachment = Bugzilla::Attachment->get($new_id);
Bugzilla::Attachment::PatchReader::process_interdiff( Bugzilla::Attachment::PatchReader::process_interdiff(
$old_attachment, $new_attachment, $format, $context); $old_attachment, $new_attachment, $format, $context);
} }
sub diff { sub diff {
# Retrieve and validate parameters # Retrieve and validate parameters
my ($attach_id) = validateID(); my $attachment = validateID();
my $format = validateFormat('html', 'raw'); my $format = validateFormat('html', 'raw');
my $context = validateContext(); my $context = validateContext();
my $attachment = Bugzilla::Attachment->get($attach_id);
# If it is not a patch, view normally. # If it is not a patch, view normally.
if (!$attachment->ispatch) { if (!$attachment->ispatch) {
view(); view();
...@@ -393,8 +343,7 @@ sub viewall { ...@@ -393,8 +343,7 @@ sub viewall {
} }
# Display a form for entering a new attachment. # Display a form for entering a new attachment.
sub enter sub enter {
{
# Retrieve and validate parameters # Retrieve and validate parameters
my $bugid = $cgi->param('bugid'); my $bugid = $cgi->param('bugid');
ValidateBugID($bugid); ValidateBugID($bugid);
...@@ -409,16 +358,13 @@ sub enter ...@@ -409,16 +358,13 @@ sub enter
if (!$user->in_group('editbugs', $bug->product_id)) { if (!$user->in_group('editbugs', $bug->product_id)) {
$canEdit = "AND submitter_id = " . $user->id; $canEdit = "AND submitter_id = " . $user->id;
} }
my $attachments = $dbh->selectall_arrayref( my $attach_ids = $dbh->selectcol_arrayref("SELECT attach_id FROM attachments
"SELECT attach_id AS id, description, isprivate WHERE bug_id = ? AND isobsolete = 0 $canEdit
FROM attachments ORDER BY attach_id", undef, $bugid);
WHERE bug_id = ?
AND isobsolete = 0 $canEdit
ORDER BY attach_id",{'Slice' =>{}}, $bugid);
# Define the variables and functions that will be passed to the UI template. # Define the variables and functions that will be passed to the UI template.
$vars->{'bug'} = $bug; $vars->{'bug'} = $bug;
$vars->{'attachments'} = $attachments; $vars->{'attachments'} = Bugzilla::Attachment->get_list($attach_ids);
my $flag_types = Bugzilla::FlagType::match({'target_type' => 'attachment', my $flag_types = Bugzilla::FlagType::match({'target_type' => 'attachment',
'product_id' => $bug->product_id, 'product_id' => $bug->product_id,
...@@ -434,8 +380,7 @@ sub enter ...@@ -434,8 +380,7 @@ sub enter
} }
# Insert a new attachment into the database. # Insert a new attachment into the database.
sub insert sub insert {
{
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
my $user = Bugzilla->user; my $user = Bugzilla->user;
...@@ -447,17 +392,16 @@ sub insert ...@@ -447,17 +392,16 @@ sub insert
my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()"); my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
my $bug = new Bugzilla::Bug($bugid); my $bug = new Bugzilla::Bug($bugid);
my $attachid = my $attachment =
Bugzilla::Attachment->insert_attachment_for_bug(THROW_ERROR, $bug, $user, Bugzilla::Attachment->insert_attachment_for_bug(THROW_ERROR, $bug, $user,
$timestamp, \$vars); $timestamp, \$vars);
# Insert a comment about the new attachment into the database. # Insert a comment about the new attachment into the database.
my $comment = "Created an attachment (id=$attachid)\n" . my $comment = "Created an attachment (id=" . $attachment->id . ")\n" .
$cgi->param('description') . "\n"; $attachment->description . "\n";
$comment .= ("\n" . $cgi->param('comment')) if defined $cgi->param('comment'); $comment .= ("\n" . $cgi->param('comment')) if defined $cgi->param('comment');
my $isprivate = $cgi->param('isprivate') ? 1 : 0; AppendComment($bugid, $user->id, $comment, $attachment->isprivate, $timestamp);
AppendComment($bugid, $user->id, $comment, $isprivate, $timestamp);
# Assign the bug to the user, if they are allowed to take it # Assign the bug to the user, if they are allowed to take it
my $owner = ""; my $owner = "";
...@@ -504,14 +448,10 @@ sub insert ...@@ -504,14 +448,10 @@ sub insert
# Define the variables and functions that will be passed to the UI template. # Define the variables and functions that will be passed to the UI template.
$vars->{'mailrecipients'} = { 'changer' => $user->login, $vars->{'mailrecipients'} = { 'changer' => $user->login,
'owner' => $owner }; 'owner' => $owner };
$vars->{'bugid'} = $bugid; $vars->{'attachment'} = $attachment;
$vars->{'attachid'} = $attachid;
$vars->{'description'} = $cgi->param('description');
$vars->{'contenttypemethod'} = $cgi->param('contenttypemethod'); $vars->{'contenttypemethod'} = $cgi->param('contenttypemethod');
$vars->{'contenttype'} = $cgi->param('contenttype');
print $cgi->header(); print $cgi->header();
# Generate and return the UI (HTML page) from the appropriate template. # Generate and return the UI (HTML page) from the appropriate template.
$template->process("attachment/created.html.tmpl", $vars) $template->process("attachment/created.html.tmpl", $vars)
|| ThrowTemplateError($template->error()); || ThrowTemplateError($template->error());
...@@ -522,10 +462,9 @@ sub insert ...@@ -522,10 +462,9 @@ sub insert
# is private and the user does not belong to the insider group. # is private and the user does not belong to the insider group.
# Validations are done later when the user submits changes. # Validations are done later when the user submits changes.
sub edit { sub edit {
my ($attach_id) = validateID(); my $attachment = validateID();
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
my $attachment = Bugzilla::Attachment->get($attach_id);
my $isviewable = !$attachment->isurl && isViewable($attachment->contenttype); my $isviewable = !$attachment->isurl && isViewable($attachment->contenttype);
# Retrieve a list of attachments for this bug as well as a summary of the bug # Retrieve a list of attachments for this bug as well as a summary of the bug
...@@ -572,19 +511,16 @@ sub edit { ...@@ -572,19 +511,16 @@ sub edit {
# content type, ispatch and isobsolete flags, and statuses, and they can # content type, ispatch and isobsolete flags, and statuses, and they can
# also submit a comment that appears in the bug. # also submit a comment that appears in the bug.
# Users cannot edit the content of the attachment itself. # Users cannot edit the content of the attachment itself.
sub update sub update {
{
my $user = Bugzilla->user; my $user = Bugzilla->user;
my $userid = $user->id;
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
# Retrieve and validate parameters # Retrieve and validate parameters
ValidateComment(scalar $cgi->param('comment')); ValidateComment(scalar $cgi->param('comment'));
my ($attach_id, $bugid) = validateID(); my $attachment = validateID();
my $bug = new Bugzilla::Bug($bugid); my $bug = new Bugzilla::Bug($attachment->bug_id);
my $attachment = Bugzilla::Attachment->get($attach_id);
$attachment->validate_can_edit($bug->product_id); $attachment->validate_can_edit($bug->product_id);
validateCanChangeBug($bugid); validateCanChangeBug($bug->id);
Bugzilla::Attachment->validate_description(THROW_ERROR); Bugzilla::Attachment->validate_description(THROW_ERROR);
Bugzilla::Attachment->validate_is_patch(THROW_ERROR); Bugzilla::Attachment->validate_is_patch(THROW_ERROR);
Bugzilla::Attachment->validate_content_type(THROW_ERROR) unless $cgi->param('ispatch'); Bugzilla::Attachment->validate_content_type(THROW_ERROR) unless $cgi->param('ispatch');
...@@ -599,9 +535,7 @@ sub update ...@@ -599,9 +535,7 @@ sub update
# old private bit twice (first here, and then below again), but this is # old private bit twice (first here, and then below again), but this is
# the less risky change. # the less risky change.
unless ($user->is_insider) { unless ($user->is_insider) {
my $oldisprivate = $dbh->selectrow_array('SELECT isprivate FROM attachments $cgi->param('isprivate', $attachment->isprivate);
WHERE attach_id = ?', undef, $attach_id);
$cgi->param('isprivate', $oldisprivate);
} }
# The order of these function calls is important, as Flag::validate # The order of these function calls is important, as Flag::validate
...@@ -610,7 +544,7 @@ sub update ...@@ -610,7 +544,7 @@ sub update
Bugzilla::User::match_field($cgi, { Bugzilla::User::match_field($cgi, {
'^requestee(_type)?-(\d+)$' => { 'type' => 'multi' } '^requestee(_type)?-(\d+)$' => { 'type' => 'multi' }
}); });
Bugzilla::Flag::validate($cgi, $bugid, $attach_id); Bugzilla::Flag::validate($cgi, $bug->id, $attachment->id);
# Lock database tables in preparation for updating the attachment. # Lock database tables in preparation for updating the attachment.
$dbh->bz_lock_tables('attachments WRITE', 'flags WRITE' , $dbh->bz_lock_tables('attachments WRITE', 'flags WRITE' ,
...@@ -625,13 +559,6 @@ sub update ...@@ -625,13 +559,6 @@ sub update
'cc READ', 'bug_group_map READ', 'user_group_map READ', 'cc READ', 'bug_group_map READ', 'user_group_map READ',
'group_group_map READ', 'groups READ', 'group_control_map READ'); 'group_group_map READ', 'groups READ', 'group_control_map READ');
# Get a copy of the attachment record before we make changes
# so we can record those changes in the activity table.
my ($olddescription, $oldcontenttype, $oldfilename, $oldispatch,
$oldisobsolete, $oldisprivate) = $dbh->selectrow_array(
"SELECT description, mimetype, filename, ispatch, isobsolete, isprivate
FROM attachments WHERE attach_id = ?", undef, $attach_id);
# Quote the description and content type for use in the SQL UPDATE statement. # Quote the description and content type for use in the SQL UPDATE statement.
my $description = $cgi->param('description'); my $description = $cgi->param('description');
my $contenttype = $cgi->param('contenttype'); my $contenttype = $cgi->param('contenttype');
...@@ -661,56 +588,43 @@ sub update ...@@ -661,56 +588,43 @@ sub update
WHERE attach_id = ?", WHERE attach_id = ?",
undef, ($description, $contenttype, $filename, undef, ($description, $contenttype, $filename,
$cgi->param('ispatch'), $cgi->param('isobsolete'), $cgi->param('ispatch'), $cgi->param('isobsolete'),
$cgi->param('isprivate'), $attach_id)); $cgi->param('isprivate'), $attachment->id));
my $updated_attachment = Bugzilla::Attachment->get($attachment->id);
# Record changes in the activity table. # Record changes in the activity table.
if ($olddescription ne $cgi->param('description')) { my $sth = $dbh->prepare('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
fieldid, removed, added)
VALUES (?, ?, ?, ?, ?, ?, ?)');
if ($attachment->description ne $updated_attachment->description) {
my $fieldid = get_field_id('attachments.description'); my $fieldid = get_field_id('attachments.description');
$dbh->do("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
fieldid, removed, added) $attachment->description, $updated_attachment->description);
VALUES (?,?,?,?,?,?,?)",
undef, ($bugid, $attach_id, $userid, $timestamp, $fieldid,
$olddescription, $description));
} }
if ($oldcontenttype ne $cgi->param('contenttype')) { if ($attachment->contenttype ne $updated_attachment->contenttype) {
my $fieldid = get_field_id('attachments.mimetype'); my $fieldid = get_field_id('attachments.mimetype');
$dbh->do("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
fieldid, removed, added) $attachment->contenttype, $updated_attachment->contenttype);
VALUES (?,?,?,?,?,?,?)",
undef, ($bugid, $attach_id, $userid, $timestamp, $fieldid,
$oldcontenttype, $contenttype));
} }
if ($oldfilename ne $cgi->param('filename')) { if ($attachment->filename ne $updated_attachment->filename) {
my $fieldid = get_field_id('attachments.filename'); my $fieldid = get_field_id('attachments.filename');
$dbh->do("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
fieldid, removed, added) $attachment->filename, $updated_attachment->filename);
VALUES (?,?,?,?,?,?,?)",
undef, ($bugid, $attach_id, $userid, $timestamp, $fieldid,
$oldfilename, $filename));
} }
if ($oldispatch ne $cgi->param('ispatch')) { if ($attachment->ispatch != $updated_attachment->ispatch) {
my $fieldid = get_field_id('attachments.ispatch'); my $fieldid = get_field_id('attachments.ispatch');
$dbh->do("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
fieldid, removed, added) $attachment->ispatch, $updated_attachment->ispatch);
VALUES (?,?,?,?,?,?,?)",
undef, ($bugid, $attach_id, $userid, $timestamp, $fieldid,
$oldispatch, $cgi->param('ispatch')));
} }
if ($oldisobsolete ne $cgi->param('isobsolete')) { if ($attachment->isobsolete != $updated_attachment->isobsolete) {
my $fieldid = get_field_id('attachments.isobsolete'); my $fieldid = get_field_id('attachments.isobsolete');
$dbh->do("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
fieldid, removed, added) $attachment->isobsolete, $updated_attachment->isobsolete);
VALUES (?,?,?,?,?,?,?)",
undef, ($bugid, $attach_id, $userid, $timestamp, $fieldid,
$oldisobsolete, $cgi->param('isobsolete')));
} }
if ($oldisprivate ne $cgi->param('isprivate')) { if ($attachment->isprivate != $updated_attachment->isprivate) {
my $fieldid = get_field_id('attachments.isprivate'); my $fieldid = get_field_id('attachments.isprivate');
$dbh->do("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
fieldid, removed, added) $attachment->isprivate, $updated_attachment->isprivate);
VALUES (?,?,?,?,?,?,?)",
undef, ($bugid, $attach_id, $userid, $timestamp, $fieldid,
$oldisprivate, $cgi->param('isprivate')));
} }
# Unlock all database tables now that we are finished updating the database. # Unlock all database tables now that we are finished updating the database.
...@@ -722,17 +636,16 @@ sub update ...@@ -722,17 +636,16 @@ sub update
{ {
# Prepend a string to the comment to let users know that the comment came # Prepend a string to the comment to let users know that the comment came
# from the "edit attachment" screen. # from the "edit attachment" screen.
my $comment = qq|(From update of attachment $attach_id)\n| . my $comment = "(From update of attachment " . $attachment->id . ")\n" .
$cgi->param('comment'); $cgi->param('comment');
# Append the comment to the list of comments in the database. # Append the comment to the list of comments in the database.
AppendComment($bugid, $userid, $comment, $cgi->param('isprivate'), $timestamp); AppendComment($bug->id, $user->id, $comment, $updated_attachment->isprivate, $timestamp);
} }
# Define the variables and functions that will be passed to the UI template. # Define the variables and functions that will be passed to the UI template.
$vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login }; $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login };
$vars->{'attachid'} = $attach_id; $vars->{'attachment'} = $attachment;
$vars->{'bugid'} = $bugid;
print $cgi->header(); print $cgi->header();
...@@ -757,9 +670,8 @@ sub delete_attachment { ...@@ -757,9 +670,8 @@ sub delete_attachment {
|| ThrowUserError('attachment_deletion_disabled'); || ThrowUserError('attachment_deletion_disabled');
# Make sure the administrator is allowed to edit this attachment. # Make sure the administrator is allowed to edit this attachment.
my ($attach_id, $bug_id) = validateID(); my $attachment = validateID();
my $attachment = Bugzilla::Attachment->get($attach_id); validateCanChangeBug($attachment->bug_id);
validateCanChangeBug($bug_id);
$attachment->datasize || ThrowUserError('attachment_removed'); $attachment->datasize || ThrowUserError('attachment_removed');
...@@ -769,7 +681,7 @@ sub delete_attachment { ...@@ -769,7 +681,7 @@ sub delete_attachment {
my ($creator_id, $date, $event) = Bugzilla::Token::GetTokenData($token); my ($creator_id, $date, $event) = Bugzilla::Token::GetTokenData($token);
unless ($creator_id unless ($creator_id
&& ($creator_id == $user->id) && ($creator_id == $user->id)
&& ($event eq "attachment$attach_id")) && ($event eq 'attachment' . $attachment->id))
{ {
# The token is invalid. # The token is invalid.
ThrowUserError('token_inexistent'); ThrowUserError('token_inexistent');
...@@ -777,8 +689,7 @@ sub delete_attachment { ...@@ -777,8 +689,7 @@ sub delete_attachment {
# The token is valid. Delete the content of the attachment. # The token is valid. Delete the content of the attachment.
my $msg; my $msg;
$vars->{'attachid'} = $attach_id; $vars->{'attachment'} = $attachment;
$vars->{'bugid'} = $bug_id;
$vars->{'date'} = $date; $vars->{'date'} = $date;
$vars->{'reason'} = clean_text($cgi->param('reason') || ''); $vars->{'reason'} = clean_text($cgi->param('reason') || '');
$vars->{'mailrecipients'} = { 'changer' => $user->login }; $vars->{'mailrecipients'} = { 'changer' => $user->login };
...@@ -787,12 +698,12 @@ sub delete_attachment { ...@@ -787,12 +698,12 @@ sub delete_attachment {
|| ThrowTemplateError($template->error()); || ThrowTemplateError($template->error());
$dbh->bz_lock_tables('attachments WRITE', 'attach_data WRITE', 'flags WRITE'); $dbh->bz_lock_tables('attachments WRITE', 'attach_data WRITE', 'flags WRITE');
$dbh->do('DELETE FROM attach_data WHERE id = ?', undef, $attach_id); $dbh->do('DELETE FROM attach_data WHERE id = ?', undef, $attachment->id);
$dbh->do('UPDATE attachments SET mimetype = ?, ispatch = ?, isurl = ?, $dbh->do('UPDATE attachments SET mimetype = ?, ispatch = ?, isurl = ?,
isobsolete = ? isobsolete = ?
WHERE attach_id = ?', undef, WHERE attach_id = ?', undef,
('text/plain', 0, 0, 1, $attach_id)); ('text/plain', 0, 0, 1, $attachment->id));
$dbh->do('DELETE FROM flags WHERE attach_id = ?', undef, $attach_id); $dbh->do('DELETE FROM flags WHERE attach_id = ?', undef, $attachment->id);
$dbh->bz_unlock_tables; $dbh->bz_unlock_tables;
# If the attachment is stored locally, remove it. # If the attachment is stored locally, remove it.
...@@ -804,14 +715,14 @@ sub delete_attachment { ...@@ -804,14 +715,14 @@ sub delete_attachment {
delete_token($token); delete_token($token);
# Paste the reason provided by the admin into a comment. # Paste the reason provided by the admin into a comment.
AppendComment($bug_id, $user->id, $msg); AppendComment($attachment->bug_id, $user->id, $msg);
$template->process("attachment/updated.html.tmpl", $vars) $template->process("attachment/updated.html.tmpl", $vars)
|| ThrowTemplateError($template->error()); || ThrowTemplateError($template->error());
} }
else { else {
# Create a token. # Create a token.
$token = issue_session_token('attachment' . $attach_id); $token = issue_session_token('attachment' . $attachment->id);
$vars->{'a'} = $attachment; $vars->{'a'} = $attachment;
$vars->{'token'} = $token; $vars->{'token'} = $token;
......
...@@ -187,15 +187,15 @@ if (defined $cgi->param('version')) { ...@@ -187,15 +187,15 @@ if (defined $cgi->param('version')) {
# Add an attachment if requested. # Add an attachment if requested.
if (defined($cgi->upload('data')) || $cgi->param('attachurl')) { if (defined($cgi->upload('data')) || $cgi->param('attachurl')) {
$cgi->param('isprivate', $cgi->param('commentprivacy')); $cgi->param('isprivate', $cgi->param('commentprivacy'));
my $attach_id = Bugzilla::Attachment->insert_attachment_for_bug(!THROW_ERROR, my $attachment = Bugzilla::Attachment->insert_attachment_for_bug(!THROW_ERROR,
$bug, $user, $timestamp, \$vars); $bug, $user, $timestamp, \$vars);
if ($attach_id) { if ($attachment) {
# Update the comment to include the new attachment ID. # Update the comment to include the new attachment ID.
# This string is hardcoded here because Template::quoteUrls() # This string is hardcoded here because Template::quoteUrls()
# expects to find this exact string. # expects to find this exact string.
my $new_comment = "Created an attachment (id=$attach_id)\n" . my $new_comment = "Created an attachment (id=" . $attachment->id . ")\n" .
$cgi->param('description') . "\n"; $attachment->description . "\n";
# We can use $bug->longdescs here because we are sure that the bug # We can use $bug->longdescs here because we are sure that the bug
# description is of type CMT_NORMAL. No need to include it if it's # description is of type CMT_NORMAL. No need to include it if it's
# empty, though. # empty, though.
......
...@@ -20,10 +20,7 @@ ...@@ -20,10 +20,7 @@
#%] #%]
[%# INTERFACE: [%# INTERFACE:
# bugid: integer. ID of the bug we just attached an attachment to. # attachment: object of the attachment just created.
# attachid: integer. ID of the attachment just created.
# description: string. Description of the attachment just created.
# contenttype: string. The Content Type we attached it as.
# contenttypemethod: string. How we got the content type of the attachment. # contenttypemethod: string. How we got the content type of the attachment.
# Possible values: autodetect, list, manual. # Possible values: autodetect, list, manual.
#%] #%]
...@@ -36,11 +33,12 @@ ...@@ -36,11 +33,12 @@
<dl> <dl>
<dt> <dt>
<a title="[% description FILTER html %]" href="attachment.cgi?id=[% attachid %]&amp;action=edit">Attachment #[% attachid %]</a> <a title="[% attachment.description FILTER html %]"
to [% "$terms.bug $bugid" FILTER bug_link(bugid) %] created href="attachment.cgi?id=[% attachment.id %]&amp;action=edit">Attachment #[% attachment.id %]</a>
to [% "$terms.bug $attachment.bug_id" FILTER bug_link(attachment.bug_id) FILTER none %] created
</dt> </dt>
<dd> <dd>
[% PROCESS "bug/process/bugmail.html.tmpl" mailing_bugid = bugid %] [% PROCESS "bug/process/bugmail.html.tmpl" mailing_bugid = attachment.bug_id %]
[% IF convertedbmp %] [% IF convertedbmp %]
<p> <p>
<b>Note:</b> [% terms.Bugzilla %] automatically converted your BMP image file to a <b>Note:</b> [% terms.Bugzilla %] automatically converted your BMP image file to a
...@@ -50,9 +48,9 @@ ...@@ -50,9 +48,9 @@
[% IF contenttypemethod == 'autodetect' %] [% IF contenttypemethod == 'autodetect' %]
<p> <p>
<b>Note:</b> [% terms.Bugzilla %] automatically detected the content type <b>Note:</b> [% terms.Bugzilla %] automatically detected the content type
<em>[% contenttype %]</em> for this attachment. If this is <em>[% attachment.contenttype FILTER html %]</em> for this attachment. If this is
incorrect, correct the value by incorrect, correct the value by editing the attachment's
editing the attachment's <a href="attachment.cgi?id=[% attachid %]&amp;action=edit">details</a>. <a href="attachment.cgi?id=[% attachment.id %]&amp;action=edit">details</a>.
</p> </p>
[% END %] [% END %]
...@@ -62,8 +60,8 @@ ...@@ -62,8 +60,8 @@
</dl> </dl>
<p> <p>
<a href="attachment.cgi?bugid=[% bugid %]&amp;action=enter">Create <a href="attachment.cgi?bugid=[% attachment.bug_id %]&amp;action=enter">Create
Another Attachment to [% terms.Bug %] #[% bugid %]</a> Another Attachment to [% terms.Bug %] #[% attachment.bug_id %]</a>
</p> </p>
[% PROCESS global/footer.html.tmpl %] [% PROCESS global/footer.html.tmpl %]
...@@ -15,12 +15,12 @@ ...@@ -15,12 +15,12 @@
#%] #%]
[%# INTERFACE: [%# INTERFACE:
# attachid: ID of the attachment the user wants to delete. # attachment: object of the attachment the user wants to delete.
# reason: string; The reason provided by the user. # reason: string; The reason provided by the user.
# date: the date when the request to delete the attachment was made. # date: the date when the request to delete the attachment was made.
#%] #%]
The content of attachment [% attachid %] has been deleted by The content of attachment [% attachment.id %] has been deleted by
[%+ user.identity %] [%+ user.identity %]
[% IF reason %] [% IF reason %]
who provided the following reason: who provided the following reason:
......
...@@ -21,8 +21,7 @@ ...@@ -21,8 +21,7 @@
#%] #%]
[%# INTERFACE: [%# INTERFACE:
# bugid: integer. ID of the bug we are updating. # attachment: object of the attachment we just attached.
# attachid: integer. ID of the attachment we just attached.
#%] #%]
[% PROCESS global/variables.none.tmpl %] [% PROCESS global/variables.none.tmpl %]
...@@ -33,11 +32,11 @@ ...@@ -33,11 +32,11 @@
<dl> <dl>
<dt>Changes to <dt>Changes to
<a href="attachment.cgi?id=[% attachid %]&amp;action=edit">attachment [% attachid %]</a> <a href="attachment.cgi?id=[% attachment.id %]&amp;action=edit">attachment [% attachment.id %]</a>
of [% "$terms.bug $bugid" FILTER bug_link(bugid) %] submitted of [% "$terms.bug $attachment.bug_id" FILTER bug_link(attachment.bug_id) FILTER none %] submitted
</dt> </dt>
<dd> <dd>
[% PROCESS "bug/process/bugmail.html.tmpl" mailing_bugid = bugid %] [% PROCESS "bug/process/bugmail.html.tmpl" mailing_bugid = attachment.bug_id %]
[%# Links to more information about the changed bug. %] [%# Links to more information about the changed bug. %]
[% Hook.process("links") %] [% Hook.process("links") %]
</dd> </dd>
......
...@@ -414,10 +414,8 @@ ...@@ -414,10 +414,8 @@
], ],
'attachment/created.html.tmpl' => [ 'attachment/created.html.tmpl' => [
'attachid', 'attachment.id',
'bugid', 'attachment.bug_id',
'contenttype',
'"$terms.bug $bugid" FILTER bug_link(bugid)',
], ],
'attachment/edit.html.tmpl' => [ 'attachment/edit.html.tmpl' => [
...@@ -439,8 +437,7 @@ ...@@ -439,8 +437,7 @@
], ],
'attachment/updated.html.tmpl' => [ 'attachment/updated.html.tmpl' => [
'attachid', 'attachment.id',
'"$terms.bug $bugid" FILTER bug_link(bugid)',
], ],
'attachment/diff-header.html.tmpl' => [ 'attachment/diff-header.html.tmpl' => [
......
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