diff options
| -rw-r--r-- | Bugzilla/Attachment.pm | 49 | ||||
| -rwxr-xr-x | attachment.cgi | 61 | ||||
| -rw-r--r-- | template/en/default/attachment/edit.html.tmpl | 26 | ||||
| -rw-r--r-- | template/en/default/attachment/show-multiple.html.tmpl | 2 |
4 files changed, 55 insertions, 83 deletions
diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index 893c46682..736959b2f 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -283,6 +283,40 @@ sub isprivate { =over +=item C<is_viewable> + +Returns 1 if the attachment has a content-type viewable in this browser. +Note that we don't use $cgi->Accept()'s ability to check if a content-type +matches, because this will return a value even if it's matched by the generic +*/* which most browsers add to the end of their Accept: headers. + +=back + +=cut + +sub is_viewable { + my $self = shift; + my $contenttype = $self->contenttype; + my $cgi = Bugzilla->cgi; + + # We assume we can view all text and image types. + return 1 if ($contenttype =~ /^(text|image)\//); + + # Mozilla can view XUL. Note the trailing slash on the Gecko detection to + # avoid sending XUL to Safari. + return 1 if (($contenttype =~ /^application\/vnd\.mozilla\./) + && ($cgi->user_agent() =~ /Gecko\//)); + + # If it's not one of the above types, we check the Accept: header for any + # types mentioned explicitly. + my $accept = join(",", $cgi->Accept()); + return 1 if ($accept =~ /^(.*,)?\Q$contenttype\E(,.*)?$/); + + return 0; +} + +=over + =item C<data> the content of the attachment @@ -625,19 +659,12 @@ Returns: 1 on success. Else an error is thrown. sub validate_can_edit { my ($attachment, $product_id) = @_; - my $dbh = Bugzilla->dbh; my $user = Bugzilla->user; - # Bug 97729 - the submitter can edit their attachments. - return if ($attachment->attacher->id == $user->id); - - # Only users in the insider group can view private attachments. - if ($attachment->isprivate && !$user->is_insider) { - ThrowUserError('illegal_attachment_edit', {attach_id => $attachment->id}); - } - - # Users with editbugs privs can edit all attachments. - return if $user->in_group('editbugs', $product_id); + # 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 seen by the user. ThrowUserError('illegal_attachment_edit', { attach_id => $attachment->id }); diff --git a/attachment.cgi b/attachment.cgi index 690a42d67..b60f0183b 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -208,54 +208,6 @@ sub validateCanChangeBug { bug_id => $bugid }); } -sub validateIsObsolete -{ - # Set the isobsolete flag to zero if it is undefined, since the UI uses - # an HTML checkbox to represent this flag, and unchecked HTML checkboxes - # do not get sent in HTML requests. - $cgi->param('isobsolete', $cgi->param('isobsolete') ? 1 : 0); -} - -sub validatePrivate -{ - # Set the isprivate flag to zero if it is undefined, since the UI uses - # an HTML checkbox to represent this flag, and unchecked HTML checkboxes - # do not get sent in HTML requests. - $cgi->param('isprivate', $cgi->param('isprivate') ? 1 : 0); -} - -# Returns 1 if the parameter is a content-type viewable in this browser -# Note that we don't use $cgi->Accept()'s ability to check if a content-type -# matches, because this will return a value even if it's matched by the generic -# */* which most browsers add to the end of their Accept: headers. -sub isViewable -{ - my $contenttype = trim(shift); - - # We assume we can view all text and image types - if ($contenttype =~ /^(text|image)\//) { - return 1; - } - - # Mozilla can view XUL. Note the trailing slash on the Gecko detection to - # avoid sending XUL to Safari. - if (($contenttype =~ /^application\/vnd\.mozilla\./) && - ($cgi->user_agent() =~ /Gecko\//)) - { - return 1; - } - - # If it's not one of the above types, we check the Accept: header for any - # types mentioned explicitly. - my $accept = join(",", $cgi->Accept()); - - if ($accept =~ /^(.*,)?\Q$contenttype\E(,.*)?$/) { - return 1; - } - - return 0; -} - ################################################################################ # Functions ################################################################################ @@ -327,10 +279,6 @@ sub viewall { my $attachments = Bugzilla::Attachment->get_attachments_by_bug($bugid); - foreach my $a (@$attachments) { - $a->{'isviewable'} = isViewable($a->contenttype); - } - # Define the variables and functions that will be passed to the UI template. $vars->{'bug'} = $bug; $vars->{'attachments'} = $attachments; @@ -465,8 +413,6 @@ sub edit { my $attachment = validateID(); my $dbh = Bugzilla->dbh; - my $isviewable = !$attachment->isurl && isViewable($attachment->contenttype); - # Retrieve a list of attachments for this bug as well as a summary of the bug # to use in a navigation bar across the top of the screen. my $bugattachments = @@ -491,8 +437,7 @@ sub edit { $vars->{'any_flags_requesteeble'} = grep($_->is_requesteeble, @$flag_types); $vars->{'attachment'} = $attachment; $vars->{'bugsummary'} = $bugsummary; - $vars->{'isviewable'} = $isviewable; - $vars->{'attachments'} = $bugattachments; + $vars->{'attachments'} = $bugattachments; # Determine if PatchReader is installed eval { @@ -524,8 +469,8 @@ sub update { Bugzilla::Attachment->validate_description(THROW_ERROR); Bugzilla::Attachment->validate_is_patch(THROW_ERROR); Bugzilla::Attachment->validate_content_type(THROW_ERROR) unless $cgi->param('ispatch'); - validateIsObsolete(); - validatePrivate(); + $cgi->param('isobsolete', $cgi->param('isobsolete') ? 1 : 0); + $cgi->param('isprivate', $cgi->param('isprivate') ? 1 : 0); # If the submitter of the attachment is not in the insidergroup, # be sure that he cannot overwrite the private bit. diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl index d0a75b3fd..10438a7a5 100644 --- a/template/en/default/attachment/edit.html.tmpl +++ b/template/en/default/attachment/edit.html.tmpl @@ -286,7 +286,19 @@ [% IF !attachment.datasize %] <td width="75%"><b>The content of this attachment has been deleted.</b></td> - [% ELSIF isviewable %] + [% ELSIF attachment.isurl %] + <td width="75%"> + <a href="[% attachment.data FILTER html %]"> + [% IF attachment.datasize < 120 %] + [% attachment.data FILTER html %] + [% ELSE %] + [% attachment.data FILTER truncate(80) FILTER html %] + ... + [% attachment.data.match(".*(.{20})$").0 FILTER html %] + [% END %] + </a> + </td> + [% ELSIF attachment.is_viewable %] <td width="75%"> [% INCLUDE global/textarea.html.tmpl id = 'editFrame' @@ -317,18 +329,6 @@ //--> </script> </td> - [% ELSIF attachment.isurl %] - <td width="75%"> - <a href="[% attachment.data FILTER html %]"> - [% IF attachment.datasize < 120 %] - [% attachment.data FILTER html %] - [% ELSE %] - [% attachment.data FILTER truncate(80) FILTER html %] - ... - [% attachment.data.match(".*(.{20})$").0 FILTER html %] - [% END %] - </a> - </td> [% ELSE %] <td id="noview" width="50%"> <p><b> diff --git a/template/en/default/attachment/show-multiple.html.tmpl b/template/en/default/attachment/show-multiple.html.tmpl index 9c0fc57b0..3d5b3b4e8 100644 --- a/template/en/default/attachment/show-multiple.html.tmpl +++ b/template/en/default/attachment/show-multiple.html.tmpl @@ -86,7 +86,7 @@ </tr> </table> - [% IF a.isviewable %] + [% IF a.is_viewable %] <iframe src="attachment.cgi?id=[% a.id %]" width="75%" height="350"> <b>You cannot view the attachment on this page because your browser does not support IFRAMEs. <a href="attachment.cgi?id=[% a.id %]">View the attachment on a separate page</a>.</b> |