summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorByron Jones <bjones@mozilla.com>2013-12-05 19:04:08 +0100
committerByron Jones <bjones@mozilla.com>2013-12-05 19:04:08 +0100
commit7ea210f58788f68369aaa8e8b91a3016038c526f (patch)
tree5e96525d10a169e41dc2522b8cd48f4dac446396
parent1e95357d5127c40acc4bf6e7ed10739f30c6ca95 (diff)
downloadbugzilla-7ea210f58788f68369aaa8e8b91a3016038c526f.tar.gz
bugzilla-7ea210f58788f68369aaa8e8b91a3016038c526f.tar.xz
Bug 943636: SQL error in quicksearch API when providing just a bug ID
-rw-r--r--Bugzilla/Search.pm68
-rwxr-xr-xbuglist.cgi88
2 files changed, 89 insertions, 67 deletions
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index bda57716a..0c52553bb 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -38,7 +38,6 @@ use base qw(Exporter);
@Bugzilla::Search::EXPORT = qw(
IsValidQueryType
split_order_term
- translate_old_column
);
use Bugzilla::Error;
@@ -598,10 +597,10 @@ sub COLUMNS {
);
# Backward-compatibility for old field names. Goes new_name => old_name.
- # These are here and not in translate_old_column because the rest of the
+ # These are here and not in _translate_old_column because the rest of the
# code actually still uses the old names, while the fielddefs table uses
# the new names (which is not the case for the fields handled by
- # translate_old_column).
+ # _translate_old_column).
my %old_names = (
creation_ts => 'opendate',
delta_ts => 'changeddate',
@@ -744,8 +743,8 @@ sub data {
my $all_in_bugs_table = 1;
foreach my $field (@orig_fields) {
next if $self->COLUMNS->{$field}->{name} =~ /^bugs\.\w+$/;
- $all_in_bugs_table = 0;
$self->{fields} = ['bug_id'];
+ $all_in_bugs_table = 0;
last;
}
@@ -944,6 +943,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 #
######################
@@ -1009,7 +1023,7 @@ sub _extra_columns {
my ($self) = @_;
# Everything that's going to be in the ORDER BY must also be
# in the SELECT.
- push(@{ $self->{extra_columns} }, $self->_input_order_columns);
+ push(@{ $self->{extra_columns} }, $self->_valid_order_columns);
return @{ $self->{extra_columns} };
}
@@ -1079,10 +1093,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 = $self->_translate_old_column($field);
+
+ # Only accept valid columns
+ return if (!exists $self->COLUMNS->{$field});
+
+ # Relevance column can be used only with one or more fulltext searches
+ return if ($field eq 'relevance' && !$self->COLUMNS->{$field}->{name});
+
+ $direction = " $direction" if $direction;
+ return "$field$direction";
}
# A hashref that describes all the special stuff that has to be done
@@ -1114,7 +1150,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} };
@@ -1259,7 +1295,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.
@@ -1417,7 +1453,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);
@@ -3388,8 +3424,8 @@ sub split_order_term {
# Used to translate old SQL fragments from buglist.cgi's "order" argument
# into our modern field IDs.
-sub translate_old_column {
- my ($column) = @_;
+sub _translate_old_column {
+ my ($self, $column) = @_;
# All old SQL fragments have a period in them somewhere.
return $column if $column !~ /\./;
@@ -3403,9 +3439,9 @@ sub translate_old_column {
# If it doesn't match the regexps above, check to see if the old
# SQL fragment matches the SQL of an existing column
- foreach my $key (%{ COLUMNS() }) {
- next unless exists COLUMNS->{$key}->{name};
- return $key if COLUMNS->{$key}->{name} eq $column;
+ foreach my $key (%{ $self->COLUMNS }) {
+ next unless exists $self->COLUMNS->{$key}->{name};
+ return $key if $self->COLUMNS->{$key}->{name} eq $column;
}
return $column;
diff --git a/buglist.cgi b/buglist.cgi
index 9c7281822..f9cda72ec 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -56,18 +56,19 @@ my $cgi = Bugzilla->cgi;
my $dbh = Bugzilla->dbh;
my $template = Bugzilla->template;
my $vars = {};
-my $buffer = $cgi->query_string();
# We have to check the login here to get the correct footer if an error is
# thrown and to prevent a logged out user to use QuickSearch if 'requirelogin'
# is turned 'on'.
my $user = Bugzilla->login();
+$cgi->redirect_search_url();
+
+my $buffer = $cgi->query_string();
if (length($buffer) == 0) {
ThrowUserError("buglist_parameters_required");
}
-$cgi->redirect_search_url();
# Determine whether this is a quicksearch query.
my $searchstring = $cgi->param('quicksearch');
@@ -330,6 +331,7 @@ sub _close_standby_message {
}
}
+
################################################################################
# Command Execution
################################################################################
@@ -373,7 +375,7 @@ if ($cmdtype eq "dorem") {
# so that it can be modified easily.
$vars->{'searchname'} = $cgi->param('namedcmd');
if (!$cgi->param('sharer_id') ||
- $cgi->param('sharer_id') == Bugzilla->user->id) {
+ $cgi->param('sharer_id') == $user->id) {
$vars->{'searchtype'} = "saved";
$vars->{'search_id'} = $query_id;
}
@@ -690,69 +692,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,
@@ -766,9 +738,20 @@ 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,
'sharer' => $sharer_id);
+$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.
$params->delete('limit') if $vars->{'default_limited'};
@@ -974,10 +957,10 @@ $vars->{'order'} = $order;
$vars->{'caneditbugs'} = 1;
$vars->{'time_info'} = $time_info;
-if (!Bugzilla->user->in_group('editbugs')) {
+if (!$user->in_group('editbugs')) {
foreach my $product (keys %$bugproducts) {
my $prod = new Bugzilla::Product({name => $product});
- if (!Bugzilla->user->in_group('editbugs', $prod->id)) {
+ if (!$user->in_group('editbugs', $prod->id)) {
$vars->{'caneditbugs'} = 0;
last;
}
@@ -985,7 +968,7 @@ if (!Bugzilla->user->in_group('editbugs')) {
}
my @bugowners = keys %$bugowners;
-if (scalar(@bugowners) > 1 && Bugzilla->user->in_group('editbugs')) {
+if (scalar(@bugowners) > 1 && $user->in_group('editbugs')) {
my $suffix = Bugzilla->params->{'emailsuffix'};
map(s/$/$suffix/, @bugowners) if $suffix;
my $bugowners = join(",", @bugowners);
@@ -996,7 +979,10 @@ if (scalar(@bugowners) > 1 && Bugzilla->user->in_group('editbugs')) {
# the list more compact.
$vars->{'splitheader'} = $cgi->cookie('SPLITHEADER') ? 1 : 0;
-$vars->{'quip'} = GetQuip();
+if ($user->settings->{'display_quips'}->{'value'} eq 'on') {
+ $vars->{'quip'} = GetQuip();
+}
+
$vars->{'currenttime'} = localtime(time());
# See if there's only one product in all the results (or only one product
@@ -1014,7 +1000,7 @@ elsif (my @product_input = $cgi->param('product')) {
}
# We only want the template to use it if the user can actually
# enter bugs against it.
-if ($one_product && Bugzilla->user->can_enter_product($one_product)) {
+if ($one_product && $user->can_enter_product($one_product)) {
$vars->{'one_product'} = $one_product;
}
@@ -1034,7 +1020,7 @@ if ($dotweak && scalar @bugs) {
$vars->{'token'} = issue_session_token('buglist_mass_change');
Bugzilla->switch_to_shadow_db();
- $vars->{'products'} = Bugzilla->user->get_enterable_products;
+ $vars->{'products'} = $user->get_enterable_products;
$vars->{'platforms'} = get_legal_field_values('rep_platform');
$vars->{'op_sys'} = get_legal_field_values('op_sys');
$vars->{'priorities'} = get_legal_field_values('priority');