Bug 163829 - move pref code into a separate package

Bug.pm

@@ -32,6 +32,7 @@ package Bug;
 use CGI::Carp qw(fatalsToBrowser);
 my %ok_field;
 
+use Bugzilla::Config;
 use Bugzilla::Util;
 
 for my $key (qw (bug_id alias product version rep_platform op_sys bug_status 
@@ -165,7 +166,7 @@ sub initBug  {
     $self->{'cc'} = \@cc;
   }
 
-  if (&::Param("useqacontact") && (defined $self->{'qa_contact'}) ) {
+  if (Param("useqacontact") && (defined $self->{'qa_contact'}) ) {
     my $name = $self->{'qa_contact'} > 0 ? &::DBID_to_name($self->{'qa_contact'}) :"";
     if ($name) {
       $self->{'qa_contact'} = $name;
@@ -273,8 +274,8 @@ sub emitXML {
     if (defined $self->{'longdescs'}) {
         for (my $i=0 ; $i < @{$self->{'longdescs'}} ; $i++) {
             next if ($self->{'longdescs'}[$i]->{'isprivate'} 
-                     && &::Param("insidergroup")
-                     && !&::UserInGroup(&::Param("insidergroup")));
+                     && Param("insidergroup")
+                     && !&::UserInGroup(Param("insidergroup")));
             $xml .= "  <long_desc>\n"; 
             $xml .= "   <who>" . &::DBID_to_name($self->{'longdescs'}[$i]->{'who'}) 
                                . "</who>\n"; 
@@ -289,8 +290,8 @@ sub emitXML {
     if (defined $self->{'attachments'}) {
         for (my $i=0 ; $i < @{$self->{'attachments'}} ; $i++) {
             next if ($self->{'attachments'}[$i]->{'isprivate'} 
-                     && &::Param("insidergroup")
-                     && !&::UserInGroup(&::Param("insidergroup")));
+                     && Param("insidergroup")
+                     && !&::UserInGroup(Param("insidergroup")));
             $xml .= "  <attachment>\n"; 
             $xml .= "    <attachid>" . $self->{'attachments'}[$i]->{'attachid'}
                                     . "</attachid>\n"; 

Bugzilla/Bug.pm

@@ -32,6 +32,7 @@ package Bug;
 use CGI::Carp qw(fatalsToBrowser);
 my %ok_field;
 
+use Bugzilla::Config;
 use Bugzilla::Util;
 
 for my $key (qw (bug_id alias product version rep_platform op_sys bug_status 
@@ -165,7 +166,7 @@ sub initBug  {
     $self->{'cc'} = \@cc;
   }
 
-  if (&::Param("useqacontact") && (defined $self->{'qa_contact'}) ) {
+  if (Param("useqacontact") && (defined $self->{'qa_contact'}) ) {
     my $name = $self->{'qa_contact'} > 0 ? &::DBID_to_name($self->{'qa_contact'}) :"";
     if ($name) {
       $self->{'qa_contact'} = $name;
@@ -273,8 +274,8 @@ sub emitXML {
     if (defined $self->{'longdescs'}) {
         for (my $i=0 ; $i < @{$self->{'longdescs'}} ; $i++) {
             next if ($self->{'longdescs'}[$i]->{'isprivate'} 
-                     && &::Param("insidergroup")
-                     && !&::UserInGroup(&::Param("insidergroup")));
+                     && Param("insidergroup")
+                     && !&::UserInGroup(Param("insidergroup")));
             $xml .= "  <long_desc>\n"; 
             $xml .= "   <who>" . &::DBID_to_name($self->{'longdescs'}[$i]->{'who'}) 
                                . "</who>\n"; 
@@ -289,8 +290,8 @@ sub emitXML {
     if (defined $self->{'attachments'}) {
         for (my $i=0 ; $i < @{$self->{'attachments'}} ; $i++) {
             next if ($self->{'attachments'}[$i]->{'isprivate'} 
-                     && &::Param("insidergroup")
-                     && !&::UserInGroup(&::Param("insidergroup")));
+                     && Param("insidergroup")
+                     && !&::UserInGroup(Param("insidergroup")));
             $xml .= "  <attachment>\n"; 
             $xml .= "    <attachid>" . $self->{'attachments'}[$i]->{'attachid'}
                                     . "</attachid>\n"; 

Bugzilla/Config.pm

+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Bug Tracking System.
+#
+# The Initial Developer of the Original Code is Netscape Communications
+# Corporation. Portions created by Netscape are
+# Copyright (C) 1998 Netscape Communications Corporation. All
+# Rights Reserved.
+#
+# Contributor(s): Terry Weissman <terry@mozilla.org>
+#                 Dawn Endico <endico@mozilla.org>
+#                 Dan Mosedale <dmose@mozilla.org>
+#                 Joe Robins <jmrobins@tgix.com>
+#                 Jake <jake@bugzilla.org>
+#                 J. Paul Reed <preed@sigkill.com>
+#                 Bradley Baetz <bbaetz@student.usyd.edu.au>
+#                 Christopher Aillon <christopher@aillon.com>
+
+package Bugzilla::Config;
+
+=head1 NAME
+
+Bugzilla::Config - Configuration paramaters for Bugzilla
+
+=head1 SYNOPSIS
+
+  # Getting paramaters
+  use Bugzilla::Config;
+
+  my $fooSetting = Param('foo');
+
+  # Administration functions
+  use Bugzilla::Config qw(:admin);
+
+  my @valid_params = GetParamList();
+  my @removed_params = UpgradeParams();
+  SetParam($param, $value);
+  WriteParams();
+
+  # Localconfig variables may also be imported
+  use Bugzilla::Config qw(:db);
+  print "Connecting to $db_name as $db_user with $db_pass\n";
+
+  # These variables do not belong in localconfig, and need to go
+  # somewhere better
+  use Bugzilla::Config($contenttypes $pages)
+
+=head1 DESCRIPTION
+
+This package contains ways to access Bugzilla configuration parameters.
+
+=cut
+
+use strict;
+
+use base qw(Exporter);
+
+use Bugzilla::Util;
+
+# Module stuff
+@Bugzilla::Config::EXPORT = qw(Param);
+
+# Don't export localvars by default - people should have to explicitly
+# ask for it, as a (probably futile) attempt to stop code using it
+# when it shouldn't
+# ChmodDataFile is here until that stuff all moves out of globals.pl
+# into this file
+@Bugzilla::Config::EXPORT_OK = qw($contenttypes $pages ChmodDataFile);
+%Bugzilla::Config::EXPORT_TAGS =
+  (
+   admin => [qw(GetParamList UpdateParams SetParam WriteParams)],
+   db => [qw($db_host $db_port $db_name $db_user $db_pass)],
+  );
+Exporter::export_ok_tags('admin', 'db');
+
+# Bugzilla version
+$Bugzilla::Config::VERSION = "2.17";
+
+use Data::Dumper;
+
+# This only has an affect for Data::Dumper >= 2.12 (ie perl >= 5.8.0)
+# Its just cosmetic, though, so that doesn't matter
+$Data::Dumper::Sortkeys = 1;
+
+use File::Temp;
+use Safe;
+
+use vars qw(@param_list);
+my %param;
+
+# INITIALISATION CODE
+
+# XXX - mod_perl - need to register Apache init handler for params
+sub _load_datafiles {
+    # read in localconfig variables
+    do 'localconfig';
+
+    if (-e 'data/params') {
+        # Handle reading old param files by munging the symbol table
+        # Don't have to do this if we use safe mode, since its evaled
+        # in a sandbox where $foo is in the same module as $::foo
+        #local *::param = \%param;
+
+        # Note that checksetup.pl sets file permissions on 'data/params'
+
+        # Using Safe mode is _not_ a guarantee of safety if someone does
+        # manage to write to the file. However, it won't hurt...
+        # See bug 165144 for not needing to eval this at all
+        my $s = new Safe;
+
+        $s->rdo('data/params');
+        die "Error evaluating data/params: $@" if $@;
+
+        # Now read the param back out from the sandbox
+        %param = %{$s->varglob('param')};
+    }
+}
+
+# Load in the datafiles
+_load_datafiles();
+
+# Load in the param defintions
+do 'defparams.pl';
+
+# Stick the params into a hash
+my %params;
+foreach my $item (@param_list) {
+    $params{$item->{'name'}} = $item;
+}
+
+# END INIT CODE
+
+# Subroutines go here
+
+=head1 FUNCTIONS
+
+=head2 Parameters
+
+Parameters can be set, retrieved, and updated.
+
+=over 4
+
+=item C<Param($name)>
+
+Returns the Param with the specified name. Either a string, or, in the case
+of multiple-choice parameters, an array reference.
+
+=cut
+
+sub Param {
+    my ($param) = @_;
+
+    # By this stage, the param must be in the hash
+    die "Can't find param named $param" unless (exists $param{$param});
+
+    return $param{$param};
+}
+
+=item C<GetParamList()>
+
+Returns the list of known paramater types, from defparams.pl. Users should not
+rely on this method; it is intended for editparams/doeditparams only
+
+The format for the list is specified in defparams.pl
+
+=cut
+
+sub GetParamList {
+    return @param_list;
+}
+
+=item C<SetParam($name, $value)>
+
+Sets the param named $name to $value. Values are checked using the checker
+function for the given param if one exists.
+
+=cut
+
+sub SetParam {
+    my ($name, $value) = @_;
+
+    die "Unknown param $name" unless (exists $params{$name});
+
+    my $entry = $params{$name};
+
+    # sanity check the value
+    if (exists $entry->{'checker'}) {
+        my $err = $entry->{'checker'}->($value, $entry);
+        die "Param $name is not valid: $err" unless $err eq '';
+    }
+
+    $param{$name} = $value;
+}
+
+=item C<UpdateParams()>
+
+Updates the parameters, by transitioning old params to new formats, setting
+defaults for new params, and removing obsolete ones.
+
+Any removed params are returned in a list, with elements [$item, $oldvalue]
+where $item is the entry in the param list.
+
+This change is not flushed to disk, use L<C<WriteParams()>> for that.
+
+=cut
+
+sub UpdateParams {
+    # --- PARAM CONVERSION CODE ---
+
+    # Note that this isn't particuarly 'clean' in terms of separating
+    # the backend code (ie this) from the actual params.
+    # We don't care about that, though
+
+    # Old bugzilla versions stored the version number in the params file
+    # We don't want it, so get rid of it
+    delete $param{'version'};
+
+    # Change from a boolean for quips to multi-state
+    if (exists $param{'usequip'} && !exists $param{'allowquips'}) {
+        $param{'allowquips'} = $param{'usequip'} ? 'on' : 'off';
+        delete $param{'usequip'};
+    }
+
+    # --- DEFAULTS FOR NEW PARAMS ---
+
+    foreach my $item (@param_list) {
+        my $name = $item->{'name'};
+        $param{$name} = $item->{'default'} unless exists $param{$name};
+    }
+
+    # --- REMOVE OLD PARAMS ---
+
+    my @oldparams = ();
+
+    # Remove any old params
+    foreach my $item (keys %param) {
+        if (!grep($_ eq $item, map ($_->{'name'}, @param_list))) {
+            local $Data::Dumper::Terse = 1;
+            local $Data::Dumper::Indent = 0;
+            push (@oldparams, [$item, Data::Dumper->Dump([$param{$item}])]);
+            delete $param{$item};
+        }
+    }
+
+    return @oldparams;
+}
+
+=item C<WriteParams()>
+
+Writes the parameters to disk.
+
+=cut
+
+sub WriteParams {
+    my ($fh, $tmpname) = File::Temp::tempfile('params.XXXXX',
+                                              DIR => 'data' );
+
+    print $fh (Data::Dumper->Dump([ \%param ], [ '*param' ]))
+      || die "Can't write param file: $!";
+
+    close $fh;
+
+    rename $tmpname, "data/params"
+      || die "Can't rename $tmpname to data/params: $!";
+
+    ChmodDataFile('data/params', 0666);
+}
+
+# Some files in the data directory must be world readable iff we don't have
+# a webserver group. Call this function to do this.
+# This will become a private function once all the datafile handling stuff
+# moves into this package
+
+# This sub is not perldoc'd for that reason - noone should know about it
+sub ChmodDataFile {
+    my ($file, $mask) = @_;
+    my $perm = 0770;
+    if ((stat('data'))[2] & 0002) {
+        $perm = 0777;
+    }
+    $perm = $perm & $mask;
+    chmod $perm,$file;
+}
+
+=back
+
+=head2 Parameter checking functions
+
+All parameter checking functions are called with two parameters:
+
+=over 4
+
+=item *
+
+The new value for the parameter
+
+=item *
+
+A reference to the entry in the param list for this parameter
+
+=back
+
+Functions should return error text, or the empty string if there was no error.
+
+=over 4
+
+=item C<check_multi>
+
+Checks that a multi-valued parameter (ie type C<s> or type C<m>) satisfies
+its contraints.
+
+=cut
+
+sub check_multi {
+    my ($value, $param) = (@_);
+
+    if ($param->{'type'} eq "s") {
+        unless (lsearch($param->{'choices'}, $value) >= 0) {
+            return "Invalid choice '$value' for single-select list param '$param'";
+        }
+
+        return "";
+    }
+    elsif ($param->{'type'} eq "m") {
+        foreach my $chkParam (@$value) {
+            unless (lsearch($param->{'choices'}, $chkParam) >= 0) {
+                return "Invalid choice '$chkParam' for multi-select list param '$param'";
+            }
+        }
+
+        return "";
+    }
+    else {
+        return "Invalid param type '$param->{'type'}' for check_multi(); " .
+          "contact your Bugzilla administrator";
+    }
+}
+
+=item C<check_numeric>
+
+Checks that the value is a valid number
+
+=cut
+
+sub check_numeric {
+    my ($value) = (@_);
+    if ($value !~ /^[0-9]+$/) {
+        return "must be a numeric value";
+    }
+    return "";
+}
+
+=item C<check_regexp>
+
+Checks that the value is a valid regexp
+
+=cut
+
+sub check_regexp {
+    my ($value) = (@_);
+    eval { qr/$value/ };
+    return $@;
+}
+
+=back
+
+=cut
+
+1;

Bugzilla/Search.pm

@@ -34,6 +34,7 @@ use vars qw($userid $usergroupset);
 
 package Bugzilla::Search;
 
+use Bugzilla::Config;
 use Bugzilla::Util;
 
 use Date::Format;
@@ -314,7 +315,7 @@ sub init {
          "^long_?desc," => sub {
              my $table = "longdescs_$chartid";
              push(@supptables, "longdescs $table");
-             if (&::Param("insidergroup") && !&::UserInGroup(&::Param("insidergroup"))) {
+             if (Param("insidergroup") && !&::UserInGroup(Param("insidergroup"))) {
                  push(@wherepart, "$table.isprivate < 1") ;
              }
              push(@wherepart, "$table.bug_id = bugs.bug_id");

Bugzilla/Token.pm

@@ -29,6 +29,8 @@ use strict;
 # Bundle the functions in this file together into the "Token" package.
 package Token;
 
+use Bugzilla::Config;
+
 use Date::Format;
 
 # This module requires that its caller have said "require CGI.pl" to import
@@ -75,14 +77,14 @@ sub IssueEmailChangeToken {
     my $template = $::template;
     my $vars = $::vars;
 
-    $vars->{'oldemailaddress'} = $old_email . &::Param('emailsuffix');
-    $vars->{'newemailaddress'} = $new_email . &::Param('emailsuffix');
+    $vars->{'oldemailaddress'} = $old_email . Param('emailsuffix');
+    $vars->{'newemailaddress'} = $new_email . Param('emailsuffix');
     
     $vars->{'max_token_age'} = $maxtokenage;
     $vars->{'token_ts'} = $token_ts;
 
     $vars->{'token'} = $token;
-    $vars->{'emailaddress'} = $old_email . &::Param('emailsuffix');
+    $vars->{'emailaddress'} = $old_email . Param('emailsuffix');
 
     my $message;
     $template->process("account/email/change-old.txt.tmpl", $vars, \$message)
@@ -93,7 +95,7 @@ sub IssueEmailChangeToken {
     close SENDMAIL;
 
     $vars->{'token'} = $newtoken;
-    $vars->{'emailaddress'} = $new_email . &::Param('emailsuffix');
+    $vars->{'emailaddress'} = $new_email . Param('emailsuffix');
 
     $message = "";
     $template->process("account/email/change-new.txt.tmpl", $vars, \$message)
@@ -136,7 +138,7 @@ sub IssuePasswordToken {
     my $vars = $::vars;
 
     $vars->{'token'} = $token;
-    $vars->{'emailaddress'} = $loginname . &::Param('emailsuffix');
+    $vars->{'emailaddress'} = $loginname . Param('emailsuffix');
 
     $vars->{'max_token_age'} = $maxtokenage;
     $vars->{'token_ts'} = $token_ts;
@@ -206,7 +208,7 @@ sub Cancel {
     my ($issuedate, $tokentype, $eventdata, $loginname, $realname) = &::FetchSQLData();
 
     # Get the email address of the Bugzilla maintainer.
-    my $maintainer = &::Param('maintainer');
+    my $maintainer = Param('maintainer');
 
     # Format the user's real name and email address into a single string.
     my $username = $realname ? $realname . " <" . $loginname . ">" : $loginname;

Bugzilla/Util.pm

@@ -258,3 +258,5 @@ sub trim {
 =back
 
 =cut
+
+1;

CGI.pl

@@ -33,6 +33,7 @@ use lib ".";
 # use Carp;                       # for confess
 
 use Bugzilla::Util;
+use Bugzilla::Config;
 
 # commented out the following snippet of code. this tosses errors into the
 # CGI if you are perl 5.6, and doesn't if you have perl 5.003. 

Token.pm

@@ -29,6 +29,8 @@ use strict;
 # Bundle the functions in this file together into the "Token" package.
 package Token;
 
+use Bugzilla::Config;
+
 use Date::Format;
 
 # This module requires that its caller have said "require CGI.pl" to import
@@ -75,14 +77,14 @@ sub IssueEmailChangeToken {
     my $template = $::template;
     my $vars = $::vars;
 
-    $vars->{'oldemailaddress'} = $old_email . &::Param('emailsuffix');
-    $vars->{'newemailaddress'} = $new_email . &::Param('emailsuffix');
+    $vars->{'oldemailaddress'} = $old_email . Param('emailsuffix');
+    $vars->{'newemailaddress'} = $new_email . Param('emailsuffix');
     
     $vars->{'max_token_age'} = $maxtokenage;
     $vars->{'token_ts'} = $token_ts;
 
     $vars->{'token'} = $token;
-    $vars->{'emailaddress'} = $old_email . &::Param('emailsuffix');
+    $vars->{'emailaddress'} = $old_email . Param('emailsuffix');
 
     my $message;
     $template->process("account/email/change-old.txt.tmpl", $vars, \$message)
@@ -93,7 +95,7 @@ sub IssueEmailChangeToken {
     close SENDMAIL;
 
     $vars->{'token'} = $newtoken;
-    $vars->{'emailaddress'} = $new_email . &::Param('emailsuffix');
+    $vars->{'emailaddress'} = $new_email . Param('emailsuffix');
 
     $message = "";
     $template->process("account/email/change-new.txt.tmpl", $vars, \$message)
@@ -136,7 +138,7 @@ sub IssuePasswordToken {
     my $vars = $::vars;
 
     $vars->{'token'} = $token;
-    $vars->{'emailaddress'} = $loginname . &::Param('emailsuffix');
+    $vars->{'emailaddress'} = $loginname . Param('emailsuffix');
 
     $vars->{'max_token_age'} = $maxtokenage;
     $vars->{'token_ts'} = $token_ts;
@@ -206,7 +208,7 @@ sub Cancel {
     my ($issuedate, $tokentype, $eventdata, $loginname, $realname) = &::FetchSQLData();
 
     # Get the email address of the Bugzilla maintainer.
-    my $maintainer = &::Param('maintainer');
+    my $maintainer = Param('maintainer');
 
     # Format the user's real name and email address into a single string.
     my $username = $realname ? $realname . " <" . $loginname . ">" : $loginname;

checksetup.pl

@@ -96,6 +96,8 @@
 
 use strict;
 
+use Bugzilla::Config qw(:DEFAULT :admin);
+
 # 12/17/00 justdave@syndicomm.com - removed declarations of the localconfig
 # variables from this location.  We don't want these declared here.  They'll
 # automatically get declared in the process of reading in localconfig, and
@@ -970,36 +972,33 @@ END
 # Just to be sure ...
 unlink "data/versioncache";
 
-# Remove parameters from the data/params file that no longer exist in Bugzilla.
-if (-e "data/params") {
-    require "data/params";
-    require "defparams.pl";
-    use vars @::param_list;
-    my @oldparams;
+# Remove parameters from the data/params file that no longer exist in Bugzilla,
+# and set the defaults for new ones
+
+my @oldparams = UpdateParams();
 
+if (@oldparams) {
     open(PARAMFILE, ">>old-params.txt") 
       || die "$0: Can't open old-params.txt for writing: $!\n";
 
-    foreach my $item (keys %::param) {
-        if (!grep($_ eq $item, @::param_list) && $item ne "version") {
-            push (@oldparams, $item);
-            print PARAMFILE "\n\n$item:\n$::param{$item}\n";
-                
-            delete $::param{$item};
-        }
-    }
-    
-    if (@oldparams) {
     print "The following parameters are no longer used in Bugzilla, " .
       "and so have been\nremoved from your parameters file and " .
       "appended to old-params.txt:\n";
-        print join(", ", @oldparams) . "\n\n";               
-    }
 
+    foreach my $p (@oldparams) {
+        my ($item, $value) = @{$p};
+
+        print PARAMFILE "\n\n$item:\n$value\n";
+
+        print $item;
+        print ", " unless $item eq $oldparams[$#oldparams]->[0];
+    }
+    print "\n";
     close PARAMFILE;
-    WriteParams();
 }
 
+# WriteParams will only write out still-valid entries
+WriteParams();
 
 ###########################################################################
 # Set proper rights
@@ -1239,14 +1238,12 @@ END { $dbh->disconnect if $dbh }
 # and that the generated images are accessible.
 #
 
-if(-e "data/params") {
-  require "data/params";
-  if( $::param{'webdotbase'} && $::param{'webdotbase'} !~ /^https?:/ ) {
+if( Param('webdotbase') && Param('webdotbase') !~ /^https?:/ ) {
     printf("Checking for %15s %-9s ", "GraphViz", "(any)");
-    if(-x $::param{'webdotbase'}) {
+    if(-x Param('webdotbase')) {
         print "ok: found\n";
     } else {
-      print "not a valid executable: $::param{'webdotbase'}\n";
+        print "not a valid executable: " . Param{'webdotbase'} . "\n";
     }
 
     # Check .htaccess allows access to generated images
@@ -1258,7 +1255,6 @@ if(-e "data/params") {
       }
       close HTACCESS;
     }
-  }
 }
 
 print "\n";
@@ -1948,23 +1944,8 @@ if ($sth->rows == 0) {
   my $pass2 = "*";
   my $admin_ok = 0;
   my $admin_create = 1;
-  my $mailcheckexp = "";
-  my $mailcheck    = ""; 
-
-  # Here we look to see what the emailregexp is set to so we can 
-  # check the email addy they enter. Bug 96675. If they have no 
-  # params (likely but not always the case), we use the default.
-  if (-e "data/params") { 
-    require "data/params"; # if they have a params file, use that
-  }
-  if ($::param{emailregexp}) {
-    $mailcheckexp = $::param{emailregexp};
-    $mailcheck    = $::param{emailregexpdesc};
-  } else {
-    $mailcheckexp = '^[^@]+@[^@]+\\.[^@]+$';
-    $mailcheck    = 'A legal address must contain exactly one \'@\', 
-      and at least one \'.\' after the @.';
-  }
+  my $mailcheckexp = Param('emailregexp');
+  my $mailcheck    = Param('emailregexpdesc');
 
   print "\nLooks like we don't have an administrator set up yet.  Either this is your\n";
   print "first time using Bugzilla, or your administrator's privs might have accidently\n";

doeditparams.cgi

@@ -25,13 +25,11 @@ use strict;
 
 use lib qw(.);
 
+use Bugzilla::Config qw(:DEFAULT :admin);
+
 require "CGI.pl";
-require "defparams.pl";
 
-# Shut up misguided -w warnings about "used only once":
-use vars %::param,
-    %::param_default,
-    @::param_list;
+use vars %::MFORM;
 
 ConnectToDatabase();
 confirm_login();
@@ -45,62 +43,57 @@ if (!UserInGroup("tweakparams")) {
     exit;
 }
 
-
 PutHeader("Saving new parameters");
 
-foreach my $i (@::param_list) {
-#    print "Processing $i...<BR>\n";
-    if (exists $::FORM{"reset-$i"}) {
-        if ($::param_type{$i} eq "s") {
-            my $index = get_select_param_index($i, $::param_default{$i}->[1]);
-            die "Param not found for '$i'" if ($index eq undef);
-            $::FORM{$i} = $index; 
+foreach my $i (GetParamList()) {
+    my $name = $i->{'name'};
+    my $value = $::FORM{$name};
+    if (exists $::FORM{"reset-$name"}) {
+        $value = $i->{'default'};
+    } else {
+        if ($i->{'type'} eq 'm') {
+            # This simplifies the code below
+            $value = \@{$::MFORM{$name}};
+        } else {
+            # Get rid of windows/mac-style line endings.
+            $value =~ s/\r\n?/\n/g;
+
+            # assume single linefeed is an empty string
+            $value =~ s/^\n$//;
         }
-        elsif ($::param_type{$i} eq "m") {
-            # For 'multi' selects, default is the 2nd anon array of the default
-            @{$::MFORM{$i}} = ();
-            foreach my $defaultPrm (@{$::param_default{$i}->[1]}) {
-                my $index = get_select_param_index($i, $defaultPrm); 
-                die "Param not found for '$i'" if ($index eq undef);
-                push(@{$::MFORM{$i}}, $index); 
     }
+    my $changed;
+    if ($i->{'type'} eq 'm') {
+        my @old = sort @{Param($name)};
+        my @new = sort @$value;
+        if (scalar(@old) != scalar(@new)) {
+            $changed = 1;
+        } else {
+            $changed = 0; # Assume not changed...
+            for (my $cnt = 0; $cnt < scalar(@old); ++$cnt) {
+                if ($old[$cnt] ne $new[$cnt]) {
+                    # entry is different, therefore changed
+                    $changed = 1;
+                    last;
                 }
-        else {
-            $::FORM{$i} = $::param_default{$i};
             }
         }
-    $::FORM{$i} =~ s/\r\n?/\n/g;   # Get rid of windows/mac-style line endings.
-    $::FORM{$i} =~ s/^\n$//;      # assume single linefeed is an empty string
-    if ($::FORM{$i} ne Param($i)) {
-        if (defined $::param_checker{$i}) {
-            my $ref = $::param_checker{$i};
-            my $ok = &$ref($::FORM{$i}, $i);
+    } else {
+        $changed = ($value eq Param($name) ? 0 : 1);
+    }
+    if ($changed) {
+        if (exists $i->{'checker'}) {
+            my $ok = $i->{'checker'}->($value, $i);
             if ($ok ne "") {
-                print "New value for $i is invalid: $ok<p>\n";
+                print "New value for " . html_quote($name) .
+                  " is invalid: $ok<p>\n";
                 print "Please hit <b>Back</b> and try again.\n";
                 PutFooter();
                 exit;
             }
         }
-        print "Changed $i.<br>\n";
-#      print "Old: '" . url_quote(Param($i)) . "'<BR>\n";
-#      print "New: '" . url_quote($::FORM{$i}) . "'<BR>\n";
-        if ($::param_type{$i} eq "s") {
-            $::param{$i} = $::param_default{$i}->[0]->[$::FORM{$i}];
-        }
-        elsif ($::param_type{$i} eq "m") {
-            my $multiParamStr = "[ ";
-            foreach my $chosenParam (@{$::MFORM{$i}}) {
-                $multiParamStr .= 
-                 "'$::param_default{$i}->[0]->[$chosenParam]', ";
-            }
-            $multiParamStr .= " ]";
-
-            $::param{$i} = $multiParamStr;
-        }
-        else {
-            $::param{$i} = $::FORM{$i};
-        }
+        print "Changed " . html_quote($name) . "<br>\n";
+        SetParam($name, $value);
     }
 }
 

editparams.cgi

@@ -25,13 +25,9 @@
 use strict;
 use lib ".";
 
-require "CGI.pl";
-require "defparams.pl";
+use Bugzilla::Config qw(:DEFAULT :admin);
 
-# Shut up misguided -w warnings about "used only once":
-use vars @::param_desc,
-    @::param_list,
-    @::param_default;
+require "CGI.pl";
 
 ConnectToDatabase();
 confirm_login();
@@ -59,18 +55,19 @@ print "<form method=post action=doeditparams.cgi><table>\n";
 my $rowbreak = "<tr><td colspan=2><hr></td></tr>";
 print $rowbreak;
 
-foreach my $i (@::param_list) {
-    print "<tr><th align=right valign=top>$i:</th><td>$::param_desc{$i}</td></tr>\n";
-    print "<tr><td valign=top><input type=checkbox name=reset-$i>Reset</td><td>\n";
-    my $value = Param($i);
-    SWITCH: for ($::param_type{$i}) {
+foreach my $i (GetParamList()) {
+    my $name = $i->{'name'};
+    my $value = Param($name);
+    print "<tr><th align=right valign=top>$name:</th><td>$i->{'desc'}</td></tr>\n";
+    print "<tr><td valign=top><input type=checkbox name=reset-$name>Reset</td><td>\n";
+    SWITCH: for ($i->{'type'}) {
         /^t$/ && do {
-            print "<input size=80 name=$i value=\"" .
+            print "<input size=80 name=$name value=\"" .
                 value_quote($value) . "\">\n";
             last SWITCH;
         };
         /^l$/ && do {
-            print "<textarea wrap=hard name=$i rows=10 cols=80>" .
+            print "<textarea wrap=hard name=$name rows=10 cols=80>" .
                 value_quote($value) . "</textarea>\n";
             last SWITCH;
         };
@@ -84,64 +81,60 @@ foreach my $i (@::param_list) {
                 $on = "";
                 $off = "checked";
             }
-            print "<input type=radio name=$i value=1 $on>On\n";
-            print "<input type=radio name=$i value=0 $off>Off\n";
+            print "<input type=radio name=$name value=1 $on>On\n";
+            print "<input type=radio name=$name value=0 $off>Off\n";
             last SWITCH;
         };
         /^m$/ && do {
-            my $optList = $::param_default{$i}->[0]; #'cause we use it so much 
+            my @choices = @{$i->{'choices'}};
             ## showing 5 options seems like a nice round number; this should
             ## probably be configurable; if you care, file a bug ;-)
-            my $boxSize = scalar(@{$optList}) < 5 ? scalar(@{$optList}) : 5;
+            my $boxSize = scalar(@choices) < 5 ? scalar(@choices) : 5;
 
-            print "<select multiple size=\"$boxSize\" name=\"$i\">\n";
+            print "<select multiple size=\"$boxSize\" name=\"$name\">\n";
 
-            for (my $optNum = 0; $optNum < scalar(@{$optList}); $optNum++) {
+            foreach my $item (@choices) {
                 my $selected = "";
 
-                foreach my $selectedVal (@{$value}) {
-                    if ($selectedVal eq $optList->[$optNum]) {
+                if (lsearch($value, $item) >= 0) {
                     $selected = "selected";
-                        last;
-                    }
                 }
 
-                print "<option $selected value=\"$optNum\">" .
-                 "$optList->[$optNum]</option>\n";
+                print "<option $selected value=\"" . html_quote($item) . "\">" .
+                 html_quote($item) . "</option>\n";
             }
 
             print "</select>\n";
             last SWITCH;
         };
         /^s$/ && do {
-            print "<select name=\"$i\">\n";
-            #'cause we use it so much below
-            my $optList = $::param_default{$i}->[0]; 
+            print "<select name=\"$name\">\n";
+            my @choices = @{$i->{'choices'}};
 
-            for (my $optNum = 0; $optNum < scalar(@{$optList}); $optNum++) {
+            foreach my $item (@choices) {
                 my $selected = "";
-                if ($value eq $optList->[$optNum]) {
+
+                if ($value eq $item) {
                     $selected = "selected";
                 }
 
-                print "<option $selected value=\"$optNum\">" .
-                 "$optList->[$optNum]</option>\n";
+                print "<option $selected value=\"" . html_quote($item) . "\">" .
+                  html_quote($item) . "</option>\n";
+
             }
             print "</select>\n";
             last SWITCH;
         };
         # DEFAULT
-        print "<font color=red><blink>Unknown param type $::param_type{$i}!!!</blink></font>\n";
+        print "<font color=red><blink>Unknown param type $i->{'type'}!!!</blink></font>\n";
     }
     print "</td></tr>\n";
     print $rowbreak;
 }
 
 print "<tr><th align=right valign=top>version:</th><td>
-What version of Bugzilla this is.  This can't be modified here, but
-<tt>%version%</tt> can be used as a parameter in places that understand
-such parameters</td></tr>
-<tr><td></td><td>" . Param('version') . "</td></tr>";
+What version of Bugzilla this is. This can't be modified.
+<tr><td></td><td>" . $Bugzilla::Config::VERSION . "</td></tr>";
 
 print "</table>\n";
 

globals.pl

@@ -28,6 +28,8 @@
 use strict;
 
 use Bugzilla::Util;
+# Bring ChmodDataFile in until this is all moved to the module
+use Bugzilla::Config qw(:DEFAULT ChmodDataFile);
 
 # Shut up misguided -w warnings about "used only once".  For some reason,
 # "use vars" chokes on me when I try it here.
@@ -53,7 +55,6 @@ sub globals_pl_sillyness {
     $zz = @main::legal_target_milestone;
     $zz = @main::legal_versions;
     $zz = @main::milestoneurl;
-    $zz = %main::param_type;
     $zz = %main::proddesc;
     $zz = @main::prodmaxvotes;
     $zz = $main::superusergroupset;
@@ -67,12 +68,8 @@ sub globals_pl_sillyness {
 # here
 # 
 
-$::db_host = "localhost";
-$::db_port = 3306;
-$::db_name = "bugs";
-$::db_user = "bugs";
-$::db_pass = "";
-
+# XXX - Move this to Bugzilla::Config once code which uses these has moved out
+# of globals.pl
 do 'localconfig';
 
 use DBI;
@@ -99,9 +96,6 @@ $::ENV{'PATH'} = '';
 $::SIG{TERM} = 'IGNORE';
 $::SIG{PIPE} = 'IGNORE';
 
-# Contains the version string for the current running Bugzilla.
-$::param{'version'} = '2.17';
-
 $::dontchange = "--do_not_change--";
 $::chooseone = "--Choose_one:--";
 $::defaultqueryname = "(Default query)";
@@ -119,18 +113,6 @@ $::superusergroupset = "9223372036854775807";
 #}
 #$::SIG{__DIE__} = \&die_with_dignity;
 
-# Some files in the data directory must be world readable iff we don't have
-# a webserver group. Call this function to do this.
-sub ChmodDataFile($$) {
-    my ($file, $mask) = @_;
-    my $perm = 0770;
-    if ((stat('data'))[2] & 0002) {
-        $perm = 0777;
-    }
-    $perm = $perm & $mask;
-    chmod $perm,$file;
-}
-
 sub ConnectToDatabase {
     my ($useshadow) = (@_);
     if (!defined $::db) {
@@ -346,70 +328,7 @@ sub GetFieldID {
     return $fieldid;
 }
 
-# Generate a string which, when later interpreted by the Perl compiler, will
-# be the same as the given string.
-
-sub PerlQuote {
-    my ($str) = (@_);
-    return SqlQuote($str);
-    
-# The below was my first attempt, but I think just using SqlQuote makes more 
-# sense...
-#     $result = "'";
-#     $length = length($str);
-#     for (my $i=0 ; $i<$length ; $i++) {
-#         my $c = substr($str, $i, 1);
-#         if ($c eq "'" || $c eq '\\') {
-#             $result .= '\\';
-#         }
-#         $result .= $c;
-#     }
-#     $result .= "'";
-#     return $result;
-}
-
-
-# Given the name of a global variable, generate Perl code that, if later
-# executed, would restore the variable to its current value.
-
-sub GenerateCode {
-    my ($name) = (@_);
-    my $result = $name . " = ";
-    if ($name =~ /^\$/) {
-        my $value = eval($name);
-        if (ref($value) eq "ARRAY") {
-            $result .= "[" . GenerateArrayCode($value) . "]";
-        } else {
-            $result .= PerlQuote(eval($name));
-        }
-    } elsif ($name =~ /^@/) {
-        my @value = eval($name);
-        $result .= "(" . GenerateArrayCode(\@value) . ")";
-    } elsif ($name =~ '%') {
-        $result = "";
-        foreach my $k (sort { uc($a) cmp uc($b)} eval("keys $name")) {
-            $result .= GenerateCode("\$" . substr($name, 1) .
-                                    "{" . PerlQuote($k) . "}");
-        }
-        return $result;
-    } else {
-        die "Can't do $name -- unacceptable variable type.";
-    }
-    $result .= ";\n";
-    return $result;
-}
-
-sub GenerateArrayCode {
-    my ($ref) = (@_);
-    my @list;
-    foreach my $i (@$ref) {
-        push @list, PerlQuote($i);
-    }
-    return join(',', @list);
-}
-
-
-
+# XXXX - this needs to go away
 sub GenerateVersionTable {
     SendSQL("SELECT versions.value, products.name " .
             "FROM versions, products " .
@@ -505,8 +424,9 @@ sub GenerateVersionTable {
     print FID "# Any changes you make will be overwritten.\n";
     print FID "#\n";
 
-    print FID GenerateCode('@::log_columns');
-    print FID GenerateCode('%::versions');
+    use Data::Dumper;
+    print FID Data::Dumper->Dump([\@::log_columns, \%::versions],
+                                 ['*::log_columns', '*::versions']);
 
     foreach my $i (@list) {
         if (!defined $::components{$i}) {
@@ -514,18 +434,23 @@ sub GenerateVersionTable {
         }
     }
     @::legal_versions = sort {uc($a) cmp uc($b)} keys(%varray);
-    print FID GenerateCode('@::legal_versions');
-    print FID GenerateCode('%::components');
+    print FID Data::Dumper->Dump([\@::legal_versions, \%::components],
+                                 ['*::legal_versions', '*::components']);
     @::legal_components = sort {uc($a) cmp uc($b)} keys(%carray);
-    print FID GenerateCode('@::legal_components');
-    foreach my $i('product', 'priority', 'severity', 'platform', 'opsys',
-                  'bug_status', 'resolution') {
-        print FID GenerateCode('@::legal_' . $i);
-    }
-    print FID GenerateCode('@::settable_resolution');
-    print FID GenerateCode('%::proddesc');
-    print FID GenerateCode('@::enterable_products');
-    print FID GenerateCode('%::prodmaxvotes');
+
+    print FID Data::Dumper->Dump([\@::legal_components, \@::legal_product,
+                                  \@::legal_priority, \@::legal_severity,
+                                  \@::legal_platform, \@::legal_opsys,
+                                  \@::legal_bug_status, \@::legal_resolution],
+                                 ['*::legal_components', '*::legal_product',
+                                  '*::legal_priority', '*::legal_severity',
+                                  '*::legal_platform', '*::legal_opsys',
+                                  '*::legal_bug_status', '*::legal_resolution']);
+
+    print FID Data::Dumper->Dump([\@::settable_resolution, \%::proddesc,
+                                  \@::enterable_products, \%::prodmaxvotes],
+                                 ['*::settable_resolution', '*::proddesc',
+                                  '*::enterable_products', '*::prodmaxvotes']);
 
     if ($dotargetmilestone) {
         # reading target milestones in from the database - matthew@zeroknowledge.com
@@ -548,9 +473,12 @@ sub GenerateVersionTable {
             }
         }
 
-        print FID GenerateCode('%::target_milestone');
-        print FID GenerateCode('@::legal_target_milestone');
-        print FID GenerateCode('%::milestoneurl');
+        print FID Data::Dumper->Dump([\%::target_milestone,
+                                      \@::legal_target_milestone,
+                                      \%::milestoneurl],
+                                     ['*::target_milestone',
+                                      '*::legal_target_milestone',
+                                      '*::milestoneurl']);
     }
 
     SendSQL("SELECT id, name FROM keyworddefs ORDER BY name");
@@ -560,11 +488,13 @@ sub GenerateVersionTable {
         $name = lc($name);
         $::keywordsbyname{$name} = $id;
     }
-    print FID GenerateCode('@::legal_keywords');
-    print FID GenerateCode('%::keywordsbyname');
+
+    print FID Data::Dumper->Dump([\@::legal_keywords, \%::keywordsbyname],
+                                 ['*::legal_keywords', '*::keywordsbyname']);
 
     print FID "1;\n";
     close FID;
+
     rename $tmpname, "data/versioncache" || die "Can't rename $tmpname to versioncache";
     ChmodDataFile('data/versioncache', 0666);
 }
@@ -589,10 +519,6 @@ sub ModTime {
     return $mtime;
 }
 
-
-
-# This proc must be called before using legal_product or the versions array.
-
 $::VersionTableLoaded = 0;
 sub GetVersionTable {
     return if $::VersionTableLoaded;
@@ -617,7 +543,6 @@ sub GetVersionTable {
     $::VersionTableLoaded = 1;
 }
 
-
 # Validates a given username as a new username
 # returns 1 if valid, 0 if invalid
 sub ValidateNewUser {
@@ -1429,48 +1354,6 @@ sub RemoveVotes {
     }
 }
 
-sub Param ($) {
-    my ($value) = (@_);
-    if (! defined $::param{$value}) {
-        # Um, maybe we haven't sourced in the params at all yet.
-        if (stat("data/params")) {
-            # Write down and restore the version # here.  That way, we get 
-            # around anyone who maliciously tries to tweak the version number
-            # by editing the params file.  Not to mention that in 2.0, there 
-            # was a bug that wrote the version number out to the params file...
-            my $v = $::param{'version'};
-            require "data/params";
-            $::param{'version'} = $v;
-        }
-    }
-
-    if (! defined $::param{$value}) {
-        # Well, that didn't help.  Maybe it's a new param, and the user
-        # hasn't defined anything for it.  Try and load a default value
-        # for it.
-        require "defparams.pl";
-        WriteParams();
-    }
-
-    # If it's still not defined, we're pimped.
-    die "Can't find param named $value" if (! defined $::param{$value});
-
-    ## Check to make sure the entry in $::param_type is there; if we don't, we
-    ## get 'use of uninitialized constant' errors (see bug 162217). 
-    ## Interestingly  enough, placing this check in the die above causes 
-    ## deaths on some params (the "languages" param?) because they don't have
-    ## a type? Odd... seems like a bug to me... but what do I know? -jpr
-
-    if (defined $::param_type{$value} && $::param_type{$value} eq "m") {
-        my $valueList = eval($::param{$value});
-        return $valueList if (!($@) && ref($valueList) eq "ARRAY");
-        die "Multi-list param '$value' eval() failure ('$@'); data/params is horked";
-    }
-    else {
-        return $::param{$value};
-    }
-}
-
 # Take two comma or space separated strings and return what
 # values were removed from or added to the new one.
 sub DiffStrings {
@@ -1768,6 +1651,9 @@ $::vars =
     
     # User Agent - useful for detecting in templates
     'user_agent' => $ENV{'HTTP_USER_AGENT'} ,
+
+    # Bugzilla version
+    'VERSION' => $Bugzilla::Config::VERSION,
   };
 
 1;

move.pl

@@ -93,7 +93,7 @@ unless ($exporter =~ /($movers)/) {
 }
 
 my $xml = "";
-$xml .= Bug::XML_Header( Param("urlbase"), $::param{'version'}, 
+$xml .= Bug::XML_Header( Param("urlbase"), $Bugzilla::Config::VERSION, 
                          Param("maintainer"), $exporter );
 print "<P>\n";
 foreach my $id (split(/:/, $::FORM{'buglist'})) {

syncshadowdb

@@ -23,6 +23,10 @@
 
 use strict;
 
+use lib qw(.);
+
+use Bugzilla::Config qw(:DEFAULT :admin);
+
 require "globals.pl";
 require "defparams.pl";
 
@@ -106,7 +110,7 @@ if ($shutdown) {
     # Record the old shutdownhtml so it can be restored at the end (this will
     # only be an issue if we are called using the -force command line param)
     $wasshutdown = Param("shutdownhtml");
-    $::param{'shutdownhtml'} = $shutdown_msg;
+    SetParam('shutdownhtml', $shutdown_msg);
     WriteParams();
     # Now we need to wait for existing connections to this database to clear. We
     # do this by looking for connections to the main or shadow database using
@@ -143,7 +147,7 @@ if ($shutdown) {
             # this happening.
             Verbose ("*** Waited for 10 minutes and there were still active \n" .
                      "    connections to the bugzilla database.  Giving up.");
-            $::param{'shutdownhtml'} = $wasshutdown;
+            SetParam('shutdownhtml', $wasshutdown);
             WriteParams();
             exit;
         }
@@ -153,7 +157,7 @@ if ($shutdown) {
 
 my $wasusing = Param("queryagainstshadowdb");
 
-$::param{'queryagainstshadowdb'} = 1; # Force us to be able to use the
+SetParam('queryagainstshadowdb', 1);  # Force us to be able to use the
                                       # shadowdb, even if other processes
                                       # are not supposed to.
 
@@ -185,13 +189,13 @@ if (!$syncall) {
 if ($syncall) {
     Verbose("Syncing up the shadow database by copying entire database in.");
     if ($wasusing) {
-        $::param{'queryagainstshadowdb'} = 0;
+        SetParam('queryagainstshadowdb',0);
         WriteParams();
         if (! $shutdown) {
             Verbose("Disabled reading from the shadowdb. Sleeping 10 seconds to let other procs catch up.");
             sleep(10);
         }
-        $::param{'queryagainstshadowdb'} = 1;
+        SetParam('queryagainstshadowdb', 1);
     }
     my @tables;
     SendSQL("SHOW TABLES");
@@ -271,12 +275,12 @@ if ($syncall) {
     SendSQL("UNLOCK TABLES");
     if ($wasusing) {
         Verbose("Reenabling other processes to read from the shadow db");
-        $::param{'queryagainstshadowdb'} = 1;
+        SetParam('queryagainstshadowdb', 1);
         WriteParams();
     }
     if ($shutdown) {
         Verbose("Restoring the original shutdown message (if any)");
-        $::param{'shutdownhtml'} = $wasshutdown;
+        SetParam('shutdownhtml', $wasshutdown);
         WriteParams();
     }
     Verbose("OK, done.");

template/en/default/global/banner.html.tmpl

@@ -27,7 +27,7 @@
       <tr>
         <td>
           <font color="#FFFFFF" size="8"><center>
-            Bugzilla Version [% Param("version") %]
+            Bugzilla Version [% VERSION %]
           </center></font>
         </td>
       </tr>

xml.cgi

@@ -46,7 +46,7 @@ my $exporter = $::COOKIE{"Bugzilla_login"} || undef;
 my @ids = split (/[, ]+/, $::FORM{'id'});
 
 print "Content-type: text/xml\n\n";
-print Bug::XML_Header(Param("urlbase"), $::param{'version'}, 
+print Bug::XML_Header(Param("urlbase"), $Bugzilla::Config::VERSION, 
                       Param("maintainer"), $exporter);
 foreach my $id (@ids) {
   my $bug = new Bug(trim($id), $::userid);