From 0087cc076fc48f24b73e8aa0935941a7b9a2b6d8 Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Fri, 16 Jul 2010 14:57:22 -0700 Subject: Bug 67036: Allow searching for product, component, etc. names that contain commas r=mkanat, a=mkanat (module owner) --- Bugzilla/Search.pm | 97 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 73 insertions(+), 24 deletions(-) diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 63dfe6d4c..563e75901 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -601,6 +601,19 @@ sub new { my $self = { @_ }; bless($self, $class); $self->{'user'} ||= Bugzilla->user; + + # There are certain behaviors of the CGI "Vars" hash that we don't want. + # In particular, if you put a single-value arrayref into it, later you + # get back out a string, which breaks anyexact charts (because they + # need arrays even for individual items, or we will re-trigger bug 67036). + # + # We can't just untie the hash--that would give us a hash with no values. + # We have to manually copy the hash into a new one, and we have to always + # do it, because there's no way to know if we were passed a tied hash + # or not. + my $params_in = $self->_params; + my %params = map { $_ => $params_in->{$_} } keys %$params_in; + $self->{params} = \%params; return $self; } @@ -1112,8 +1125,8 @@ sub _convert_special_params_to_chart_params { my @special_charts = $self->_special_charts(); - # first we delete any sign of "Chart #-1" from the HTML form hash - # since we want to guarantee the user didn't hide something here + # First we delete any sign of "Chart #-1" from the input parameters, + # because we want to guarantee the user didn't hide something there. my @badcharts = grep { /^(field|type|value)-1-/ } keys %$params; foreach my $field (@badcharts) { delete $params->{$field}; @@ -1457,20 +1470,37 @@ sub _handle_chart { my $field = $params->{"field$identifier"}; my $operator = $params->{"type$identifier"}; - my @values = $self->_param_array("value$identifier"); + my $value = $params->{"value$identifier"}; + + return if (!defined $field or !defined $operator or !defined $value); - return if (!defined $field or !defined $operator or !@values); - my $value = trim(join(',', @values)); - return if $value eq ''; + my $string_value; + if (ref $value eq 'ARRAY') { + # Trim input and ignore blank values. + @$value = map { trim($_) } @$value; + @$value = grep { defined $_ and $_ ne '' } @$value; + return if !@$value; + $string_value = join(',', @$value); + } + else { + return if $value eq ''; + $string_value = $value; + } + $self->_chart_fields->{$field} or ThrowCodeError("invalid_field_name", { field => $field }); trick_taint($field); - + # This is the field as you'd reference it in a SQL statement. my $full_field = $field =~ /\./ ? $field : "bugs.$field"; - my $quoted = $dbh->quote($value); - trick_taint($quoted); + # "value" and "quoted" are for search functions that always operate + # on a scalar string and never care if they were passed multiple + # parameters. If the user does pass multiple parameters, they will + # become a space-separated string for those search functions. + # + # all_values and all_quoted are for search functions that do operate + # on multiple values, like anyexact. my %search_args = ( chart_id => $sql_chart_id, @@ -1478,8 +1508,9 @@ sub _handle_chart { field => $field, full_field => $full_field, operator => $operator, - value => $value, - quoted => $quoted, + value => $string_value, + quoted => $dbh->quote($string_value), + all_values => $value, joins => [], having => [], ); @@ -1491,7 +1522,7 @@ sub _handle_chart { # do_search_function modified them. $self->search_description({ field => $field, type => $operator, - value => $value, term => $search_args{term}, + value => $string_value, term => $search_args{term}, }); return \%search_args; @@ -1710,6 +1741,29 @@ sub pronoun { return 0; } +sub _all_values { + my ($self, $args, $split_on) = @_; + $split_on ||= qr/[\s,]+/; + my $dbh = Bugzilla->dbh; + my $all_values = $args->{all_values}; + + my @array; + if (ref $all_values eq 'ARRAY') { + @array = @$all_values; + } + else { + @array = split($split_on, $all_values); + @array = map { trim($_) } @array; + @array = grep { defined $_ and $_ ne '' } @array; + } + + if ($args->{field} eq 'resolution') { + @array = map { $_ eq '---' ? '' : $_ } @array; + } + + return @array; +} + ###################### # Public Subroutines # ###################### @@ -2582,6 +2636,10 @@ sub _multiselect_nonchanged { push(@$joins, { table => "bug_$field", as => $table }); } +############################### +# Standard Operator Functions # +############################### + sub _simple_operator { my ($self, $args) = @_; my ($full_field, $quoted, $operator) = @@ -2634,24 +2692,15 @@ sub _notregexp { sub _anyexact { my ($self, $args) = @_; - my ($field, $value, $full_field) = @$args{qw(field value full_field)}; + my ($field, $full_field) = @$args{qw(field full_field)}; my $dbh = Bugzilla->dbh; - my @list; - foreach my $word (split(/,/, $value)) { - $word = trim($word); - if ($word eq "---" && $field eq 'resolution') { - $word = ""; - } - my $quoted_word = $dbh->quote($word); - trick_taint($quoted_word); - push(@list, $quoted_word); - } + my @list = $self->_all_values($args, ','); + @list = map { $dbh->quote($_) } @list; if (@list) { $args->{term} = $dbh->sql_in($full_field, \@list); } - # XXX Perhaps if it's all commas, we should just throw an error. else { $args->{term} = ''; } -- cgit v1.2.3-24-g4f1b