From b8851cdd5c15e0d21543d9fe08159b9ced8c950f Mon Sep 17 00:00:00 2001 From: "gerv%gerv.net" <> Date: Sun, 6 Oct 2002 18:52:28 +0000 Subject: Bug 163114 - Templatise all calls to DisplayError. Patch D (the last one). Patch by gerv; r=burnus. --- Bugzilla/Search.pm | 7 +-- CGI.pl | 12 ----- describecomponents.cgi | 7 +-- globals.pl | 20 +++----- reports.cgi | 8 ++-- sanitycheck.cgi | 4 +- showdependencygraph.cgi | 3 +- sidebar.cgi | 3 +- template/en/default/global/code-error.html.tmpl | 13 ++++++ template/en/default/global/user-error.html.tmpl | 62 +++++++++++++++++++++++++ token.cgi | 6 +-- userprefs.cgi | 41 +++++----------- votes.cgi | 42 ++++++----------- 13 files changed, 119 insertions(+), 109 deletions(-) diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 6d11c0739..db97af3f2 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -746,12 +746,7 @@ sub init { # chart -1 is generated by other code above, not from the user- # submitted form, so we'll blindly accept any values in chart -1 if ((!$chartfields{$f}) && ($chart != -1)) { - my $errstr = "Can't use $f as a field name. " . - "If you think you're getting this in error, please copy the " . - "entire URL out of the address bar at the top of your browser " . - "window and email it to <109679\@bugzilla.org>"; - die "Internal error: $errstr" if $chart < 0; - return &::DisplayError($errstr); + ThrowCodeError("invalid_field_name", {field => $f}); } # This is either from the internal chart (in which case we diff --git a/CGI.pl b/CGI.pl index 6e121b273..3d6752a57 100644 --- a/CGI.pl +++ b/CGI.pl @@ -801,18 +801,6 @@ sub PutFooter { # ThrowUserError("some_tag", { bug_id => $bug_id, size => 127 }); ############################################################################### -# DisplayError is deprecated. Use ThrowCodeError, ThrowUserError or -# ThrowTemplateError instead. -sub DisplayError { - ($vars->{'error'}, $vars->{'title'}) = (@_); - - print "Content-type: text/html\n\n" if !$vars->{'header_done'}; - $template->process("global/user-error.html.tmpl", $vars) - || ThrowTemplateError($template->error()); - - return 1; -} - # For "this shouldn't happen"-type places in the code. # The contents of $extra_vars get printed out in the template - useful for # debugging info. diff --git a/describecomponents.cgi b/describecomponents.cgi index 7a24b692c..b4953ddc6 100755 --- a/describecomponents.cgi +++ b/describecomponents.cgi @@ -57,9 +57,7 @@ if (!defined $::FORM{'product'}) { my $prodsize = scalar(keys %products); if ($prodsize == 0) { - DisplayError("Either no products have been defined ". - "or you have not been given access to any.\n"); - exit; + ThrowUserError("no_products"); } elsif ($prodsize > 1) { $::vars->{'proddesc'} = \%products; @@ -93,8 +91,7 @@ if (!$product_id) { if (Param("usebuggroups") && GroupExists($product)) { confirm_login() unless $::userid; UserInGroup($product) - || DisplayError("You are not authorized to access that product.") - && exit; + || ThrowUserError("product_access_denied"); } ###################################################################### diff --git a/globals.pl b/globals.pl index 605549315..f1e8f0d74 100644 --- a/globals.pl +++ b/globals.pl @@ -680,24 +680,18 @@ sub CanSeeBug { sub ValidatePassword { # Determines whether or not a password is valid (i.e. meets Bugzilla's - # requirements for length and content). If the password is valid, the - # function returns boolean false. Otherwise it returns an error message - # (synonymous with boolean true) that can be displayed to the user. - + # requirements for length and content). # If a second password is passed in, this function also verifies that # the two passwords match. - my ($password, $matchpassword) = @_; - if ( length($password) < 3 ) { - return "The password is less than three characters long. It must be at least three characters."; - } elsif ( length($password) > 16 ) { - return "The password is more than 16 characters long. It must be no more than 16 characters."; - } elsif ( $matchpassword && $password ne $matchpassword ) { - return "The two passwords do not match."; + if (length($password) < 3) { + ThrowUserError("password_too_short"); + } elsif (length($password) > 16) { + ThrowUserError("password_too_long"); + } elsif ($matchpassword && $password ne $matchpassword) { + ThrowUserError("passwords_dont_match"); } - - return 0; } diff --git a/reports.cgi b/reports.cgi index 40a6786b4..546ac5990 100755 --- a/reports.cgi +++ b/reports.cgi @@ -105,23 +105,21 @@ if (! defined $FORM{'product'}) { # Valid values are those products for which the user has permissions which appear # in the "product" drop-down menu on the report generation form. grep($_ eq $FORM{'product'}, @myproducts) - || DisplayError("You entered an invalid product name.") && exit; + || ThrowUserError("invalid_product_name", {product => $FORM{'product'}}); # If usebuggroups is on, we don't want people to be able to view # reports for products they don't have permissions for... Param("usebuggroups") && GroupExists($FORM{'product'}) && !UserInGroup($FORM{'product'}) - && DisplayError("You do not have the permissions necessary to view reports for this product.") - && exit; + && ThrowUserError("report_access_denied"); # For security and correctness, validate the value of the "output" form variable. # Valid values are the keys from the %reports hash defined above which appear in # the "output" drop-down menu on the report generation form. $FORM{'output'} ||= "most_doomed"; # a reasonable default grep($_ eq $FORM{'output'}, keys %reports) - || DisplayError("You entered an invalid output type.") - && exit; + || ThrowCodeError("invalid_output_type", {type => $FORM{'output'}}); # We've checked that the product exists, and that the user can see it # This means that is OK to detaint diff --git a/sanitycheck.cgi b/sanitycheck.cgi index 286729aa5..8977ce3b5 100755 --- a/sanitycheck.cgi +++ b/sanitycheck.cgi @@ -64,9 +64,7 @@ confirm_login(); # prevents users with a legitimate interest in Bugzilla integrity # from accessing the script). UserInGroup("editbugs") - || DisplayError("You are not authorized to access this script, - which is reserved for users with the ability to edit bugs.") - && exit; + || ThrowUserError("sanity_check_access_denied"); print "Content-type: text/html\n"; print "\n"; diff --git a/showdependencygraph.cgi b/showdependencygraph.cgi index 021150bf0..c05911ee2 100755 --- a/showdependencygraph.cgi +++ b/showdependencygraph.cgi @@ -70,8 +70,7 @@ sub AddLink { $::FORM{'rankdir'} = "LR" if !defined $::FORM{'rankdir'}; if (!defined($::FORM{'id'}) && !defined($::FORM{'doall'})) { - DisplayError("No bug numbers given."); - exit; + ThrowCodeError("missing_bug_id"); } my $filename = "data/webdot/$$.dot"; diff --git a/sidebar.cgi b/sidebar.cgi index ec021ea1c..7cf823aaa 100755 --- a/sidebar.cgi +++ b/sidebar.cgi @@ -68,8 +68,7 @@ if ($useragent =~ m:Mozilla/([1-9][0-9]*):i && $1 >= 5 && $useragent !~ m/compat $template->process("sidebar.xul.tmpl", $vars) || ThrowTemplateError($template->error()); } else { - DisplayError("sidebar.cgi currently only supports Mozilla based web browsers"); - exit; + ThrowUserError("sidebar_supports_mozilla_only"); } diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index ecc222484..572c216c6 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -125,6 +125,16 @@ The target type was neither bug nor attachment but rather [% variables.target_type FILTER html %]. + [% ELSIF error == "invalid_field_name" %] + Can't use [% field FILTER html %] as a field name. + + [% ELSIF error == "invalid_output_type" %] + [% title = "Invalid Output Type" %] + Invalid output type [% type FILTER html %]. + + [% ELSIF error == "missing_bug_id" %] + No bug ID was given. + [% ELSIF error == "no_y_axis_defined" %] No Y axis was defined when creating report. The X axis is optional, but the Y axis is compulsory. @@ -138,6 +148,9 @@ [% ELSIF error == "template_error" %] [% template_error_msg %] + [% ELSIF error == "unable_to_retrieve_password" %] + I was unable to retrieve your old password from the database. + [% ELSIF error == "undefined_field" %] [% field FILTER html %] was not defined; [% Param('browserbugmessage') %] diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index fd9ffe09b..1f36f6f1e 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -143,6 +143,10 @@ really make sense to mark a bug as a duplicate of itself, does it? + [% ELSIF error == "email_change_in_progress" %] + [% title = "Email Change Already In Progress" %] + Email change already in progress; please check your email. + [% ELSIF error == "email_confirmation_failed" %] [% title = "Email Address Email Address Confirmation Failed" %] Email address confirmation failed. @@ -336,6 +340,10 @@ [% title = "Quip Required" %] Please enter a quip in the text field. + [% ELSIF error == "new_password_missing" %] + [% title = "New Password Missing" %] + You must enter a new password. + [% ELSIF error == "no_bugs_chosen" %] [% title = "No Bugs Chosen" %] You apparently didn't choose any bugs to modify. @@ -392,12 +400,38 @@ Either no products have been defined to enter bugs against or you have not been given access to any. + [% ELSIF error == "old_password_incorrect" %] + [% title = "Incorrect Old Password" %] + You did not enter your old password correctly. + + [% ELSIF error == "old_password_required" %] + [% title = "Old Password Required" %] + You must enter your old password to change email address. + + [% ELSIF error == "passwords_dont_match" %] + [% title = "Passwords Don't Match" %] + The two passwords you entered did not match. + + [% ELSIF error == "password_too_long" %] + [% title = "Password Too Long" %] + The password is more than 16 characters long. It must be no more than + 16 characters. + + [% ELSIF error == "password_too_short" %] + [% title = "Password Too Short" %] + The password is less than three characters long. It must be at least + three characters. + [% ELSIF error == "patch_too_large" %] [% title = "File Too Large" %] The file you are trying to attach is [% filesize %] kilobytes (KB) in size. Patches cannot be more than [% Param('maxpatchsize') %] KB in size. Try breaking your patch into several pieces. + [% ELSIF error == "product_access_denied" %] + [% title = "Access Denied" %] + You do not have the permissions necessary to access that product. + [% ELSIF error == "query_name_missing" %] [% title = "No Query Name Specified" %] You must enter a name for your query. @@ -408,6 +442,10 @@ intentionally cleared out the "Reassign bug to" field, [% Param("browserbugmessage") %] + [% ELSIF error == "report_access_denied" %] + [% title = "Access Denied" %] + You do not have the permissions necessary to view reports for this product. + [% ELSIF error == "requestee_too_short" %] [% title = "Requestee Name Too Short" %] One or two characters match too many users, so please enter at least @@ -433,6 +471,26 @@ [% title = "Summary Needed" %] You must enter a summary for this bug. + [% ELSIF error == "sanity_check_access_denied" %] + [% title = "Access Denied" %] + You do not have the permissions necessary to run a sanity check. + + [% ELSIF error == "sidebar_supports_mozilla_only" %] + Sorry - sidebar.cgi currently only supports Mozilla based web browsers. + Upgrade today. :-) + + [% ELSIF error == "too_many_votes_for_bug" %] + [% title = "Illegal Vote" %] + You may only use at most [% max %] votes for a single bug in the + [% prod FILTER html %] product, but you are trying to use + [% votes %]. + + [% ELSIF error == "too_many_votes_for_product" %] + [% title = "Illegal Vote" %] + You may only use at most [% max %] votes for bugs in the + [% prod FILTER html %] product, but you are trying to use + [% votes %]. + [% ELSIF error == "token_inexistent" %] [% title = "Token Does Not Exist" %] The token you submitted does not exist, has expired, or has @@ -447,6 +505,10 @@ [% title = "Unknown Tab" %] [% current_tab_name FILTER html %] is not a legal tab name. + [% ELSIF error == "votes_must_be_nonnegative" %] + [% title = "Votes Must Be Non-negative" %] + Only use non-negative numbers for your bug votes. + [% ELSIF error == "wrong_token_for_cancelling_email_change" %] [% title = "Wrong Token" %] That token cannot be used to cancel an email address change. diff --git a/token.cgi b/token.cgi index 64f5710bd..d4055d058 100755 --- a/token.cgi +++ b/token.cgi @@ -113,11 +113,7 @@ if ( $::action eq 'chgpw' ) { && defined $::FORM{'matchpassword'} || ThrowUserError("require_new_password"); - my $passworderror = ValidatePassword($::FORM{'password'}, $::FORM{'matchpassword'}); - if ( $passworderror ) { - DisplayError($passworderror); - exit; - } + ValidatePassword($::FORM{'password'}, $::FORM{'matchpassword'}); } ################################################################################ diff --git a/userprefs.cgi b/userprefs.cgi index 3e4011201..d7ad1760d 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -92,29 +92,19 @@ sub SaveAccount { my $old = SqlQuote($::FORM{'Bugzilla_password'}); SendSQL("SELECT cryptpassword FROM profiles WHERE userid = $userid"); my $oldcryptedpwd = FetchOneColumn(); - if (!$oldcryptedpwd) { - DisplayError("I was unable to retrieve your old password from the database."); - exit; - } + $oldcryptedpwd || ThrowCodeError("unable_to_retrieve_password"); + if (crypt($::FORM{'Bugzilla_password'}, $oldcryptedpwd) ne $oldcryptedpwd) { - DisplayError("You did not enter your old password correctly."); - exit; + ThrowUserError("old_password_incorrect"); } if ($pwd1 ne "" || $pwd2 ne "") { - if ($pwd1 ne $pwd2) { - DisplayError("The two passwords you entered did not match."); - exit; - } - if ($::FORM{'new_password1'} eq '') { - DisplayError("You must enter a new password."); - exit; - } - my $passworderror = ValidatePassword($pwd1); - (DisplayError($passworderror) && exit) if $passworderror; + ($pwd1 eq $pwd2) || ThrowUserError("passwords_dont_match"); + $::FORM{'new_password1'} || ThrowUserError("new_password_missing"); + ValidatePassword($pwd1); my $cryptedpassword = SqlQuote(Crypt($pwd1)); SendSQL("UPDATE profiles @@ -130,27 +120,20 @@ sub SaveAccount { my $new_login_name = trim($::FORM{'new_login_name'}); if($old_login_name ne $new_login_name) { - if( $::FORM{'Bugzilla_password'} eq "") { - DisplayError("You must enter your old password to - change email address."); - exit; - } + $::FORM{'Bugzilla_password'} + || ThrowCodeError("old_password_required"); use Token; # Block multiple email changes for the same user. if (Token::HasEmailChangeToken($userid)) { - DisplayError("Email change already in progress; - please check your email."); - exit; + ThrowUserError("email_change_in_progress"); } # Before changing an email address, confirm one does not exist. CheckEmailSyntax($new_login_name); trick_taint($new_login_name); - if (!ValidateNewUser($new_login_name)) { - DisplayError("Account $new_login_name already exists"); - exit; - } + ValidateNewUser($new_login_name) + || ThrowUserError("account_exists", {email => $new_login_name}); Token::IssueEmailChangeToken($userid,$old_login_name, $new_login_name); @@ -325,7 +308,7 @@ sub SaveFooter { "AND name = " . SqlQuote($name)); } } else { - DisplayError("Hmm, the $name query seems to have gone away."); + ThrowUserError("missing_query", {queryname => $name}); } } SendSQL("UPDATE profiles SET mybugslink = " . diff --git a/votes.cgi b/votes.cgi index 52dfb0f5c..b8fbfa847 100755 --- a/votes.cgi +++ b/votes.cgi @@ -79,7 +79,7 @@ elsif ($action eq "vote") { show_user(); } else { - DisplayError("Unknown action: " . html_quote($action)); + ThrowCodeError("unknown_action", {action => $action}); } exit; @@ -87,8 +87,8 @@ exit; # Display the names of all the people voting for this one bug. sub show_bug { my $bug_id = $::FORM{'bug_id'} - || DisplayError("Please give a bug ID to show the votes for.") - && exit; + || ThrowCodeError("missing_bug_id"); + my $total = 0; my @users; @@ -126,10 +126,7 @@ sub show_user { # After DBNameToIdAndCheck is templatised and prints a Content-Type, # the above should revert to a call to that function, and this # special error handling should go away. - if (!$who) { - DisplayError(html_quote($name) . " is not a valid username.\n"); - exit; - } + $who || ThrowUserError("invalid_username", {name => $name}); my $canedit = 1 if ($name eq $::COOKIE{'Bugzilla_login'}); @@ -255,8 +252,7 @@ sub record_votes { foreach my $id (@buglist) { ValidateBugID($id); detaint_natural($::FORM{$id}) - || DisplayError("Only use non-negative numbers for your bug votes.") - && exit; + || ThrowUserError("votes_must_be_nonnegative"); } ############################################################################ @@ -283,28 +279,20 @@ sub record_votes { $prodcount{$prod} += $::FORM{$id}; # Make sure we haven't broken the votes-per-bug limit - if ($::FORM{$id} > $max) { - $prod = html_quote($prod); - my $votes = html_quote($::FORM{$id}); - - DisplayError("You may only use at most $max votes for a single - bug in the $prod product, but you are - trying to use $votes.", "Illegal vote"); - exit(); - } + ($::FORM{$id} <= $max) + || ThrowUserError("too_many_votes_for_bug", + {max => $max, + product => $prod, + votes => $::FORM{$id}}); } # Make sure we haven't broken the votes-per-product limit foreach my $prod (keys(%prodcount)) { - if ($prodcount{$prod} > $::prodmaxvotes{$prod}) { - $prod = html_quote($prod); - - DisplayError("You may only use at most $::prodmaxvotes{$prod} - votes for bugs in the $prod product, - but you are trying to use $prodcount{$prod}.", - "Illegal vote"); - exit(); - } + ($prodcount{$prod} <= $::prodmaxvotes{$prod}) + || ThrowUserError("too_many_votes_for_product", + {max => $::prodmaxvotes{$prod}, + product => $prod, + votes => $prodcount{$prod}}); } } -- cgit v1.2.3-24-g4f1b