diff options
| -rw-r--r-- | Bugzilla/Search.pm | 51 | ||||
| -rwxr-xr-x | buglist.cgi | 64 |
2 files changed, 66 insertions, 49 deletions
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 6bbf4ab42..e6682fcc4 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -738,6 +738,21 @@ sub boolean_charts_to_custom_search { } } +sub invalid_order_columns { + my ($self) = @_; + my @invalid_columns; + foreach my $order ($self->_input_order) { + next if defined $self->_validate_order_column($order); + push(@invalid_columns, $order); + } + return \@invalid_columns; +} + +sub order { + my ($self) = @_; + return $self->_valid_order; +} + ###################### # Internal Accessors # ###################### @@ -803,7 +818,7 @@ sub _extra_columns { my ($self) = @_; # Everything that's going to be in the ORDER BY must also be # in the SELECT. - $self->{extra_columns} ||= [ $self->_input_order_columns ]; + $self->{extra_columns} ||= [ $self->_valid_order_columns ]; return @{ $self->{extra_columns} }; } @@ -854,10 +869,32 @@ sub _sql_select { # The "order" that was requested by the consumer, exactly as it was # requested. sub _input_order { @{ $_[0]->{'order'} || [] } } -# The input order with just the column names, and no ASC or DESC. -sub _input_order_columns { +# Requested order with invalid values removed and old names translated +sub _valid_order { + my ($self) = @_; + return map { ($self->_validate_order_column($_)) } $self->_input_order; +} +# The valid order with just the column names, and no ASC or DESC. +sub _valid_order_columns { my ($self) = @_; - return map { (split_order_term($_))[0] } $self->_input_order; + return map { (split_order_term($_))[0] } $self->_valid_order; +} + +sub _validate_order_column { + my ($self, $order_item) = @_; + + # Translate old column names + my ($field, $direction) = split_order_term($order_item); + $field = translate_old_column($field); + + # Only accept valid columns + return if (!exists COLUMNS->{$field}); + + # Relevance column can be used only with one or more fulltext searches + return if ($field eq 'relevance' && !COLUMNS->{$field}->{name}); + + $direction = " $direction" if $direction; + return "$field$direction"; } # A hashref that describes all the special stuff that has to be done @@ -889,7 +926,7 @@ sub _sql_order_by { my ($self) = @_; if (!$self->{sql_order_by}) { my @order_by = map { $self->_translate_order_by_column($_) } - $self->_input_order; + $self->_valid_order; $self->{sql_order_by} = \@order_by; } return @{ $self->{sql_order_by} }; @@ -1033,7 +1070,7 @@ sub _select_order_joins { my @column_join = $self->_column_join($field); push(@joins, @column_join); } - foreach my $field ($self->_input_order_columns) { + foreach my $field ($self->_valid_order_columns) { my $join_info = $self->_special_order->{$field}->{join}; if ($join_info) { # Don't let callers modify SPECIAL_ORDER. @@ -1196,7 +1233,7 @@ sub _sql_group_by { # And all items from ORDER BY must be in the GROUP BY. The above loop # doesn't catch items that were put into the ORDER BY from SPECIAL_ORDER. - foreach my $column ($self->_input_order_columns) { + foreach my $column ($self->_valid_order_columns) { my $special_order = $self->_special_order->{$column}->{order}; next if !$special_order; push(@extra_group_by, @$special_order); diff --git a/buglist.cgi b/buglist.cgi index df421171d..f9c9a7720 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -698,69 +698,39 @@ if (!$order || $order =~ /^reuse/i) { } } +my @order_columns; if ($order) { # Convert the value of the "order" form field into a list of columns # by which to sort the results. ORDER: for ($order) { /^Bug Number$/ && do { - $order = "bug_id"; + @order_columns = ("bug_id"); last ORDER; }; /^Importance$/ && do { - $order = "priority,bug_severity"; + @order_columns = ("priority", "bug_severity"); last ORDER; }; /^Assignee$/ && do { - $order = "assigned_to,bug_status,priority,bug_id"; + @order_columns = ("assigned_to", "bug_status", "priority", + "bug_id"); last ORDER; }; /^Last Changed$/ && do { - $order = "changeddate,bug_status,priority,assigned_to,bug_id"; + @order_columns = ("changeddate", "bug_status", "priority", + "assigned_to", "bug_id"); last ORDER; }; do { - my (@order, @invalid_fragments); - - # A custom list of columns. Make sure each column is valid. - foreach my $fragment (split(/,/, $order)) { - $fragment = trim($fragment); - next unless $fragment; - my ($column_name, $direction) = split_order_term($fragment); - $column_name = translate_old_column($column_name); - - # Special handlings for certain columns - next if $column_name eq 'relevance' && !$fulltext; - - if (exists $columns->{$column_name}) { - $direction = " $direction" if $direction; - push(@order, "$column_name$direction"); - } - else { - push(@invalid_fragments, $fragment); - } - } - if (scalar @invalid_fragments) { - $vars->{'message'} = 'invalid_column_name'; - $vars->{'invalid_fragments'} = \@invalid_fragments; - } - - $order = join(",", @order); - # Now that we have checked that all columns in the order are valid, - # detaint the order string. - trick_taint($order) if $order; + # A custom list of columns. Bugzilla::Search will validate items. + @order_columns = split(/\s*,\s*/, $order); }; } } -if (!$order) { +if (!scalar @order_columns) { # DEFAULT - $order = "bug_status,priority,assigned_to,bug_id"; -} - -my @orderstrings = split(/,\s*/, $order); - -if ($fulltext and grep { /^relevance/ } @orderstrings) { - $vars->{'message'} = 'buglist_sorted_by_relevance' + @order_columns = ("bug_status", "priority", "assigned_to", "bug_id"); } # In the HTML interface, by default, we limit the returned results, @@ -774,9 +744,19 @@ if ($format->{'extension'} eq 'html' && !defined $params->param('limit')) { # Generate the basic SQL query that will be used to generate the bug list. my $search = new Bugzilla::Search('fields' => \@selectcolumns, 'params' => scalar $params->Vars, - 'order' => \@orderstrings); + 'order' => \@order_columns); my $query = $search->sql; $vars->{'search_description'} = $search->search_description; +$order = join(',', $search->order); + +if (scalar @{$search->invalid_order_columns}) { + $vars->{'message'} = 'invalid_column_name'; + $vars->{'invalid_fragments'} = $search->invalid_order_columns; +} + +if ($fulltext and grep { /^relevance/ } $search->order) { + $vars->{'message'} = 'buglist_sorted_by_relevance' +} # We don't want saved searches and other buglist things to save # our default limit. |