Big 163291 - Move utility funcs into a module

r=preed x2

7 files changed, 294 insertions, 121 deletions

diff --git a/Bug.pm b/Bug.pm
index d73bc536f..51b51cec9 100755
--- a/Bug.pm
+++ b/Bug.pm
@@ -33,6 +33,8 @@ package Bug;
 use CGI::Carp qw(fatalsToBrowser);
 my %ok_field;
 
+use Bugzilla::Util;
+
 for my $key (qw (bug_id alias product version rep_platform op_sys bug_status 
                 resolution priority bug_severity component assigned_to
                 reporter bug_file_loc short_desc target_milestone 
@@ -78,7 +80,7 @@ sub initBug  {
   $bug_id = &::BugAliasToID($bug_id) if $bug_id !~ /^[1-9][0-9]*$/;
   
   my $old_bug_id = $bug_id;
-  if ((! defined $bug_id) || (!$bug_id) || (!&::detaint_natural($bug_id))) {
+  if ((! defined $bug_id) || (!$bug_id) || (!detaint_natural($bug_id))) {
       # no bug number given or the alias didn't match a bug
       $self->{'bug_id'} = $old_bug_id;
       $self->{'error'} = "InvalidBugId";
@@ -387,7 +389,7 @@ sub CanChangeField {
     if ($oldvalue eq $newvalue) {
         return 1;
     }
-    if (&::trim($oldvalue) eq &::trim($newvalue)) {
+    if (trim($oldvalue) eq trim($newvalue)) {
         return 1;
     }
     if ($f =~ /^longdesc/) {
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index d73bc536f..51b51cec9 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -33,6 +33,8 @@ package Bug;
 use CGI::Carp qw(fatalsToBrowser);
 my %ok_field;
 
+use Bugzilla::Util;
+
 for my $key (qw (bug_id alias product version rep_platform op_sys bug_status 
                 resolution priority bug_severity component assigned_to
                 reporter bug_file_loc short_desc target_milestone 
@@ -78,7 +80,7 @@ sub initBug  {
   $bug_id = &::BugAliasToID($bug_id) if $bug_id !~ /^[1-9][0-9]*$/;
   
   my $old_bug_id = $bug_id;
-  if ((! defined $bug_id) || (!$bug_id) || (!&::detaint_natural($bug_id))) {
+  if ((! defined $bug_id) || (!$bug_id) || (!detaint_natural($bug_id))) {
       # no bug number given or the alias didn't match a bug
       $self->{'bug_id'} = $old_bug_id;
       $self->{'error'} = "InvalidBugId";
@@ -387,7 +389,7 @@ sub CanChangeField {
     if ($oldvalue eq $newvalue) {
         return 1;
     }
-    if (&::trim($oldvalue) eq &::trim($newvalue)) {
+    if (trim($oldvalue) eq trim($newvalue)) {
         return 1;
     }
     if ($f =~ /^longdesc/) {
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index e73c6660e..257b7656d 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -35,6 +35,8 @@ use vars qw($userid $usergroupset);
 
 package Bugzilla::Search;
 
+use Bugzilla::Util;
+
 # Create a new Search
 sub new {
     my $invocant = shift;
@@ -66,33 +68,33 @@ sub init {
     my @andlist;
 
     # First, deal with all the old hard-coded non-chart-based poop.
-    if (&::lsearch($fieldsref, 'map_assigned_to.login_name') >= 0) {
+    if (lsearch($fieldsref, 'map_assigned_to.login_name') >= 0) {
         push @supptables, "profiles AS map_assigned_to";
         push @wherepart, "bugs.assigned_to = map_assigned_to.userid";
     }
 
-    if (&::lsearch($fieldsref, 'map_reporter.login_name') >= 0) {
+    if (lsearch($fieldsref, 'map_reporter.login_name') >= 0) {
         push @supptables, "profiles AS map_reporter";
         push @wherepart, "bugs.reporter = map_reporter.userid";
     }
 
-    if (&::lsearch($fieldsref, 'map_qa_contact.login_name') >= 0) {
+    if (lsearch($fieldsref, 'map_qa_contact.login_name') >= 0) {
         push @supptables, "LEFT JOIN profiles map_qa_contact ON bugs.qa_contact = map_qa_contact.userid";
     }
 
-    if (&::lsearch($fieldsref, 'map_products.name') >= 0) {
+    if (lsearch($fieldsref, 'map_products.name') >= 0) {
         push @supptables, "products AS map_products";
         push @wherepart, "bugs.product_id = map_products.id";
     }
 
-    if (&::lsearch($fieldsref, 'map_components.name') >= 0) {
+    if (lsearch($fieldsref, 'map_components.name') >= 0) {
         push @supptables, "components AS map_components";
         push @wherepart, "bugs.component_id = map_components.id";
     }
 
     my $minvotes;
     if (defined $F{'votes'}) {
-        my $c = &::trim($F{'votes'});
+        my $c = trim($F{'votes'});
         if ($c ne "") {
             if ($c !~ /^[0-9]*$/) {
                 $::vars->{'value'} = $c;
@@ -116,7 +118,7 @@ sub init {
                         "target_milestone", "groupset");
 
     foreach my $field (keys %F) {
-        if (&::lsearch(\@legal_fields, $field) != -1) {
+        if (lsearch(\@legal_fields, $field) != -1) {
             push(@specialchart, [$field, "anyexact",
                                  join(',', @{$M{$field}})]);
         }
@@ -146,7 +148,7 @@ sub init {
         if (!defined ($F{"email$id"})) {
             next;
         }
-        my $email = &::trim($F{"email$id"});
+        my $email = trim($F{"email$id"});
         if ($email eq "") {
             next;
         }
@@ -154,7 +156,7 @@ sub init {
         if ($type eq "exact") {
             $type = "anyexact";
             foreach my $name (split(',', $email)) {
-                $name = &::trim($name);
+                $name = trim($name);
                 if ($name) {
                     &::DBNameToIdAndCheck($name);
                 }
@@ -186,7 +188,7 @@ sub init {
 
 
     if (defined $F{'changedin'}) {
-        my $c = &::trim($F{'changedin'});
+        my $c = trim($F{'changedin'});
         if ($c ne "") {
             if ($c !~ /^[0-9]*$/) {
                 $::vars->{'value'} = $c;
@@ -200,14 +202,14 @@ sub init {
     my $ref = $M{'chfield'};
 
     if (defined $ref) {
-        my $which = &::lsearch($ref, "[Bug creation]");
+        my $which = lsearch($ref, "[Bug creation]");
         if ($which >= 0) {
             splice(@$ref, $which, 1);
             push(@specialchart, ["creation_ts", "greaterthan",
                                  SqlifyDate($F{'chfieldfrom'})]);
             my $to = $F{'chfieldto'};
             if (defined $to) {
-                $to = &::trim($to);
+                $to = trim($to);
                 if ($to ne "" && $to !~ /^now$/i) {
                     push(@specialchart, ["creation_ts", "lessthan",
                                          SqlifyDate($to)]);
@@ -229,7 +231,7 @@ sub init {
              &::SqlQuote(SqlifyDate($F{'chfieldfrom'})));
         my $to = $F{'chfieldto'};
         if (defined $to) {
-            $to = &::trim($to);
+            $to = trim($to);
             if ($to ne "" && $to !~ /^now$/i) {
                 push(@wherepart, "actcheck.bug_when <= " .
                      &::SqlQuote(SqlifyDate($to)));
@@ -237,7 +239,7 @@ sub init {
         }
         my $value = $F{'chfieldvalue'};
         if (defined $value) {
-            $value = &::trim($value);
+            $value = trim($value);
             if ($value ne "") {
                 push(@wherepart, "actcheck.added = " .
                      &::SqlQuote($value))
@@ -248,7 +250,7 @@ sub init {
     foreach my $f ("short_desc", "long_desc", "bug_file_loc",
                    "status_whiteboard") {
         if (defined $F{$f}) {
-            my $s = &::trim($F{$f});
+            my $s = trim($F{$f});
             if ($s ne "") {
                 my $n = $f;
                 my $q = &::SqlQuote($s);
@@ -731,14 +733,14 @@ sub init {
                 $t = $F{"type$chart-$row-$col"} || "noop";
                 $v = $F{"value$chart-$row-$col"};
                 $v = "" if !defined $v;
-                $v = &::trim($v);
+                $v = trim($v);
                 if ($f eq "noop" || $t eq "noop" || $v eq "") {
                     next;
                 }
                 # 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 " . &::html_quote($f) . " as a field name.  " .
+                    my $errstr = "Can't use " . html_quote($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>";
@@ -749,7 +751,7 @@ sub init {
                 # This is either from the internal chart (in which case we
                 # already know about it), or it was in %chartfields, so it is
                 # a valid field name, which means that its ok.
-                &::trick_taint($f);
+                trick_taint($f);
                 $q = &::SqlQuote($v);
                 my $func;
                 $term = undef;
@@ -805,7 +807,7 @@ sub init {
     $query = &::SelectVisible($query, $::userid, $::usergroupset);
 
     if ($debug) {
-        print "<p><code>" . &::value_quote($query) . "</code></p>\n";
+        print "<p><code>" . value_quote($query) . "</code></p>\n";
         exit;
     }
     
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
new file mode 100644
index 000000000..aabaabb88
--- /dev/null
+++ b/Bugzilla/Util.pm
@@ -0,0 +1,260 @@
+# -*- 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>
+#                 Dan Mosedale <dmose@mozilla.org>
+#                 Jake <jake@acutex.net>
+#                 Bradley Baetz <bbaetz@student.usyd.edu.au>
+#                 Christopher Aillon <christopher@aillon.com>
+
+package Bugzilla::Util;
+
+=head1 NAME
+
+Bugzilla::Util - Generic utility functions for bugzilla
+
+=head1 SYNOPSIS
+
+  use Bugzilla::Util;
+
+  # Functions for dealing with variable tainting
+  $rv = is_tainted($var);
+  trick_taint($var);
+  detaint_natural($var);
+
+  # Functions for quoting
+  html_quote($var);
+  value_quote($var);
+
+  # Functions for searching
+  $loc = lsearch(\@arr, $val);
+  $val = max($a, $b, $c);
+  $val = min($a, $b, $c);
+
+  # Functions for trimming variables
+  $val = trim(" abc ");
+
+=head1 DESCRIPTION
+
+This package contains various utility functions which do not belong anywhere
+else.
+
+B<It is not intended as a general dumping group for something which
+people feel might be useful somewhere, someday>. Do not add methods to this
+package unless it is intended to be used for a significant number of files,
+and it does not belong anywhere else.
+
+=cut
+
+use base qw(Exporter);
+@Bugzilla::Util::EXPORT = qw(is_tainted trick_taint detaint_natural
+                             html_quote value_quote
+                             lsearch max min
+                             trim);
+
+use strict;
+use diagnostics;
+
+=head1 FUNCTIONS
+
+This package provides several types of routines:
+
+=head2 Tainting
+
+Several functions are available to deal with tainted variables. B<Use these
+with care> to avoid security holes.
+
+=over 4
+
+=item C<is_tainted>
+
+Determines whether a particular variable is tainted
+
+=cut
+
+# This is from the perlsec page, slightly modifed to remove a warning
+# From that page:
+#      This function makes use of the fact that the presence of
+#      tainted data anywhere within an expression renders the
+#      entire expression tainted.
+# Don't ask me how it works...
+sub is_tainted {
+    return not eval { my $foo = join('',@_), kill 0; 1; };
+}
+
+=item C<trick_taint($val)>
+
+Tricks perl into untainting a particular variable.
+
+Use trick_taint() when you know that there is no way that the data
+in a scalar can be tainted, but taint mode still bails on it.
+
+B<WARNING!! Using this routine on data that really could be tainted defeats
+the purpose of taint mode.  It should only be used on variables that have been sanity checked in some way and have been determined to be OK.>
+
+=cut
+
+sub trick_taint {
+    $_[0] =~ /^(.*)$/s;
+    $_[0] = $1;
+    return (defined($_[0]));
+}
+
+=item C<detaint_natural($num)>
+
+This routine detaints a natural number. It returns a true value if the
+value passed in was a valid natural number, else it returns false. You
+B<MUST> check the result of this routine to avoid security holes.
+
+=cut
+
+sub detaint_natural {
+    $_[0] =~ /^(\d+)$/;
+    $_[0] = $1;
+    return (defined($_[0]));
+}
+
+=back
+
+=head2 Quoting
+
+Some values may need to be quoted from perl. However, this should in general
+be done in the template where possible.
+
+=over 4
+
+=item C<html_quote($val)>
+
+Returns a value quoted for use in HTML, with &, E<lt>, E<gt>, and E<34> being
+replaced with their appropriate HTML entities.
+
+=cut
+
+sub html_quote {
+    my ($var) = (@_);
+    $var =~ s/\&/\&amp;/g;
+    $var =~ s/</\&lt;/g;
+    $var =~ s/>/\&gt;/g;
+    $var =~ s/\"/\&quot;/g;
+    return $var;
+}
+
+=item C<value_quote($val)>
+
+As well as escaping html like C<html_quote>, this routine converts newlines
+into &#013;, suitable for use in html attributes.
+
+=cut
+
+sub value_quote {
+    my ($var) = (@_);
+    $var =~ s/\&/\&amp;/g;
+    $var =~ s/</\&lt;/g;
+    $var =~ s/>/\&gt;/g;
+    $var =~ s/\"/\&quot;/g;
+    # See bug http://bugzilla.mozilla.org/show_bug.cgi?id=4928 for 
+    # explanaion of why bugzilla does this linebreak substitution. 
+    # This caused form submission problems in mozilla (bug 22983, 32000).
+    $var =~ s/\r\n/\&#013;/g;
+    $var =~ s/\n\r/\&#013;/g;
+    $var =~ s/\r/\&#013;/g;
+    $var =~ s/\n/\&#013;/g;
+    return $var;
+}
+
+=back
+
+=head2 Searching
+
+Functions for searching within a set of values.
+
+=over 4
+
+=item C<lsearch($list, $item)>
+
+Returns the position of C<$item> in C<$list>. C<$list> must be a list
+reference.
+
+If the item is not in the list, returns -1.
+
+=cut
+
+sub lsearch {
+    my ($list,$item) = (@_);
+    my $count = 0;
+    foreach my $i (@$list) {
+        if ($i eq $item) {
+            return $count;
+        }
+        $count++;
+    }
+    return -1;
+}
+
+=item C<max($a, $b, ...)>
+
+Returns the maximum from a set of values.
+
+=cut
+
+sub max {
+    my $max = shift(@_);
+    foreach my $val (@_) {
+        $max = $val if $val > $max;
+    }
+    return $max;
+}
+
+=item C<min($a, $b, ...)>
+
+Returns the minimum from a set of values.
+
+=cut
+
+sub min {
+    my $min = shift(@_);
+    foreach my $val (@_) {
+        $min = $val if $val < $min;
+    }
+    return $min;
+}
+
+=back
+
+=head2 Trimming
+
+=over 4
+
+=item C<trim($str)>
+
+Removes any leading or trailing whitespace from a string. This routine does not
+modify the existing string.
+
+=cut
+
+sub trim {
+    my ($str) = @_;
+    $str =~ s/^\s+//g;
+    $str =~ s/\s+$//g;
+    return $str;
+}
+
+=back
+
+=cut
diff --git a/CGI.pl b/CGI.pl
index ec4ddedfb..2a10a335a 100644
--- a/CGI.pl
+++ b/CGI.pl
@@ -33,6 +33,8 @@ use lib ".";
 
 # use Carp;                       # for confess
 
+use Bugzilla::Util;
+
 # 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. 
 # We want to check for the existence of the LDAP modules here.
@@ -334,31 +336,6 @@ sub ValidateComment {
     }
 }
 
-sub html_quote {
-    my ($var) = (@_);
-    $var =~ s/\&/\&amp;/g;
-    $var =~ s/</\&lt;/g;
-    $var =~ s/>/\&gt;/g;
-    $var =~ s/"/\&quot;/g;
-    return $var;
-}
-
-sub value_quote {
-    my ($var) = (@_);
-    $var =~ s/\&/\&amp;/g;
-    $var =~ s/</\&lt;/g;
-    $var =~ s/>/\&gt;/g;
-    $var =~ s/"/\&quot;/g;
-    # See bug http://bugzilla.mozilla.org/show_bug.cgi?id=4928 for 
-    # explanaion of why bugzilla does this linebreak substitution. 
-    # This caused form submission problems in mozilla (bug 22983, 32000).
-    $var =~ s/\r\n/\&#013;/g;
-    $var =~ s/\n\r/\&#013;/g;
-    $var =~ s/\r/\&#013;/g;
-    $var =~ s/\n/\&#013;/g;
-    return $var;
-}
-
 # Adds <link> elements for bug lists. These can be inserted into the header by
 # using the "header_html" parameter to PutHeader, which inserts an arbitrary
 # string into the header. This function is currently used only in
diff --git a/checksetup.pl b/checksetup.pl
index 728db3798..34b487443 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -912,7 +912,6 @@ END
         {
          strike => sub { return $_; } ,
          js => sub { return $_; },
-         html => sub { return $_; },
          html_linebreak => sub { return $_; },
          url_quote => sub { return $_; },
         },
diff --git a/globals.pl b/globals.pl
index 100c3ae2f..9f15976b2 100644
--- a/globals.pl
+++ b/globals.pl
@@ -28,6 +28,8 @@
 use diagnostics;
 use strict;
 
+use Bugzilla::Util;
+
 # Shut up misguided -w warnings about "used only once".  For some reason,
 # "use vars" chokes on me when I try it here.
 
@@ -230,16 +232,6 @@ sub SqlLog {
     }
 }
 
-# This is from the perlsec page, slightly modifed to remove a warning
-# From that page:
-#      This function makes use of the fact that the presence of
-#      tainted data anywhere within an expression renders the
-#      entire expression tainted.
-# Don't ask me how it works...
-sub is_tainted {
-    return not eval { my $foo = join('',@_), kill 0; 1; };
-}
-
 sub SendSQL {
     my ($str, $dontshadow) = (@_);
 
@@ -353,21 +345,6 @@ sub GetFieldID {
     die "Unknown field id: $f" if !$fieldid;
     return $fieldid;
 }
-        
-
-
-
-sub lsearch {
-    my ($list,$item) = (@_);
-    my $count = 0;
-    foreach my $i (@$list) {
-        if ($i eq $item) {
-            return $count;
-        }
-        $count++;
-    }
-    return -1;
-}
 
 # Generate a string which, when later interpreted by the Perl compiler, will
 # be the same as the given string.
@@ -993,24 +970,6 @@ sub get_component_name {
     return $comp;
 }
 
-# Use trick_taint() when you know that there is no way that the data
-# in a scalar can be tainted, but taint mode still bails on it.
-# WARNING!! Using this routine on data that really could be tainted
-#           defeats the purpose of taint mode.  It should only be
-#           used on variables that cannot be touched by users.
-
-sub trick_taint {
-    $_[0] =~ /^(.*)$/s;
-    $_[0] = $1;
-    return (defined($_[0]));
-}
-
-sub detaint_natural {
-    $_[0] =~ /^(\d+)$/;
-    $_[0] = $1;
-    return (defined($_[0]));
-}
-
 # This routine quoteUrls contains inspirations from the HTML::FromText CPAN
 # module by Gareth Rees <garethr@cre.canon.co.uk>.  It has been heavily hacked,
 # all that is really recognizable from the original is bits of the regular
@@ -1541,32 +1500,6 @@ sub PerformSubsts {
     return $str;
 }
 
-# Min and max routines.
-sub min {
-    my $min = shift(@_);
-    foreach my $val (@_) {
-        $min = $val if $val < $min;
-    }
-    return $min;
-}
-
-sub max {
-    my $max = shift(@_);
-    foreach my $val (@_) {
-        $max = $val if $val > $max;
-    }
-    return $max;
-}
-
-# Trim whitespace from front and back.
-
-sub trim {
-    my ($str) = @_;
-    $str =~ s/^\s+//g;
-    $str =~ s/\s+$//g;
-    return $str;
-}
-
 ###############################################################################
 # Global Templatization Code
 
@@ -1614,8 +1547,6 @@ $::template ||= Template->new(
             $var =~ s/\r/\\r/g; 
             return $var;
         } , 
-        
-        html => \&html_quote , 
 
         # HTML collapses newlines in element attributes to a single space,
         # so form elements which may have whitespace (ie comments) need
@@ -1821,7 +1752,7 @@ $::vars =
     'PerformSubsts' => \&PerformSubsts ,
 
     # Generic linear search function
-    'lsearch' => \&lsearch ,
+    'lsearch' => \&Bugzilla::Util::lsearch ,
 
     # UserInGroup - you probably want to cache this
     'UserInGroup' => \&UserInGroup ,