Commit 89b15c8f authored by mkanat%bugzilla.org's avatar mkanat%bugzilla.org

Bug 491467: Make Search.pm and buglist.cgi consistently take column ids for the…

Bug 491467: Make Search.pm and buglist.cgi consistently take column ids for the "fields" and "order" arguments, to prevent problems with using SQL fragments in the order and columnlist. Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=wicked, a=mkanat
parent 6a51c4c3
......@@ -52,7 +52,6 @@ use Storable qw(dclone);
use constant BLOB_TYPE => DBI::SQL_BLOB;
use constant ISOLATION_LEVEL => 'REPEATABLE READ';
use constant GROUPBY_REGEXP => '(?:.*\s+AS\s+|SUBSTRING\()?(\w+(\.\w+)?)(?:\s+(ASC|DESC|FROM\s+.+))?$';
# Set default values for what used to be the enum types. These values
# are no longer stored in localconfig. If we are upgrading from a
......
......@@ -52,7 +52,6 @@ use base qw(Bugzilla::DB);
use constant EMPTY_STRING => '__BZ_EMPTY_STR__';
use constant ISOLATION_LEVEL => 'READ COMMITTED';
use constant BLOB_TYPE => { ora_type => ORA_BLOB };
use constant GROUPBY_REGEXP => '((CASE\s+WHEN.+END)|(SUBSTR.+\))|(TO_CHAR\(.+\))|(\(SCORE.+\))|(\(MATCH.+\))|(\w+(\.\w+)?))(\s+AS\s+)?(.*)?$';
sub new {
my ($class, $user, $pass, $host, $dbname, $port) = @_;
......
......@@ -561,7 +561,7 @@ sub CollectSeriesData {
# login name or a renamed product or component, etc.
eval {
my $search = new Bugzilla::Search('params' => $cgi,
'fields' => ["bugs.bug_id"],
'fields' => ["bug_id"],
'user' => $user);
my $sql = $search->getSQL();
$data = $shadow_dbh->selectall_arrayref($sql);
......
......@@ -199,14 +199,14 @@ if (scalar(%count)) {
$params->param('product', join(',', @query_products));
}
my $query = new Bugzilla::Search('fields' => [qw(bugs.bug_id
map_components.name
bugs.bug_severity
bugs.op_sys
bugs.target_milestone
bugs.short_desc
bugs.bug_status
bugs.resolution
my $query = new Bugzilla::Search('fields' => [qw(bug_id
component
bug_severity
op_sys
target_milestone
short_desc
bug_status
resolution
)
],
'params' => $params,
......
......@@ -102,53 +102,42 @@ else {
}
}
my %columns;
$columns{'bug_severity'} = "bugs.bug_severity";
$columns{'priority'} = "bugs.priority";
$columns{'rep_platform'} = "bugs.rep_platform";
$columns{'assigned_to'} = "map_assigned_to.login_name";
$columns{'reporter'} = "map_reporter.login_name";
$columns{'qa_contact'} = "map_qa_contact.login_name";
$columns{'bug_status'} = "bugs.bug_status";
$columns{'resolution'} = "bugs.resolution";
$columns{'component'} = "map_components.name";
$columns{'product'} = "map_products.name";
$columns{'classification'} = "map_classifications.name";
$columns{'version'} = "bugs.version";
$columns{'op_sys'} = "bugs.op_sys";
$columns{'votes'} = "bugs.votes";
$columns{'keywords'} = "bugs.keywords";
$columns{'target_milestone'} = "bugs.target_milestone";
# Single-select fields are also accepted as valid column names.
my @single_select_fields =
grep { $_->type == FIELD_TYPE_SINGLE_SELECT } Bugzilla->active_custom_fields;
foreach my $custom_field (@single_select_fields) {
my $field_name = $custom_field->name;
$columns{$field_name} = "bugs.$field_name";
}
# One which means "nothing". Any number would do, really. It just gets SELECTed
# so that we always select 3 items in the query.
$columns{''} = "42217354";
# Valid bug fields that can be reported on.
my @columns = qw(
assigned_to
reporter
qa_contact
component
classification
version
votes
keywords
target_milestone
);
# Single-select fields (custom or not) are also accepted as valid.
my @single_selects = Bugzilla->get_fields({ type => FIELD_TYPE_SINGLE_SELECT,
obsolete => 0 });
push(@columns, map { $_->name } @single_selects);
my %valid_columns = map { $_ => 1 } @columns;
# Validate the values in the axis fields or throw an error.
!$row_field
|| ($columns{$row_field} && trick_taint($row_field))
|| ($valid_columns{$row_field} && trick_taint($row_field))
|| ThrowCodeError("report_axis_invalid", {fld => "x", val => $row_field});
!$col_field
|| ($columns{$col_field} && trick_taint($col_field))
|| ($valid_columns{$col_field} && trick_taint($col_field))
|| ThrowCodeError("report_axis_invalid", {fld => "y", val => $col_field});
!$tbl_field
|| ($columns{$tbl_field} && trick_taint($tbl_field))
|| ($valid_columns{$tbl_field} && trick_taint($tbl_field))
|| ThrowCodeError("report_axis_invalid", {fld => "z", val => $tbl_field});
my @axis_fields = ($row_field, $col_field, $tbl_field);
my @selectnames = map($columns{$_}, @axis_fields);
my @axis_fields = ($row_field || EMPTY_COLUMN,
$col_field || EMPTY_COLUMN,
$tbl_field || EMPTY_COLUMN);
# Clone the params, so that Bugzilla::Search can modify them
my $params = new Bugzilla::CGI($cgi);
my $search = new Bugzilla::Search('fields' => \@selectnames,
my $search = new Bugzilla::Search('fields' => \@axis_fields,
'params' => $params);
my $query = $search->getSQL();
......@@ -179,9 +168,9 @@ foreach my $result (@$results) {
$col = ' ' if ($col eq '');
$tbl = ' ' if ($tbl eq '');
$row = "" if ($row eq $columns{''});
$col = "" if ($col eq $columns{''});
$tbl = "" if ($tbl eq $columns{''});
$row = "" if ($row eq EMPTY_COLUMN);
$col = "" if ($col eq EMPTY_COLUMN);
$tbl = "" if ($tbl eq EMPTY_COLUMN);
# account for the fact that names may start with '_' or '.'. Change this
# so the template doesn't hide hash elements with those keys
......
......@@ -87,11 +87,11 @@
[% END %]
<th colspan="[% splitheader ? 2 : 1 %]" class="first-child">
[% desc = '' %]
[% IF (om = order.match("^bugs\.bug_id( desc)?")) %]
[% desc = ' desc' IF NOT om.0 %]
[% IF (om = order.match("^bug_id( DESC)?")) %]
[% desc = ' DESC' IF NOT om.0 %]
[% END %]
<a href="buglist.cgi?
[% urlquerypart FILTER html %]&amp;order=bugs.bug_id[% desc FILTER url_quote %]
[% urlquerypart FILTER html %]&amp;order=bug_id[% desc FILTER url_quote %]
[%-#%]&amp;query_based_on=
[% defaultsavename OR searchname FILTER url_quote %]">ID</a>
</th>
......@@ -130,20 +130,13 @@
[% BLOCK columnheader %]
<th colspan="[% splitheader ? 2 : 1 %]">
[% IF column.name.match('\s+AS\s+') %]
[%# For aliased columns, use their ID for sorting. %]
[% column.sortalias = id %]
[% ELSE %]
[%# Other columns may sort on their name directly. %]
[% column.sortalias = column.name %]
[% END %]
[% desc = '' %]
[% IF (om = order.match("$column.sortalias( desc)?")) %]
[% desc = ' desc' IF NOT om.0 %]
[% IF (om = order.match("$id( DESC)?")) %]
[% desc = ' DESC' IF NOT om.0 %]
[% END %]
[% order = order.remove("$column.sortalias( desc)?,?") %]
[% order = order.remove("$id( DESC)?,?") %]
<a href="buglist.cgi?[% urlquerypart FILTER html %]&amp;order=
[% column.sortalias FILTER url_quote %][% desc FILTER url_quote %]
[% id FILTER url_quote %][% desc FILTER url_quote %]
[% ",$order" FILTER url_quote IF order %]
[%-#%]&amp;query_based_on=
[% defaultsavename OR searchname FILTER url_quote %]">
......@@ -204,6 +197,13 @@
[%- get_status(bug.$column).truncate(abbrev.$column.maxlength, abbrev.$column.ellipsis) FILTER html %]
[% ELSIF column == 'resolution' %]
[%- get_resolution(bug.$column).truncate(abbrev.$column.maxlength, abbrev.$column.ellipsis) FILTER html %]
[%# Display the login name of the user if their real name is empty. %]
[% ELSIF column.match('_realname$') && bug.$column == '' %]
[% SET login_column = column.remove('_realname$') %]
[% bug.${login_column}.truncate(abbrev.$column.maxlength,
abbrev.$column.ellipsis) FILTER html %]
[% ELSE %]
[%- bug.$column.truncate(abbrev.$column.maxlength, abbrev.$column.ellipsis) FILTER html -%]
[% END %]
......
......@@ -431,16 +431,16 @@ sub run_queries {
next unless $savedquery; # silently ignore missing queries
# Execute the saved query
my @searchfields = (
'bugs.bug_id',
'bugs.bug_severity',
'bugs.priority',
'bugs.rep_platform',
'bugs.assigned_to',
'bugs.bug_status',
'bugs.resolution',
'bugs.short_desc',
'map_assigned_to.login_name',
my @searchfields = qw(
bug_id
bug_severity
priority
rep_platform
assigned_to
bug_status
resolution
short_desc
assigned_to
);
# A new Bugzilla::CGI object needs to be created to allow
# Bugzilla::Search to execute a saved query. It's exceedingly weird,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment