diff options
| -rw-r--r-- | Bugzilla/Bug.pm | 35 | ||||
| -rw-r--r-- | Bugzilla/Search/Quicksearch.pm | 7 | ||||
| -rwxr-xr-x | show_bug.cgi | 24 | ||||
| -rw-r--r-- | template/en/default/bug/comments.html.tmpl | 3 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 8 |
5 files changed, 44 insertions, 33 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index eae23d995..2e9ed52e0 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -358,12 +358,14 @@ sub check { # Bugzilla::Bug throws lots of special errors, so we don't call # SUPER::check, we just call our new and do our own checks. - my $self = $class->new(trim($id)); - # For error messages, use the id that was returned by new(), because - # it's cleaned up. - $id = $self->id; + $id = trim($id); + my $self = $class->new($id); if ($self->{error}) { + # For error messages, use the id that was returned by new(), because + # it's cleaned up. + $id = $self->id; + if ($self->{error} eq 'NotFound') { ThrowUserError("bug_id_does_not_exist", { bug_id => $id }); } @@ -375,7 +377,7 @@ sub check { } unless ($field && $field =~ /^(dependson|blocked|dup_id)$/) { - $self->check_is_visible; + $self->check_is_visible($id); } return $self; } @@ -391,16 +393,19 @@ sub check_for_edit { } sub check_is_visible { - my $self = shift; + my ($self, $input_id) = @_; + $input_id ||= $self->id; my $user = Bugzilla->user; if (!$user->can_see_bug($self->id)) { # The error the user sees depends on whether or not they are # logged in (i.e. $user->id contains the user's positive integer ID). + # If we are validating an alias, then use it in the error message + # instead of its corresponding bug ID, to not disclose it. if ($user->id) { - ThrowUserError("bug_access_denied", { bug_id => $self->id }); + ThrowUserError("bug_access_denied", { bug_id => $input_id }); } else { - ThrowUserError("bug_access_query", { bug_id => $self->id }); + ThrowUserError("bug_access_query", { bug_id => $input_id }); } } } @@ -1471,9 +1476,7 @@ sub _check_dependencies { : split(/[\s,]+/, $deps_in{$type}); # Eliminate nulls. @bug_ids = grep {$_} @bug_ids; - # We do this up here to make sure all aliases are converted to IDs. - @bug_ids = map { $invocant->check($_, $type)->id } @bug_ids; - + my @check_access = @bug_ids; # When we're updating a bug, only added or removed bug_ids are # checked for whether or not we can see/edit those bugs. @@ -1503,7 +1506,8 @@ sub _check_dependencies { } } } - + # Replace all aliases by their corresponding bug ID. + @bug_ids = map { $_ =~ /^(\d+)$/ ? $1 : $invocant->check($_, $type)->id } @bug_ids; $deps_in{$type} = \@bug_ids; } @@ -1520,8 +1524,9 @@ sub _check_dependencies { sub _check_dup_id { my ($self, $dupe_of) = @_; my $dbh = Bugzilla->dbh; - - $dupe_of = trim($dupe_of); + + # Store the bug ID/alias passed by the user for visibility checks. + my $orig_dupe_of = $dupe_of = trim($dupe_of); $dupe_of || ThrowCodeError('undefined_field', { field => 'dup_id' }); # Validate the bug ID. The second argument will force check() to only # make sure that the bug exists, and convert the alias to the bug ID @@ -1534,7 +1539,7 @@ sub _check_dup_id { # If we come here, then the duplicate is new. We have to make sure # that we can view/change it (issue A on bug 96085). - $dupe_of_bug->check_is_visible; + $dupe_of_bug->check_is_visible($orig_dupe_of); # Make sure a loop isn't created when marking this bug # as duplicate. diff --git a/Bugzilla/Search/Quicksearch.pm b/Bugzilla/Search/Quicksearch.pm index 10f3f768b..17c5635ff 100644 --- a/Bugzilla/Search/Quicksearch.pm +++ b/Bugzilla/Search/Quicksearch.pm @@ -285,9 +285,10 @@ sub _handle_alias { if ($searchstring =~ /^([^,\s]+)$/) { my $alias = $1; # We use this direct SQL because we want quicksearch to be VERY fast. - my $is_alias = Bugzilla->dbh->selectrow_array( - q{SELECT 1 FROM bugs WHERE alias = ?}, undef, $alias); - if ($is_alias) { + my $bug_id = Bugzilla->dbh->selectrow_array( + q{SELECT bug_id FROM bugs WHERE alias = ?}, undef, $alias); + # If the user cannot see the bug, do not resolve its alias. + if ($bug_id && Bugzilla->user->can_see_bug($bug_id)) { $alias = url_quote($alias); print Bugzilla->cgi->redirect( -uri => correct_urlbase() . "show_bug.cgi?id=$alias"); diff --git a/show_bug.cgi b/show_bug.cgi index 0da4c4842..48d75512b 100755 --- a/show_bug.cgi +++ b/show_bug.cgi @@ -13,8 +13,7 @@ use lib qw(. lib); use Bugzilla; use Bugzilla::Constants; use Bugzilla::Error; -use Bugzilla::User; -use Bugzilla::Keyword; +use Bugzilla::Util; use Bugzilla::Bug; my $cgi = Bugzilla->cgi; @@ -38,7 +37,7 @@ if (!$cgi->param('id') && $single) { my $format = $template->get_format("bug/show", scalar $cgi->param('format'), scalar $cgi->param('ctype')); -my @bugs; +my (@bugs, @illegal_bugs); my %marks; # If the user isn't logged in, we use data from the shadow DB. If he plans @@ -64,22 +63,23 @@ if ($single) { foreach my $id ($cgi->param('id')) { # Be kind enough and accept URLs of the form: id=1,2,3. my @ids = split(/,/, $id); - foreach (@ids) { - my $bug = new Bugzilla::Bug($_); - # This is basically a backwards-compatibility hack from when - # Bugzilla::Bug->new used to set 'NotPermitted' if you couldn't - # see the bug. - if (!$bug->{error} && !$user->can_see_bug($bug->bug_id)) { - $bug->{error} = 'NotPermitted'; + foreach my $bug_id (@ids) { + next unless $bug_id; + my $bug = new Bugzilla::Bug($bug_id); + if (!$bug->{error} && $user->can_see_bug($bug->bug_id)) { + push(@bugs, $bug); + } + else { + push(@illegal_bugs, { bug_id => trim($bug_id), + error => $bug->{error} || 'NotPermitted' }); } - push(@bugs, $bug); } } } Bugzilla::Bug->preload(\@bugs); -$vars->{'bugs'} = \@bugs; +$vars->{'bugs'} = [@bugs, @illegal_bugs]; $vars->{'marks'} = \%marks; my @bugids = map {$_->bug_id} grep {!$_->error} @bugs; diff --git a/template/en/default/bug/comments.html.tmpl b/template/en/default/bug/comments.html.tmpl index 4a433230b..62beb61ea 100644 --- a/template/en/default/bug/comments.html.tmpl +++ b/template/en/default/bug/comments.html.tmpl @@ -85,7 +85,8 @@ [% count = count + increment %] [% END %] -[% IF user.settings.comment_box_position.value == "before_comments" && user.id %] +[% IF mode == "edit" && user.id + && user.settings.comment_box_position.value == "before_comments" %] <div class="bz_add_comment"> <a href="#" onclick="return goto_add_comments();"> diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index fbb9ca169..8f4d7d21c 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -77,9 +77,13 @@ [% ELSIF error == "alias_in_use" %] [% title = "Alias In Use" %] - [% terms.Bug %] [%+ bug_id FILTER bug_link(bug_id) FILTER none %] + [% IF user.can_see_bug(bug_id) %] + [% terms.Bug %] [%+ bug_id FILTER bug_link(bug_id) FILTER none %] + [% ELSE %] + Another [% terms.bug %] + [% END %] has already taken the alias <em>[% alias FILTER html %]</em>. - Please choose another one. + Please choose another alias. [% ELSIF error == "alias_is_numeric" %] [% title = "Alias Is Numeric" %] |