All users were logged out of Bugzilla on October 13th, 2018

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

RESOLVED FIXED in Bugzilla 6.0

Status

()

--
minor
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: chris-bugzilla.mozilla.org, Assigned: selsky)

Tracking

unspecified
Bugzilla 6.0
Bug Flags:
approval +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

9 years ago
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
(Reporter)

Comment 1

9 years ago
Created attachment 437314 [details] [diff] [review]
Patch to add quotes

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

Comment 2

9 years ago
  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
(Reporter)

Updated

9 years ago
Attachment #437314 - Flags: review?(mkanat)

Comment 3

9 years ago
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.
Attachment #437314 - Flags: review?(mkanat) → review-

Updated

9 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → Bugzilla 3.6

Comment 4

8 years ago
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.
Target Milestone: Bugzilla 3.6 → ---
(Assignee)

Comment 5

4 years ago
Created attachment 8530757 [details] [diff] [review]
v2
Assignee: database → selsky
Attachment #437314 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8530757 - Flags: review?(gerv)

Comment 6

4 years ago
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)?
(Assignee)

Comment 7

4 years ago
` 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

4 years ago
Comment on attachment 8530757 [details] [diff] [review]
v2

So you cannot use ` here as it's not supported everywhere.
Attachment #8530757 - Flags: review?(gerv) → review-
(Assignee)

Comment 9

4 years ago
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

4 years ago
(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.
(Assignee)

Comment 11

4 years ago
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.
(Assignee)

Comment 12

4 years ago
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

4 years ago
But all DB have their own get_create_database_sql() sub.
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

4 years ago
Comment 10 hasn't been fully answered. If Oracle and Pg are affected too, they must also be fixed.
(Assignee)

Comment 16

4 years ago
Created attachment 8536223 [details] [diff] [review]
v3

I've added the double-quoting for postgres and other databases since that's the standard.  Only mysql uses the backticks.
Attachment #8536223 - Flags: review?(gerv)
(Assignee)

Updated

4 years ago
Attachment #8530757 - Attachment is obsolete: true
Attachment #8536223 - Flags: review?(gerv) → review+
Flags: approval?
Target Milestone: --- → Bugzilla 6.0
Flags: approval? → approval+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   fe5bd2c..b93e6fe  master -> master

Gerv
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.