557536 - checksetup.pl fails on ALTER DATABASE if database name contains hyphen

Status: RESOLVED FIXED

(Bugzilla :: Database, defect)

Description

[Christoph Probst]

User-Agent:       Opera/9.80 (Windows NT 5.1; U; de) Presto/2.5.22 Version/10.51
Build Identifier: bugzilla-3.4.6.tar.gz

I created an empty database named 'bugzilla-test' using phpMyAdmin 3.3.1, modified localconfig to use this database and run checksetup.pl. This script failed with:

#1064 - You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-test CHARACTER SET utf8' at line 1

The reason is that "ALTER DATABASE bugzilla-test CHARACTER SET utf8" isn't a valid mysql query, due to missing quotes.

Reproducible: Always

Steps to Reproduce:
1. Create new database with a hyphen in its name
2. choose another charset than utf8
3. run checksetup.pl

Comment 1

[Christoph Probst]

Attachment: Patch to add quotes

After changing the ALTER DATABASE line there were no further issues with my this database name.

Comment 2

[Max Kanat-Alexander]

  Hey Christopher. Thank you for your patch! If you'd like to get it into Bugzilla, see our development process, here:

  http://wiki.mozilla.org/Bugzilla:Developers

Comment 3

[Max Kanat-Alexander]

Comment on attachment 437314 [details] [diff] [review]
Patch to add quotes

You probably also need to fix CREATE DATABASE and any other DATABASE commands that we use. You might want to use $dbh->quote_identifier.

Comment 4

[Frédéric Buclin]

Bugzilla 3.6 is now restricted to security fixes only, and this bug got no traction for several months. We will retarget this bug once it has a patch ready for checkin.

Comment 5

[Matt Selsky [:selsky]]

Attachment: v2

Comment 6

[Frédéric Buclin]

Comment on attachment 8530757 [details] [diff] [review]
v2

>+    $self->do("ALTER DATABASE `$db_name` CHARACTER SET utf8"); 

Is ` legal here? And is it universal (MySQL, Pg, Oracle, SQLite)?

Comment 7

[Matt Selsky [:selsky]]

` is used by OBDC and MySQL (unless MySQL is in ANSI mode) per http://www.nntp.perl.org/group/perl.dbi.users/2003/11/msg20931.html

It looks like everything else uses double-quotes.

Comment 8

[Frédéric Buclin]

Comment on attachment 8530757 [details] [diff] [review]
v2

So you cannot use ` here as it's not supported everywhere.

Comment 9

[Matt Selsky [:selsky]]

But I'm only patching the MySQL module. And there's no support for obdc.  Or do you want the quoting done always(even if not needed) in the parent module?

Comment 10

[Frédéric Buclin]

(In reply to Matt Selsky [:selsky] from comment #9)
> But I'm only patching the MySQL module. And there's no support for obdc.  Or
> do you want the quoting done always(even if not needed) in the parent module?

What's the behavior of Pg, SQLite and Oracle when DB names contain whitespaces or hyphens? If they require no quoting, then your patch would be fine. Else they would need to be fixed too.

Comment 11

[Matt Selsky [:selsky]]

SQLite works without any patch when the db name contain hyphens or whitespace.

I don't have Oracle handy to test with.

I can attempt to set up Pg unless someone else has a test environment handy.

Comment 12

[Matt Selsky [:selsky]]

Only mysql accesses $db_name outside of the DBI connect string (in the ALTER DATABASE query) so that's why only it needs the special quoting.

Comment 13

[Frédéric Buclin]

But all DB have their own get_create_database_sql() sub.

Comment 14

[Gervase Markham [:gerv]]

LpSolit: Matt's point is that this is MySQL-specific code (it's in Mysql.pm) and so it doesn't matter if he's using a MySQL-specific construct. That's precisely what such DB-specific files are for, isn't it? 

What's wrong with that logic?

Gerv

Comment 15

[Frédéric Buclin]

Comment 10 hasn't been fully answered. If Oracle and Pg are affected too, they must also be fixed.

Comment 16

[Matt Selsky [:selsky]]

Attachment: v3

I've added the double-quoting for postgres and other databases since that's the standard.  Only mysql uses the backticks.

Comment 17

[Gervase Markham [:gerv]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   fe5bd2c..b93e6fe  master -> master

Gerv