Bug 119715 - strictvaluechecks should always be enabled

r=justdave, gerv

5 files changed, 35 insertions, 57 deletions

diff --git a/CGI.pl b/CGI.pl
index d1a274680..9875b85dd 100644
--- a/CGI.pl
+++ b/CGI.pl
@@ -597,7 +597,7 @@ sub make_options {
         }
     }
     if (!$found && $default ne "") {
-      if ( Param("strictvaluechecks") && $::CheckOptionValues &&
+      if ( $::CheckOptionValues &&
            ($default ne $::dontchange) && ($default ne "-All-") &&
            ($default ne "DUPLICATE") ) {
         print "Possible bug database corruption has been detected.  " .
diff --git a/defparams.pl b/defparams.pl
index 819013341..871a30e3a 100644
--- a/defparams.pl
+++ b/defparams.pl
@@ -598,15 +598,8 @@ DefParam("allowuserdeletion",
          "b",
          0);
 
-
-DefParam("strictvaluechecks",
-         "Do stricter integrity checking on both form submission values and values read in from the database.",
-         "b",
-         0);
-
-
 DefParam("browserbugmessage",
-         "If strictvaluechecks is on, and the bugzilla gets unexpected data from the browser, in addition to displaying the cause of the problem, it will output this HTML as well.",
+         "If bugzilla gets unexpected data from the browser, in addition to displaying the cause of the problem, it will output this HTML as well.",
          "l",
          "this may indicate a bug in your browser.\n");
 
diff --git a/post_bug.cgi b/post_bug.cgi
index 361cba848..fa3fd075f 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -142,23 +142,21 @@ if (!exists $::FORM{'target_milestone'}) {
     $::FORM{'target_milestone'} = FetchOneColumn();
 }
 
-if ( Param("strictvaluechecks") ) {
-    GetVersionTable();  
-    CheckFormField(\%::FORM, 'product', \@::legal_product);
-    CheckFormField(\%::FORM, 'version', \@{$::versions{$::FORM{'product'}}});
-    CheckFormField(\%::FORM, 'target_milestone',
-                   \@{$::target_milestone{$::FORM{'product'}}});
-    CheckFormField(\%::FORM, 'rep_platform', \@::legal_platform);
-    CheckFormField(\%::FORM, 'bug_severity', \@::legal_severity);
-    CheckFormField(\%::FORM, 'priority', \@::legal_priority);
-    CheckFormField(\%::FORM, 'op_sys', \@::legal_opsys);
-    CheckFormFieldDefined(\%::FORM, 'assigned_to');
-    CheckFormField(\%::FORM, 'bug_status', \@::legal_bug_status);
-    CheckFormFieldDefined(\%::FORM, 'bug_file_loc');
-    CheckFormField(\%::FORM, 'component', 
-                   \@{$::components{$::FORM{'product'}}});
-    CheckFormFieldDefined(\%::FORM, 'comment');
-}
+GetVersionTable();
+CheckFormField(\%::FORM, 'product', \@::legal_product);
+CheckFormField(\%::FORM, 'version', \@{$::versions{$::FORM{'product'}}});
+CheckFormField(\%::FORM, 'target_milestone',
+               \@{$::target_milestone{$::FORM{'product'}}});
+CheckFormField(\%::FORM, 'rep_platform', \@::legal_platform);
+CheckFormField(\%::FORM, 'bug_severity', \@::legal_severity);
+CheckFormField(\%::FORM, 'priority', \@::legal_priority);
+CheckFormField(\%::FORM, 'op_sys', \@::legal_opsys);
+CheckFormFieldDefined(\%::FORM, 'assigned_to');
+CheckFormField(\%::FORM, 'bug_status', \@::legal_bug_status);
+CheckFormFieldDefined(\%::FORM, 'bug_file_loc');
+CheckFormField(\%::FORM, 'component', 
+               \@{$::components{$::FORM{'product'}}});
+CheckFormFieldDefined(\%::FORM, 'comment');
 
 my @used_fields;
 foreach my $f (@bug_fields) {
diff --git a/process_bug.cgi b/process_bug.cgi
index dcde93035..15daf152b 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -102,15 +102,13 @@ PutHeader ("Bug processed");
 
 GetVersionTable();
 
-if ( Param("strictvaluechecks") ) {
-    CheckFormFieldDefined(\%::FORM, 'product');
-    CheckFormFieldDefined(\%::FORM, 'version');
-    CheckFormFieldDefined(\%::FORM, 'component');
+CheckFormFieldDefined(\%::FORM, 'product');
+CheckFormFieldDefined(\%::FORM, 'version');
+CheckFormFieldDefined(\%::FORM, 'component');
 
-    # check if target milestone is defined - matthew@zeroknowledge.com
-    if ( Param("usetargetmilestone") ) {
-        CheckFormFieldDefined(\%::FORM, 'target_milestone');
-    }
+# check if target milestone is defined - matthew@zeroknowledge.com
+if ( Param("usetargetmilestone") ) {
+  CheckFormFieldDefined(\%::FORM, 'target_milestone');
 }
 
 ConnectToDatabase();
@@ -155,9 +153,7 @@ if ((($::FORM{'id'} && $::FORM{'product'} ne $::oldproduct)
      || (!$::FORM{'id'} && $::FORM{'product'} ne $::dontchange))
     && CheckonComment( "reassignbycomponent" ))
 {
-    if ( Param("strictvaluechecks") ) {
-        CheckFormField(\%::FORM, 'product', \@::legal_product);
-    }
+    CheckFormField(\%::FORM, 'product', \@::legal_product);
     my $prod = $::FORM{'product'};
 
     # note that when this script is called from buglist.cgi (rather
@@ -431,7 +427,7 @@ Do you wish to do this?</P>
     exit;
 } # end DuplicateUserConfirm()
 
-if (defined $::FORM{'id'} && Param('strictvaluechecks')) {
+if (defined $::FORM{'id'}) {
     # since this means that we were called from show_bug.cgi, now is a good
     # time to do a whole bunch of error checking that can't easily happen when
     # we've been called from buglist.cgi, because buglist.cgi only tweaks
@@ -668,9 +664,7 @@ if (defined $::FORM{newcc} || defined $::FORM{removecc} || defined $::FORM{massc
 }
 
 
-if ( Param('strictvaluechecks') ) {
-    CheckFormFieldDefined(\%::FORM, 'knob');
-}
+CheckFormFieldDefined(\%::FORM, 'knob');
 SWITCH: for ($::FORM{'knob'}) {
     /^none$/ && do {
         last SWITCH;
@@ -707,14 +701,12 @@ SWITCH: for ($::FORM{'knob'}) {
         }
         ChangeStatus('NEW');
         DoComma();
-        if ( Param("strictvaluechecks") ) {
-          if ( !defined$::FORM{'assigned_to'} ||
-               trim($::FORM{'assigned_to'}) eq "") {
-            PuntTryAgain("You cannot reassign to a bug to nobody.  Unless " .
-                         "you intentionally cleared out the " .
-                         "\"Reassign bug to\" field, " .
-                         Param("browserbugmessage"));
-          }
+        if ( !defined$::FORM{'assigned_to'} ||
+             trim($::FORM{'assigned_to'}) eq "") {
+          PuntTryAgain("You cannot reassign to a bug to nobody.  Unless " .
+                       "you intentionally cleared out the " .
+                       "\"Reassign bug to\" field, " .
+                       Param("browserbugmessage"));
         }
         my $newid = DBNameToIdAndCheck($::FORM{'assigned_to'});
         $::query .= "assigned_to = $newid";
@@ -772,9 +764,7 @@ SWITCH: for ($::FORM{'knob'}) {
     /^duplicate$/ && CheckonComment( "duplicate" ) && do {
         ChangeStatus('RESOLVED');
         ChangeResolution('DUPLICATE');
-        if ( Param('strictvaluechecks') ) {
-            CheckFormFieldDefined(\%::FORM,'dup_id');
-        }
+        CheckFormFieldDefined(\%::FORM,'dup_id');
         my $num = trim($::FORM{'dup_id'});
         SendSQL("SELECT bug_id FROM bugs WHERE bug_id = " . SqlQuote($num));
         $num = FetchOneColumn();
@@ -1395,9 +1385,7 @@ The changes made were:
             SendSQL("INSERT INTO cc (who, bug_id) VALUES ($reporter, " . SqlQuote($duplicate) . ")");
         }
         AppendComment($duplicate, $::COOKIE{'Bugzilla_login'}, "*** Bug $::FORM{'id'} has been marked as a duplicate of this bug. ***");
-        if ( Param('strictvaluechecks') ) {
-          CheckFormFieldDefined(\%::FORM,'comment');
-        }
+        CheckFormFieldDefined(\%::FORM,'comment');
         SendSQL("INSERT INTO duplicates VALUES ($duplicate, $::FORM{'id'})");
         print "<TABLE BORDER=1><TD><H2>Duplicate notation added to bug $duplicate</H2>\n";
         system("./processmail", $duplicate, $::COOKIE{'Bugzilla_login'});
diff --git a/query.cgi b/query.cgi
index 3795c98ea..3149d07b7 100755
--- a/query.cgi
+++ b/query.cgi
@@ -31,8 +31,7 @@ require "CGI.pl";
 
 # Prevents &make_options in CGI.pl from throwing an error if we give it
 # an invalid list of selections (from a remembered query containing values
-# that no longer exist). We don't want to die in the query page even if
-# strict value checks are turned on.
+# that no longer exist), since we don't want to die in the query page.
 $::CheckOptionValues = 0;
 
 use vars