From 0f27f72d85d5889e482ab8c425715519ad6c7619 Mon Sep 17 00:00:00 2001 From: "bbaetz%student.usyd.edu.au" Date: Mon, 19 Aug 2002 13:59:45 +0000 Subject: [PATCH] Big 163291 - Move utility funcs into a module r=preed x2 --- webtools/bugzilla/Bug.pm | 6 +- webtools/bugzilla/Bugzilla/Bug.pm | 6 +- webtools/bugzilla/Bugzilla/Search.pm | 40 +++-- webtools/bugzilla/Bugzilla/Util.pm | 260 +++++++++++++++++++++++++++ webtools/bugzilla/CGI.pl | 27 +-- webtools/bugzilla/checksetup.pl | 1 - webtools/bugzilla/globals.pl | 75 +------- 7 files changed, 294 insertions(+), 121 deletions(-) create mode 100644 webtools/bugzilla/Bugzilla/Util.pm diff --git a/webtools/bugzilla/Bug.pm b/webtools/bugzilla/Bug.pm index d73bc536f9f4..51b51cec90a0 100755 --- a/webtools/bugzilla/Bug.pm +++ b/webtools/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/webtools/bugzilla/Bugzilla/Bug.pm b/webtools/bugzilla/Bugzilla/Bug.pm index d73bc536f9f4..51b51cec90a0 100644 --- a/webtools/bugzilla/Bugzilla/Bug.pm +++ b/webtools/bugzilla/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/webtools/bugzilla/Bugzilla/Search.pm b/webtools/bugzilla/Bugzilla/Search.pm index e73c6660e299..257b7656d8ec 100644 --- a/webtools/bugzilla/Bugzilla/Search.pm +++ b/webtools/bugzilla/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 "

" . &::value_quote($query) . "

\n"; + print "

" . value_quote($query) . "

\n"; exit; } diff --git a/webtools/bugzilla/Bugzilla/Util.pm b/webtools/bugzilla/Bugzilla/Util.pm new file mode 100644 index 000000000000..aabaabb888d5 --- /dev/null +++ b/webtools/bugzilla/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 +# Dan Mosedale +# Jake +# Bradley Baetz +# Christopher Aillon + +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. 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 to avoid security holes. + +=over 4 + +=item C + +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 + +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 + +=cut + +sub trick_taint { + $_[0] =~ /^(.*)$/s; + $_[0] = $1; + return (defined($_[0])); +} + +=item C + +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 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 + +Returns a value quoted for use in HTML, with &, E, E, and E<34> being +replaced with their appropriate HTML entities. + +=cut + +sub html_quote { + my ($var) = (@_); + $var =~ s/\&/\&/g; + $var =~ s//\>/g; + $var =~ s/\"/\"/g; + return $var; +} + +=item C + +As well as escaping html like C, this routine converts newlines +into , suitable for use in html attributes. + +=cut + +sub value_quote { + my ($var) = (@_); + $var =~ s/\&/\&/g; + $var =~ s//\>/g; + $var =~ s/\"/\"/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/\ /g; + $var =~ s/\n\r/\ /g; + $var =~ s/\r/\ /g; + $var =~ s/\n/\ /g; + return $var; +} + +=back + +=head2 Searching + +Functions for searching within a set of values. + +=over 4 + +=item C + +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 + +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 + +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 + +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/webtools/bugzilla/CGI.pl b/webtools/bugzilla/CGI.pl index ec4ddedfbba4..2a10a335a131 100644 --- a/webtools/bugzilla/CGI.pl +++ b/webtools/bugzilla/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/\&/\&/g; - $var =~ s//\>/g; - $var =~ s/"/\"/g; - return $var; -} - -sub value_quote { - my ($var) = (@_); - $var =~ s/\&/\&/g; - $var =~ s//\>/g; - $var =~ s/"/\"/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/\ /g; - $var =~ s/\n\r/\ /g; - $var =~ s/\r/\ /g; - $var =~ s/\n/\ /g; - return $var; -} - # Adds 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/webtools/bugzilla/checksetup.pl b/webtools/bugzilla/checksetup.pl index 728db37981ee..34b4874437ea 100755 --- a/webtools/bugzilla/checksetup.pl +++ b/webtools/bugzilla/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/webtools/bugzilla/globals.pl b/webtools/bugzilla/globals.pl index 100c3ae2f0da..9f15976b26ee 100644 --- a/webtools/bugzilla/globals.pl +++ b/webtools/bugzilla/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 . 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 ,