Skip to content

bugzilla
bugzilla
Commit cc69d134 authored by mkanat%bugzilla.org's avatar mkanat%bugzilla.org
Browse files
Options

Bug 348057: Move the checks for bug visibility out of Bugzilla::Bug->new

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=myk
parent b7358c82
Hide whitespace changes
Inline Side-by-side
Showing with 47 additions and 69 deletions
Bugzilla/Bug.pm
View file @ cc69d134
...@@ -65,36 +65,18 @@ use constant MAX_COMMENT_LENGTH => 65535; ...@@ -65,36 +65,18 @@ use constant MAX_COMMENT_LENGTH => 65535;
##################################################################### #####################################################################
# create a new empty bug
#
sub new { sub new {
my $type = shift(); my $invocant = shift;
my %bug; my $class = ref($invocant) || $invocant;
my $self = {};
# create a ref to an empty hash and bless it bless $self, $class;
# $self->_init(@_);
my $self = {%bug};
bless $self, $type;
# construct from a hash containing a bug's info
#
if ($#_ == 1) {
$self->initBug(@_);
} else {
confess("invalid number of arguments \($#_\)($_)");
}
# bless as a Bug
#
return $self; return $self;
} }
# dump info about bug into hash unless user doesn't have permission sub _init {
# user_id 0 is used when person is not logged in.
#
sub initBug {
my $self = shift(); my $self = shift();
my ($bug_id, $user_id) = (@_); my ($bug_id) = (@_);
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
$bug_id = trim($bug_id || 0); $bug_id = trim($bug_id || 0);
...@@ -104,18 +86,6 @@ sub initBug { ...@@ -104,18 +86,6 @@ sub initBug {
# If the bug ID isn't numeric, it might be an alias, so try to convert it. # If the bug ID isn't numeric, it might be an alias, so try to convert it.
$bug_id = bug_alias_to_id($bug_id) if $bug_id !~ /^0*[1-9][0-9]*$/; $bug_id = bug_alias_to_id($bug_id) if $bug_id !~ /^0*[1-9][0-9]*$/;
# If the user is not logged in, sets $user_id to 0.
# Else gets $user_id from the user login name if this
# argument is not numeric.
my $stored_user_id = $user_id;
if (!defined $user_id) {
$user_id = 0;
} elsif (!detaint_natural($user_id)) {
$user_id = login_to_id($stored_user_id);
}
$self->{'who'} = new Bugzilla::User($user_id);
unless ($bug_id && detaint_natural($bug_id)) { unless ($bug_id && detaint_natural($bug_id)) {
# no bug number given or the alias didn't match a bug # no bug number given or the alias didn't match a bug
$self->{'bug_id'} = $old_bug_id; $self->{'bug_id'} = $old_bug_id;
...@@ -166,8 +136,7 @@ sub initBug { ...@@ -166,8 +136,7 @@ sub initBug {
$bug_sth->execute($bug_id); $bug_sth->execute($bug_id);
my @row; my @row;
if ((@row = $bug_sth->fetchrow_array()) if (@row = $bug_sth->fetchrow_array) {
&& $self->{'who'}->can_see_bug($bug_id)) {
my $count = 0; my $count = 0;
my %fields; my %fields;
foreach my $field ("bug_id", "alias", "classification_id", "classification", foreach my $field ("bug_id", "alias", "classification_id", "classification",
...@@ -188,10 +157,6 @@ sub initBug { ...@@ -188,10 +157,6 @@ sub initBug {
} }
$count++; $count++;
} }
} elsif (@row) {
$self->{'bug_id'} = $bug_id;
$self->{'error'} = "NotPermitted";
return $self;
} else { } else {
$self->{'bug_id'} = $bug_id; $self->{'bug_id'} = $bug_id;
$self->{'error'} = "NotFound"; $self->{'error'} = "NotFound";
...@@ -1186,7 +1151,7 @@ sub CheckIfVotedConfirmed { ...@@ -1186,7 +1151,7 @@ sub CheckIfVotedConfirmed {
sub check_can_change_field { sub check_can_change_field {
my $self = shift; my $self = shift;
my ($field, $oldvalue, $newvalue, $PrivilegesRequired, $data) = (@_); my ($field, $oldvalue, $newvalue, $PrivilegesRequired, $data) = (@_);
my $user = $self->{'who'}; my $user = Bugzilla->user;
$oldvalue = defined($oldvalue) ? $oldvalue : ''; $oldvalue = defined($oldvalue) ? $oldvalue : '';
$newvalue = defined($newvalue) ? $newvalue : ''; $newvalue = defined($newvalue) ? $newvalue : '';
......
attachment.cgi
View file @ cc69d134
...@@ -501,7 +501,7 @@ sub insert ...@@ -501,7 +501,7 @@ sub insert
ValidateComment(scalar $cgi->param('comment')); ValidateComment(scalar $cgi->param('comment'));
my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()"); my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
my $bug = new Bugzilla::Bug($bugid, $user->id); my $bug = new Bugzilla::Bug($bugid);
my $attachid = my $attachid =
Bugzilla::Attachment->insert_attachment_for_bug(THROW_ERROR, $bug, $user, Bugzilla::Attachment->insert_attachment_for_bug(THROW_ERROR, $bug, $user,
$timestamp, \$vars); $timestamp, \$vars);
...@@ -652,7 +652,7 @@ sub update ...@@ -652,7 +652,7 @@ sub update
Bugzilla::Flag::validate($cgi, $bugid, $attach_id); Bugzilla::Flag::validate($cgi, $bugid, $attach_id);
Bugzilla::FlagType::validate($cgi, $bugid, $attach_id); Bugzilla::FlagType::validate($cgi, $bugid, $attach_id);
my $bug = new Bugzilla::Bug($bugid, $userid); my $bug = new Bugzilla::Bug($bugid);
# 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' ,
'flagtypes READ', 'fielddefs READ', 'bugs_activity WRITE', 'flagtypes READ', 'fielddefs READ', 'bugs_activity WRITE',
......
editcomponents.cgi
View file @ cc69d134
...@@ -252,7 +252,9 @@ if ($action eq 'delete') { ...@@ -252,7 +252,9 @@ if ($action eq 'delete') {
if ($component->bug_count) { if ($component->bug_count) {
if (Bugzilla->params->{"allowbugdeletion"}) { if (Bugzilla->params->{"allowbugdeletion"}) {
foreach my $bug_id (@{$component->bug_ids}) { foreach my $bug_id (@{$component->bug_ids}) {
my $bug = new Bugzilla::Bug($bug_id, $whoid); # Note: We allow admins to delete bugs even if they can't
# see them, as long as they can see the product.
my $bug = new Bugzilla::Bug($bug_id);
$bug->remove_from_db(); $bug->remove_from_db();
} }
} else { } else {
......
editflagtypes.cgi
View file @ cc69d134
...@@ -393,7 +393,7 @@ sub update { ...@@ -393,7 +393,7 @@ sub update {
undef, $id); undef, $id);
foreach my $flag (@$flags) { foreach my $flag (@$flags) {
my ($flag_id, $bug_id, $attach_id) = @$flag; my ($flag_id, $bug_id, $attach_id) = @$flag;
my $bug = new Bugzilla::Bug($bug_id, $user->id); my $bug = new Bugzilla::Bug($bug_id);
my $attachment = $attach_id ? Bugzilla::Attachment->get($attach_id) : undef; my $attachment = $attach_id ? Bugzilla::Attachment->get($attach_id) : undef;
Bugzilla::Flag::clear($flag_id, $bug, $attachment); Bugzilla::Flag::clear($flag_id, $bug, $attachment);
} }
...@@ -412,7 +412,7 @@ sub update { ...@@ -412,7 +412,7 @@ sub update {
undef, $id); undef, $id);
foreach my $flag (@$flags) { foreach my $flag (@$flags) {
my ($flag_id, $bug_id, $attach_id) = @$flag; my ($flag_id, $bug_id, $attach_id) = @$flag;
my $bug = new Bugzilla::Bug($bug_id, $user->id); my $bug = new Bugzilla::Bug($bug_id);
my $attachment = $attach_id ? Bugzilla::Attachment->get($attach_id) : undef; my $attachment = $attach_id ? Bugzilla::Attachment->get($attach_id) : undef;
Bugzilla::Flag::clear($flag_id, $bug, $attachment); Bugzilla::Flag::clear($flag_id, $bug, $attachment);
} }
...@@ -680,4 +680,4 @@ sub filter_group { ...@@ -680,4 +680,4 @@ sub filter_group {
|| ($_->request_group && $_->request_group->id == $gid)} @$flag_types; || ($_->request_group && $_->request_group->id == $gid)} @$flag_types;
return \@flag_types; return \@flag_types;
} }
\ No newline at end of file
editproducts.cgi
View file @ cc69d134
...@@ -373,7 +373,9 @@ if ($action eq 'delete') { ...@@ -373,7 +373,9 @@ if ($action eq 'delete') {
if ($product->bug_count) { if ($product->bug_count) {
if (Bugzilla->params->{"allowbugdeletion"}) { if (Bugzilla->params->{"allowbugdeletion"}) {
foreach my $bug_id (@{$product->bug_ids}) { foreach my $bug_id (@{$product->bug_ids}) {
my $bug = new Bugzilla::Bug($bug_id, $whoid); # Note that we allow the user to delete bugs he can't see,
# which is okay, because he's deleting the whole Product.
my $bug = new Bugzilla::Bug($bug_id);
$bug->remove_from_db(); $bug->remove_from_db();
} }
} }
......
enter_bug.cgi
View file @ cc69d134
...@@ -307,7 +307,7 @@ $cloned_bug_id = $cgi->param('cloned_bug_id'); ...@@ -307,7 +307,7 @@ $cloned_bug_id = $cgi->param('cloned_bug_id');
if ($cloned_bug_id) { if ($cloned_bug_id) {
ValidateBugID($cloned_bug_id); ValidateBugID($cloned_bug_id);
$cloned_bug = new Bugzilla::Bug($cloned_bug_id, $user->id); $cloned_bug = new Bugzilla::Bug($cloned_bug_id);
} }
if (scalar(@{$product->components}) == 1) { if (scalar(@{$product->components}) == 1) {
......
importxml.pl
View file @ cc69d134
...@@ -224,7 +224,7 @@ sub flag_handler { ...@@ -224,7 +224,7 @@ sub flag_handler {
} ); } );
} }
else { else {
my $bug = new Bugzilla::Bug( $bugid, $exporterid ); my $bug = new Bugzilla::Bug($bugid);
$flag_types = $bug->flag_types; $flag_types = $bug->flag_types;
} }
unless ($flag_types){ unless ($flag_types){
......
post_bug.cgi
View file @ cc69d134
...@@ -537,7 +537,10 @@ $dbh->do("UPDATE bugs SET creation_ts = ? WHERE bug_id = ?", ...@@ -537,7 +537,10 @@ $dbh->do("UPDATE bugs SET creation_ts = ? WHERE bug_id = ?",
$dbh->bz_unlock_tables(); $dbh->bz_unlock_tables();
my $bug = new Bugzilla::Bug($id, $user->id); my $bug = new Bugzilla::Bug($id);
# We don't have to check if the user can see the bug, because a user filing
# a bug can always see it. You can't change reporter_accessible until
# after the bug is filed.
# 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')) {
......
process_bug.cgi
View file @ cc69d134
...@@ -141,7 +141,7 @@ scalar(@idlist) || ThrowUserError("no_bugs_chosen"); ...@@ -141,7 +141,7 @@ scalar(@idlist) || ThrowUserError("no_bugs_chosen");
# Build a bug object using $cgi->param('id') as ID. # Build a bug object using $cgi->param('id') as ID.
# If there are more than one bug changed at once, the bug object will be # If there are more than one bug changed at once, the bug object will be
# empty, which doesn't matter. # empty, which doesn't matter.
my $bug = new Bugzilla::Bug(scalar $cgi->param('id'), $whoid); my $bug = new Bugzilla::Bug(scalar $cgi->param('id'));
# Make sure form param 'dontchange' is defined so it can be compared to easily. # Make sure form param 'dontchange' is defined so it can be compared to easily.
$cgi->param('dontchange','') unless defined $cgi->param('dontchange'); $cgi->param('dontchange','') unless defined $cgi->param('dontchange');
...@@ -190,7 +190,7 @@ foreach my $field ("dependson", "blocked") { ...@@ -190,7 +190,7 @@ foreach my $field ("dependson", "blocked") {
# throw an error if any of the changed bugs are not visible. # throw an error if any of the changed bugs are not visible.
ValidateBugID($id); ValidateBugID($id);
if (Bugzilla->params->{"strict_isolation"}) { if (Bugzilla->params->{"strict_isolation"}) {
my $deltabug = new Bugzilla::Bug($id, $user->id); my $deltabug = new Bugzilla::Bug($id);
if (!$user->can_edit_product($deltabug->{'product_id'})) { if (!$user->can_edit_product($deltabug->{'product_id'})) {
$vars->{'field'} = $field; $vars->{'field'} = $field;
ThrowUserError("illegal_change_deps", $vars); ThrowUserError("illegal_change_deps", $vars);
...@@ -527,7 +527,7 @@ if ($action eq Bugzilla->params->{'move-button-text'}) { ...@@ -527,7 +527,7 @@ if ($action eq Bugzilla->params->{'move-button-text'}) {
my @bugs; my @bugs;
# First update all moved bugs. # First update all moved bugs.
foreach my $id (@idlist) { foreach my $id (@idlist) {
my $bug = new Bugzilla::Bug($id, $whoid); my $bug = new Bugzilla::Bug($id);
push(@bugs, $bug); push(@bugs, $bug);
$sth->execute($timestamp, $id); $sth->execute($timestamp, $id);
...@@ -1346,7 +1346,7 @@ if ($prod_changed && Bugzilla->params->{"strict_isolation"}) { ...@@ -1346,7 +1346,7 @@ if ($prod_changed && Bugzilla->params->{"strict_isolation"}) {
foreach my $id (@idlist) { foreach my $id (@idlist) {
my $query = $basequery; my $query = $basequery;
my @bug_values = @values; my @bug_values = @values;
my $old_bug_obj = new Bugzilla::Bug($id, $whoid); my $old_bug_obj = new Bugzilla::Bug($id);
if ($cgi->param('knob') eq 'reassignbycomponent') { if ($cgi->param('knob') eq 'reassignbycomponent') {
# We have to check whether the bug is moved to another product # We have to check whether the bug is moved to another product
...@@ -1897,7 +1897,7 @@ foreach my $id (@idlist) { ...@@ -1897,7 +1897,7 @@ foreach my $id (@idlist) {
# and then generate any necessary bug activity entries by seeing # and then generate any necessary bug activity entries by seeing
# what has changed since before we wrote out the new values. # what has changed since before we wrote out the new values.
# #
my $new_bug_obj = new Bugzilla::Bug($id, $whoid); my $new_bug_obj = new Bugzilla::Bug($id);
my @newvalues = SnapShotBug($id); my @newvalues = SnapShotBug($id);
my %newhash; my %newhash;
$i = 0; $i = 0;
...@@ -2096,7 +2096,7 @@ if ($action eq 'next_bug') { ...@@ -2096,7 +2096,7 @@ if ($action eq 'next_bug') {
} }
if ($next_bug) { if ($next_bug) {
if (detaint_natural($next_bug) && Bugzilla->user->can_see_bug($next_bug)) { if (detaint_natural($next_bug) && Bugzilla->user->can_see_bug($next_bug)) {
my $bug = new Bugzilla::Bug($next_bug, $whoid); my $bug = new Bugzilla::Bug($next_bug);
ThrowCodeError("bug_error", { bug => $bug }) if $bug->error; ThrowCodeError("bug_error", { bug => $bug }) if $bug->error;
$vars->{'bugs'} = [$bug]; $vars->{'bugs'} = [$bug];
...@@ -2110,7 +2110,7 @@ if ($action eq 'next_bug') { ...@@ -2110,7 +2110,7 @@ if ($action eq 'next_bug') {
} }
} elsif ($action eq 'same_bug') { } elsif ($action eq 'same_bug') {
if (Bugzilla->user->can_see_bug($cgi->param('id'))) { if (Bugzilla->user->can_see_bug($cgi->param('id'))) {
my $bug = new Bugzilla::Bug($cgi->param('id'), $whoid); my $bug = new Bugzilla::Bug($cgi->param('id'));
ThrowCodeError("bug_error", { bug => $bug }) if $bug->error; ThrowCodeError("bug_error", { bug => $bug }) if $bug->error;
$vars->{'bugs'} = [$bug]; $vars->{'bugs'} = [$bug];
......
show_bug.cgi
View file @ cc69d134
...@@ -35,7 +35,7 @@ my $cgi = Bugzilla->cgi; ...@@ -35,7 +35,7 @@ my $cgi = Bugzilla->cgi;
my $template = Bugzilla->template; my $template = Bugzilla->template;
my $vars = {}; my $vars = {};
Bugzilla->login(); my $user = Bugzilla->login();
# Editable, 'single' HTML bugs are treated slightly specially in a few places # Editable, 'single' HTML bugs are treated slightly specially in a few places
my $single = !$cgi->param('format') my $single = !$cgi->param('format')
...@@ -60,7 +60,7 @@ if ($single) { ...@@ -60,7 +60,7 @@ if ($single) {
# Its a bit silly to do the validation twice - that functionality should # Its a bit silly to do the validation twice - that functionality should
# probably move into Bug.pm at some point # probably move into Bug.pm at some point
ValidateBugID($id); ValidateBugID($id);
push @bugs, new Bugzilla::Bug($id, Bugzilla->user->id); push @bugs, new Bugzilla::Bug($id);
if (defined $cgi->param('mark')) { if (defined $cgi->param('mark')) {
foreach my $range (split ',', $cgi->param('mark')) { foreach my $range (split ',', $cgi->param('mark')) {
if ($range =~ /^(\d+)-(\d+)$/) { if ($range =~ /^(\d+)-(\d+)$/) {
...@@ -77,7 +77,13 @@ if ($single) { ...@@ -77,7 +77,13 @@ if ($single) {
# Be kind enough and accept URLs of the form: id=1,2,3. # Be kind enough and accept URLs of the form: id=1,2,3.
my @ids = split(/,/, $id); my @ids = split(/,/, $id);
foreach (@ids) { foreach (@ids) {
my $bug = new Bugzilla::Bug($_, Bugzilla->user->id); 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';
}
push(@bugs, $bug); push(@bugs, $bug);
} }
} }
......
showdependencytree.cgi
View file @ cc69d134
...@@ -51,7 +51,7 @@ my $dbh = Bugzilla->switch_to_shadow_db(); ...@@ -51,7 +51,7 @@ my $dbh = Bugzilla->switch_to_shadow_db();
# bug that the user is authorized to access. # bug that the user is authorized to access.
my $id = $cgi->param('id'); my $id = $cgi->param('id');
ValidateBugID($id); ValidateBugID($id);
my $current_bug = new Bugzilla::Bug($id, $user->id); my $current_bug = new Bugzilla::Bug($id);
my $hide_resolved = $cgi->param('hide_resolved') ? 1 : 0; my $hide_resolved = $cgi->param('hide_resolved') ? 1 : 0;
...@@ -120,7 +120,7 @@ sub GenerateTree { ...@@ -120,7 +120,7 @@ sub GenerateTree {
# its sub-tree if we haven't already done so (which happens # its sub-tree if we haven't already done so (which happens
# when bugs appear in dependency trees multiple times). # when bugs appear in dependency trees multiple times).
if (!$bugs->{$dep_id}) { if (!$bugs->{$dep_id}) {
$bugs->{$dep_id} = new Bugzilla::Bug($dep_id, $user->id); $bugs->{$dep_id} = new Bugzilla::Bug($dep_id);
GenerateTree($dep_id, $relationship, $depth+1, $bugs, $ids); GenerateTree($dep_id, $relationship, $depth+1, $bugs, $ids);
} }
...@@ -129,6 +129,7 @@ sub GenerateTree { ...@@ -129,6 +129,7 @@ sub GenerateTree {
# wants the tree to go, and if the dependency isn't resolved # wants the tree to go, and if the dependency isn't resolved
# (if we're ignoring resolved dependencies). # (if we're ignoring resolved dependencies).
if (!$bugs->{$dep_id}->{'error'} if (!$bugs->{$dep_id}->{'error'}
&& Bugzilla->user->can_see_bug($dep_id)
&& (!$maxdepth || $depth <= $maxdepth) && (!$maxdepth || $depth <= $maxdepth)
&& ($bugs->{$dep_id}->{'isopened'} || !$hide_resolved)) && ($bugs->{$dep_id}->{'isopened'} || !$hide_resolved))
{ {
......
template/en/default/bug/show-multiple.html.tmpl
View file @ cc69d134
...@@ -54,7 +54,7 @@ ...@@ -54,7 +54,7 @@
<h1> <h1>
[% terms.Bug %] [% terms.Bug %]
<a href="show_bug.cgi?id=[% bug.bug_id %]">[% bug.bug_id %]</a> <a href="show_bug.cgi?id=[% bug.bug_id %]">[% bug.bug_id %]</a>
[% IF Param("usebugaliases") AND bug.alias %] [% IF Param("usebugaliases") AND bug.alias AND NOT bug.error %]
([% bug.alias FILTER html %]) ([% bug.alias FILTER html %])
[% END %] [% END %]
</h1> </h1>
......
votes.cgi
View file @ cc69d134
...@@ -266,7 +266,7 @@ sub record_votes { ...@@ -266,7 +266,7 @@ sub record_votes {
my %products; my %products;
# XXX - We really need a $bug->product() method. # XXX - We really need a $bug->product() method.
foreach my $bug_id (@buglist) { foreach my $bug_id (@buglist) {
my $bug = new Bugzilla::Bug($bug_id, $who); my $bug = new Bugzilla::Bug($bug_id);
my $prod = $bug->{'product'}; my $prod = $bug->{'product'};
$products{$prod} ||= new Bugzilla::Product({name => $prod}); $products{$prod} ||= new Bugzilla::Product({name => $prod});
$prodcount{$prod} ||= 0; $prodcount{$prod} ||= 0;
......
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