From c068e2b90f06f898217dc9919371a583517d985e Mon Sep 17 00:00:00 2001 From: "jake%bugzilla.org" <> Date: Tue, 21 Dec 2004 17:04:00 +0000 Subject: Bug 264601 - Bugzilla will now tell a user which field contained an "invalid bug number or alias." Patch by Frédéric Buclin r=myk, a=justdave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CGI.pl | 8 ++-- attachment.cgi | 5 --- post_bug.cgi | 2 +- process_bug.cgi | 52 ++++++++---------------- template/en/default/global/field-descs.none.tmpl | 3 ++ template/en/default/global/user-error.html.tmpl | 28 ++++++------- 6 files changed, 39 insertions(+), 59 deletions(-) diff --git a/CGI.pl b/CGI.pl index 456022808..0d03562fe 100644 --- a/CGI.pl +++ b/CGI.pl @@ -148,7 +148,7 @@ sub ValidateBugID { # database, and that the user is authorized to access that bug. # We detaint the number here, too - my ($id, $skip_authorization) = @_; + my ($id, $field) = @_; # Get rid of white-space around the ID. $id = trim($id); @@ -157,7 +157,9 @@ sub ValidateBugID { my $alias = $id; if (!detaint_natural($id)) { $id = BugAliasToID($alias); - $id || ThrowUserError("invalid_bug_id_or_alias", {'bug_id' => $alias}); + $id || ThrowUserError("invalid_bug_id_or_alias", + {'bug_id' => $alias, + 'field' => $field }); } # Modify the calling code's original variable to contain the trimmed, @@ -170,7 +172,7 @@ sub ValidateBugID { FetchOneColumn() || ThrowUserError("invalid_bug_id_non_existent", {'bug_id' => $id}); - return if $skip_authorization; + return if ($field eq "dependson" || $field eq "blocked"); return if Bugzilla->user->can_see_bug($id); diff --git a/attachment.cgi b/attachment.cgi index 31dcd12e7..b486bf6db 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -1060,11 +1060,6 @@ sub update # Get the bug ID for the bug to which this attachment is attached. SendSQL("SELECT bug_id FROM attachments WHERE attach_id = $::FORM{'id'}"); my $bugid = FetchSQLData(); - unless ($bugid) - { - ThrowUserError("invalid_bug_id", - { bug_id => $bugid }); - } # Lock database tables in preparation for updating the attachment. SendSQL("LOCK TABLES attachments WRITE , flags WRITE , " . diff --git a/post_bug.cgi b/post_bug.cgi index 5fc50e66f..7282f8fa9 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -256,7 +256,7 @@ foreach my $field ("dependson", "blocked") { my @validvalues; foreach my $id (split(/[\s,]+/, $::FORM{$field})) { next unless $id; - ValidateBugID($id, 1); + ValidateBugID($id, $field); push(@validvalues, $id); } $::FORM{$field} = join(",", @validvalues); diff --git a/process_bug.cgi b/process_bug.cgi index 47957d193..6fb94711a 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -120,23 +120,13 @@ foreach my $field ("dependson", "blocked") { my @validvalues; foreach my $id (split(/[\s,]+/, $::FORM{$field})) { next unless $id; - ValidateBugID($id, 1); + ValidateBugID($id, $field); push(@validvalues, $id); } $::FORM{$field} = join(",", @validvalues); } } -# If we are duping bugs, let's also make sure that we can change -# the original. This takes care of issue A on bug 96085. -if (defined $::FORM{'dup_id'} && $::FORM{'knob'} eq "duplicate") { - ValidateBugID($::FORM{'dup_id'}); - - # Also, let's see if the reporter has authorization to see the bug - # to which we are duping. If not we need to prompt. - DuplicateUserConfirm(); -} - # do a match on the fields if applicable # The order of these function calls is important, as both Flag::validate @@ -490,8 +480,8 @@ sub DuplicateUserConfirm { return; } - my $dupe = trim($::FORM{'id'}); - my $original = trim($::FORM{'dup_id'}); + my $dupe = $::FORM{'id'}; + my $original = $::FORM{'dup_id'}; SendSQL("SELECT reporter FROM bugs WHERE bug_id = " . SqlQuote($dupe)); my $reporter = FetchOneColumn(); @@ -520,7 +510,7 @@ sub DuplicateUserConfirm { $template->process("bug/process/confirm-duplicate.html.tmpl", $vars) || ThrowTemplateError($template->error()); exit; -} # end DuplicateUserConfirm() +} if (defined $::FORM{'id'}) { # since this means that we were called from show_bug.cgi, now is a good @@ -976,28 +966,22 @@ SWITCH: for ($::FORM{'knob'}) { last SWITCH; }; /^duplicate$/ && CheckonComment( "duplicate" ) && do { - ChangeStatus('RESOLVED'); - ChangeResolution('DUPLICATE'); - CheckFormFieldDefined(\%::FORM,'dup_id'); - my $num = trim($::FORM{'dup_id'}); - SendSQL("SELECT bug_id FROM bugs WHERE bug_id = " . SqlQuote($num)); - $num = FetchOneColumn(); - if (!$num) { - ThrowUserError("dupe_invalid_bug_id") - } - if (!defined($::FORM{'id'}) || $num == $::FORM{'id'}) { + # Make sure we can change the original bug (issue A on bug 96085) + CheckFormFieldDefined(\%::FORM, 'dup_id'); + ValidateBugID($::FORM{'dup_id'}, 'dup_id'); + + # Also, let's see if the reporter has authorization to see + # the bug to which we are duping. If not we need to prompt. + DuplicateUserConfirm(); + + $duplicate = $::FORM{'dup_id'}; + if (!defined($::FORM{'id'}) || $duplicate == $::FORM{'id'}) { ThrowUserError("dupe_of_self_disallowed"); } - my $checkid = trim($::FORM{'id'}); - SendSQL("SELECT bug_id FROM bugs where bug_id = " . SqlQuote($checkid)); - $checkid = FetchOneColumn(); - if (!$checkid) { - ThrowUserError("invalid_bug_id", - { bug_id => $checkid }); - } - $::FORM{'comment'} .= "\n\n*** This bug has been marked as a duplicate of $num ***"; - $duplicate = $num; - + ChangeStatus('RESOLVED'); + ChangeResolution('DUPLICATE'); + $::FORM{'comment'} .= "\n\n*** This bug has been marked " . + "as a duplicate of $duplicate ***"; last SWITCH; }; diff --git a/template/en/default/global/field-descs.none.tmpl b/template/en/default/global/field-descs.none.tmpl index 1080e8e44..e49db6b5b 100644 --- a/template/en/default/global/field-descs.none.tmpl +++ b/template/en/default/global/field-descs.none.tmpl @@ -26,6 +26,7 @@ [% field_descs = { "[Bug creation]" => "[$terms.Bug creation]", "alias" => "Alias", "assigned_to" => "Assignee", + "blocked" => "Blocks", "bug_file_loc" => "URL", "bug_id" => "$terms.Bug ID", "bug_severity" => "Severity", @@ -38,6 +39,8 @@ "component" => "Component", "creation_ts" => "$terms.Bug Creation time", "delta_ts" => "Last Changed time", + "dependson" => "Depends on", + "dup_id" => "Duplicate", "estimated_time" => "Orig. Est.", "everconfirmed" => "Ever confirmed?", "groupset" => "Groupset", diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index affad39cc..ebd44906e 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -262,11 +262,6 @@ [% title = "Description Required" %] You must provide a description of the [% terms.bug %]. - [% ELSIF error == "dupe_invalid_bug_id" %] - [% title = BLOCK %]Valid [% terms.Bug %] Number Required[% END %] - You must specify a valid [% terms.bug %] number of which this - [%+ terms.bug %] is a duplicate. The [% terms.bug %] has not been changed. - [% ELSIF error == "dupe_of_self_disallowed" %] [% title = "Cannot mark $terms.abug as a duplicate of itself" %] You can't mark [% terms.abug %] as a duplicate of itself. @@ -468,19 +463,24 @@ [% title = "Invalid Attachment ID" %] The attachment id [% attach_id FILTER html %] is invalid. - [% ELSIF error == "invalid_bug_id" %] - [% title = BLOCK %]Invalid [% terms.Bug %] ID[% END %] - The [% terms.bug %] id [% bug_id FILTER html %] is invalid. - [% ELSIF error == "invalid_bug_id_non_existent" %] [% title = BLOCK %]Invalid [% terms.Bug %] ID[% END %] [% terms.Bug %] #[% bug_id FILTER html %] does not exist. [% ELSIF error == "invalid_bug_id_or_alias" %] [% title = BLOCK %]Invalid [% terms.Bug %] ID[% END %] - '[% bug_id FILTER html %]' is not a valid [% terms.bug %] number - [% IF Param("usebugaliases") %] nor an alias - to [% terms.abug %] number[% END %]. + [% IF bug_id %] + '[% bug_id FILTER html %]' is not a valid [% terms.bug %] number + [% IF Param("usebugaliases") %] + nor an alias to [% terms.abug %] number + [% END %]. + [% ELSE %] + [% IF field %] + The '[% field_descs.$field FILTER html %]' field + cannot be empty. + [% END %] + You must enter a valid [% terms.bug %] number! + [% END %]