461729 - [PostgreSQL] Incorrect SQL is generated when searching for keywords with dashes in them

Status: RESOLVED FIXED

(Bugzilla :: Database, defect)

Description

[Emmanuel Seyman]

Running the QA tests, I had the keywords one fail because Bugzilla generated incorrect SQL. Tracking down the error, I realize the line responsible for the error is this one (in Bugzilla/Search.pm) :

  $word = $dbh->quote(quotemeta($word));

The following line :

perl -MBugzilla -e '$dbh = Bugzilla->dbh; print $dbh->quote(quotemeta("key-selenium-ktwo"))'

gives the following result :

E'key\\-selenium\\-ktwo'

when 'key\\-selenium\\-ktwo' is expected. The results makes the SQL query incorrect and Bugzilla dies with an internal server error.

This only occurs when using Pg as a database and only when the keyword has a '-' in it. LpSolit wasn't able to reproduce the error so I suspect it's a bug in DBD::Pg and not in Bugzilla.

This is on a Fedora 9 with all updates :
fedora-release-9-5.transition.noarch
postgresql-8.3.4-1.fc9.i386
perl-DBD-Pg-2.10.0-1.fc9.i386

Comment 1

[Frédéric Buclin]

Actually, I can reproduce. I just tried again now, and both tip and 3.2 fail.

Comment 2

[Emmanuel Seyman]

(In reply to comment #1)
>
> Actually, I can reproduce. I just tried again now, and both tip and 3.2 fail.

What version of DBD::Pg are you using ?

Comment 3

[Frédéric Buclin]

(In reply to comment #2)
> What version of DBD::Pg are you using ?

* This is Bugzilla 3.3 on perl 5.10.0                                           
Checking for              DBD-Pg (v1.45)   ok: found v2.10.3
Checking for      PostgreSQL (v8.00.0000) ok: found v08.03.0400

Also, I can reproduce on 3.0 and 2.22.5, so it seems that either Pg has a problem, or quotemeta() in Perl 5.10 is broken.

Comment 4

[Frédéric Buclin]

(In reply to comment #0)
>   $word = $dbh->quote(quotemeta($word));

You know what? I suspect that $word is over-escaped.

Comment 5

[Max Kanat-Alexander]

This may be a bug in DBD::Pg 2.x, which is a fairly new series of releases and is not in common use.

And yes, that does look kind of over-escaped to me.

Comment 6

[Frédéric Buclin]

(In reply to comment #5)
> This may be a bug in DBD::Pg 2.x, which is a fairly new series of releases and
> is not in common use.

I'm pretty sure DBD::Pg 2.x is in all newer distros. It's already in Fedora and Mandriva, and I'm mostly sure the coming OpenSUSE and Ubuntu have it.

Comment 7

[Emmanuel Seyman]

The weird thing is that quotemeta("key-selenium-ktwo") returns "key\-selenium\-ktwo" and quote("key\-selenium\-ktwo") returns 'key-selenium-ktwo' (the expected result).

Yet somehow, quote(quotemeta("key-selenium-ktwo")) returns "E'key\\-selenium\\-ktwo'".

Comment 8

[Frédéric Buclin]

When you read http://rt.cpan.org/Public/Bug/Display.html?id=39390, nobody seems surprised to see E' in front of the quoted string.

Comment 9

[Frédéric Buclin]

For the record, this code was originally in buglist.cgi:

1.70 <[e-mail]> 2000-01-22 09:50
Patch by Christine Begle <[e-mail]>, with heavy
modifications by me -- let you query for "any words" and "all words",
as well as the existing substring and regexp stuff.

It has no bug ID, and so we don't have a chance why quotemeta() is used. Could it mean that $dbh->quote() alone doesn't escape all required characters correctly to be used in a regexp?

Comment 10

[Frédéric Buclin]

I reported the bug upstream: http://rt.cpan.org/Ticket/Display.html?id=40440

Comment 11

[Frédéric Buclin]

Attachment: patch, v1

I followed what was suggested in the bug reported upstream about DBD::Pg. I tested on both Pg and MySQL, and it's working fine.

Comment 12

[Max Kanat-Alexander]

Comment on attachment 345196 [details] [diff] [review]
patch, v1

Yes, that makes much more sense anyhow.

By the way, why aren't we using "\b" here instead of all that? (Maybe the DBs don't support \b?)

Comment 13

[Max Kanat-Alexander]

2.22 is locked to security fixes only, and this is not a security fix.

Comment 14

[Frédéric Buclin]

Yeah, I'm not sure \b is supported. Either that or this is old code for old DBs.


tip:

Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.166; previous revision: 1.165
done

3.2rc1:

Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.159.2.5; previous revision: 1.159.2.4
done

3.0.5:

Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.145.2.3; previous revision: 1.145.2.2
done