Closed Bug 461729 Opened 12 years ago Closed 12 years ago

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

Categories

(Bugzilla :: Database, defect)

2.22.5
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 3.0

People

(Reporter: emmanuel, Assigned: LpSolit)

References

()

Details

Attachments

(1 file)

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
Actually, I can reproduce. I just tried again now, and both tip and 3.2 fail.
Severity: normal → major
Flags: blocking3.2?
Target Milestone: --- → Bugzilla 3.2
(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 ?
(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.
Flags: blocking3.0.6?
Flags: blocking2.22.6?
Target Milestone: Bugzilla 3.2 → Bugzilla 2.22
Version: 3.2 → 2.22.5
(In reply to comment #0)
>   $word = $dbh->quote(quotemeta($word));

You know what? I suspect that $word is over-escaped.
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.
Severity: major → normal
Flags: blocking3.2?
Flags: blocking3.2+
Flags: blocking3.0.6?
Flags: blocking3.0.6-
Flags: blocking2.22.6?
Flags: blocking2.22.6-
Summary: Incorrect SQL is generated by Bugzilla/Search.pm → [PostgreSQL] Incorrect SQL is generated when searching for keywords
(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.
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'".
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.
For the record, this code was originally in buglist.cgi:

1.70 <terry@mozilla.org> 2000-01-22 09:50
Patch by Christine Begle <cbegle@mozilla.org>, 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?
Attached patch patch, v1Splinter Review
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.
Assignee: database → LpSolit
Status: NEW → ASSIGNED
Attachment #345196 - Flags: review?(mkanat)
Attachment #345196 - Flags: review?(eseyman)
Attachment #345196 - Flags: review?(mkanat)
Attachment #345196 - Flags: review?(eseyman)
Attachment #345196 - Flags: review+
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?)
2.22 is locked to security fixes only, and this is not a security fix.
Flags: approval3.2+
Flags: approval3.0+
Flags: approval2.22-
Flags: approval+
Target Milestone: Bugzilla 2.22 → Bugzilla 3.0
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
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Summary: [PostgreSQL] Incorrect SQL is generated when searching for keywords → [PostgreSQL] Incorrect SQL is generated when searching for keywords with dashes in them
You need to log in before you can comment on or make changes to this bug.