summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rwxr-xr-xBugzilla/Bug.pm72
-rwxr-xr-xprocess_bug.cgi46
2 files changed, 62 insertions, 56 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 62be49564..e523621e0 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -411,7 +411,8 @@ sub run_create_validators {
$params->{assigned_to}, $params->{qa_contact});
($params->{dependson}, $params->{blocked}) =
- $class->_check_dependencies($product, $params->{dependson}, $params->{blocked});
+ $class->_check_dependencies($params->{dependson}, $params->{blocked},
+ $product);
# You can't set these fields on bug creation (or sometimes ever).
delete $params->{resolution};
@@ -701,28 +702,59 @@ sub _check_deadline {
# Takes two comma/space-separated strings and returns arrayrefs
# of valid bug IDs.
sub _check_dependencies {
- my ($invocant, $product, $depends_on, $blocks) = @_;
+ my ($invocant, $depends_on, $blocks, $product) = @_;
- # Only editbugs users can set dependencies on bug entry.
- return ([], []) unless Bugzilla->user->in_group('editbugs', $product->id);
-
- $depends_on ||= '';
- $blocks ||= '';
+ if (!ref $invocant) {
+ # Only editbugs users can set dependencies on bug entry.
+ return ([], []) unless Bugzilla->user->in_group('editbugs',
+ $product->id);
+ }
+
+ my %deps_in = (dependson => $depends_on || '', blocked => $blocks || '');
+
+ foreach my $type qw(dependson blocked) {
+ my @bug_ids = split(/[\s,]+/, $deps_in{$type});
+ # Eliminate nulls.
+ @bug_ids = grep {$_} @bug_ids;
+ # We do Validate up here to make sure all aliases are converted to IDs.
+ ValidateBugID($_, $type) foreach @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.
+ if (ref $invocant) {
+ my $old = $invocant->$type;
+ my ($removed, $added) = diff_arrays($old, \@bug_ids);
+ @check_access = (@$added, @$removed);
+
+ # Check field permissions if we've changed anything.
+ if (@check_access) {
+ my $privs;
+ if (!$invocant->check_can_change_field($type, 0, 1, \$privs)) {
+ ThrowUserError('illegal_change', { field => $type,
+ privs => $privs });
+ }
+ }
+ }
- # Make sure all the bug_ids are valid.
- my @results;
- foreach my $string ($depends_on, $blocks) {
- my @array = split(/[\s,]+/, $string);
- # Eliminate nulls
- @array = grep($_, @array);
- # $field is not passed to ValidateBugID to prevent adding new
- # dependencies on inaccessible bugs.
- ValidateBugID($_) foreach (@array);
- push(@results, \@array);
+ my $user = Bugzilla->user;
+ foreach my $modified_id (@check_access) {
+ ValidateBugID($modified_id);
+ # Under strict isolation, you can't modify a bug if you can't
+ # edit it, even if you can see it.
+ if (Bugzilla->params->{"strict_isolation"}) {
+ my $delta_bug = new Bugzilla::Bug($modified_id);
+ if (!$user->can_edit_product($delta_bug->{'product_id'})) {
+ ThrowUserError("illegal_change_deps", {field => $type});
+ }
+ }
+ }
+
+ $deps_in{$type} = \@bug_ids;
}
-
- # dependson blocks
- my %deps = ValidateDependencies($results[0], $results[1]);
+
+ # And finally, check for dependency loops.
+ my %deps = ValidateDependencies($deps_in{dependson}, $deps_in{blocked});
return ($deps{'dependson'}, $deps{'blocked'});
}
diff --git a/process_bug.cgi b/process_bug.cgi
index 8be6cb48f..a28efa02f 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -185,42 +185,16 @@ $cgi->param('comment', $bug->_check_comment($cgi->param('comment')));
# instead of throwing an error, f.e. when one or more of the values
# is a bug alias that gets converted to its corresponding bug ID
# during validation.
-foreach my $field ("dependson", "blocked") {
- if ($cgi->param('id')) {
- my @old = @{$bug->$field};
- my @new;
- foreach my $id (split(/[\s,]+/, $cgi->param($field))) {
- next unless $id;
- ValidateBugID($id, $field);
- push @new, $id;
- }
- $cgi->param($field, join(",", @new));
- my ($added, $removed) = Bugzilla::Util::diff_arrays(\@old, \@new);
- foreach my $id (@$added , @$removed) {
- # ValidateBugID is called without $field here so that it will
- # throw an error if any of the changed bugs are not visible.
- ValidateBugID($id);
- if (Bugzilla->params->{"strict_isolation"}) {
- my $deltabug = new Bugzilla::Bug($id);
- if (!$user->can_edit_product($deltabug->{'product_id'})) {
- $vars->{'field'} = $field;
- ThrowUserError("illegal_change_deps", $vars);
- }
- }
- }
- if ((@$added || @$removed)
- && !$bug->check_can_change_field($field, 0, 1, \$PrivilegesRequired))
- {
- $vars->{'privs'} = $PrivilegesRequired;
- $vars->{'field'} = $field;
- ThrowUserError("illegal_change", $vars);
- }
- } else {
- # Bugzilla does not support mass-change of dependencies so they
- # are not validated. To prevent a URL-hacking risk, the dependencies
- # are deleted for mass-changes.
- $cgi->delete($field);
- }
+if ($cgi->param('id')) {
+ my ($dependson, $blocks) = $bug->_check_dependencies(
+ scalar $cgi->param('dependson'), scalar $cgi->param('blocked'));
+ $cgi->param('dependson', $dependson);
+ $cgi->param('blocked', $blocks);
+}
+# Right now, you can't modify dependencies on a mass change.
+else {
+ $cgi->delete('dependson');
+ $cgi->delete('blocked');
}
# do a match on the fields if applicable