diff options
| -rw-r--r-- | Bugzilla/Attachment.pm | 14 | ||||
| -rwxr-xr-x | attachment.cgi | 82 |
2 files changed, 51 insertions, 45 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index b1aecd5b0..42372393c 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -720,7 +720,7 @@ Description: validates if the user is allowed to view and edit the attachment. Params: $attachment - the attachment object being edited. $product_id - the product ID the attachment belongs to. -Returns: 1 on success. Else an error is thrown. +Returns: 1 on success, 0 otherwise. =cut @@ -729,12 +729,9 @@ sub validate_can_edit { my $user = Bugzilla->user; # The submitter can edit their attachments. - return 1 if ($attachment->attacher->id == $user->id - || ((!$attachment->isprivate || $user->is_insider) - && $user->in_group('editbugs', $product_id))); - - # If we come here, then this attachment cannot be edited by the user. - ThrowUserError('illegal_attachment_edit', { attach_id => $attachment->id }); + return ($attachment->attacher->id == $user->id + || ((!$attachment->isprivate || $user->is_insider) + && $user->in_group('editbugs', $product_id))) ? 1 : 0; } =item C<validate_obsolete($bug)> @@ -769,7 +766,8 @@ sub validate_obsolete { || ThrowUserError('invalid_attach_id', $vars); # Check that the user can view and edit this attachment. - $attachment->validate_can_edit($bug->product_id); + $attachment->validate_can_edit($bug->product_id) + || ThrowUserError('illegal_attachment_edit', { attach_id => $attachment->id }); $vars->{'description'} = $attachment->description; diff --git a/attachment.cgi b/attachment.cgi index bbbf4afb3..32f6e5ec0 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -572,37 +572,39 @@ sub update { my $attachment = validateID(); my $bug = $attachment->bug; $attachment->_check_bug; - $attachment->validate_can_edit($bug->product_id); # FIXME: allow comments anyway. - - $attachment->set_description(scalar $cgi->param('description')); - $attachment->set_is_patch(scalar $cgi->param('ispatch')); - $attachment->set_content_type(scalar $cgi->param('contenttypeentry')); - $attachment->set_is_obsolete(scalar $cgi->param('isobsolete')); - $attachment->set_is_private(scalar $cgi->param('isprivate')); - $attachment->set_filename(scalar $cgi->param('filename')); - - # Now make sure the attachment has not been edited since we loaded the page. - if (defined $cgi->param('delta_ts') - && $cgi->param('delta_ts') ne $attachment->modification_time) - { - ($vars->{'operations'}) = - Bugzilla::Bug::GetBugActivity($bug->id, $attachment->id, $cgi->param('delta_ts')); - - # The token contains the old modification_time. We need a new one. - $cgi->param('token', issue_hash_token([$attachment->id, $attachment->modification_time])); - - # If the modification date changed but there is no entry in - # the activity table, this means someone commented only. - # In this case, there is no reason to midair. - if (scalar(@{$vars->{'operations'}})) { - $cgi->param('delta_ts', $attachment->modification_time); - $vars->{'attachment'} = $attachment; - - print $cgi->header(); - # Warn the user about the mid-air collision and ask them what to do. - $template->process("attachment/midair.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - exit; + my $can_edit = $attachment->validate_can_edit($bug->product_id); + + if ($can_edit) { + $attachment->set_description(scalar $cgi->param('description')); + $attachment->set_is_patch(scalar $cgi->param('ispatch')); + $attachment->set_content_type(scalar $cgi->param('contenttypeentry')); + $attachment->set_is_obsolete(scalar $cgi->param('isobsolete')); + $attachment->set_is_private(scalar $cgi->param('isprivate')); + $attachment->set_filename(scalar $cgi->param('filename')); + + # Now make sure the attachment has not been edited since we loaded the page. + if (defined $cgi->param('delta_ts') + && $cgi->param('delta_ts') ne $attachment->modification_time) + { + ($vars->{'operations'}) = + Bugzilla::Bug::GetBugActivity($bug->id, $attachment->id, $cgi->param('delta_ts')); + + # The token contains the old modification_time. We need a new one. + $cgi->param('token', issue_hash_token([$attachment->id, $attachment->modification_time])); + + # If the modification date changed but there is no entry in + # the activity table, this means someone commented only. + # In this case, there is no reason to midair. + if (scalar(@{$vars->{'operations'}})) { + $cgi->param('delta_ts', $attachment->modification_time); + $vars->{'attachment'} = $attachment; + + print $cgi->header(); + # Warn the user about the mid-air collision and ask them what to do. + $template->process("attachment/midair.html.tmpl", $vars) + || ThrowTemplateError($template->error()); + exit; + } } } @@ -622,16 +624,22 @@ sub update { $bug->add_comment($comment, { isprivate => $attachment->isprivate }); } - my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars); - $attachment->set_flags($flags, $new_flags); + if ($can_edit) { + my ($flags, $new_flags) = + Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars); + $attachment->set_flags($flags, $new_flags); + } # Figure out when the changes were made. my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); - my $changes = $attachment->update($timestamp); - # If there are changes, we updated delta_ts in the DB. We have to - # reflect this change in the bug object. - $bug->{delta_ts} = $timestamp if scalar(keys %$changes); + if ($can_edit) { + my $changes = $attachment->update($timestamp); + # If there are changes, we updated delta_ts in the DB. We have to + # reflect this change in the bug object. + $bug->{delta_ts} = $timestamp if scalar(keys %$changes); + } + # Commit the comment, if any. $bug->update($timestamp); |