All users were logged out of Bugzilla on October 13th, 2018
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
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.
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 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-
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → Bugzilla 3.6
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 → ---
Created attachment 8530757 [details] [diff] [review] v2
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)?
` 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 on attachment 8530757 [details] [diff] [review] v2 So you cannot use ` here as it's not supported everywhere.
Attachment #8530757 - Flags: review?(gerv) → review-
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?
(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.
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.
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.
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 10 hasn't been fully answered. If Oracle and Pg are affected too, they must also be fixed.
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)
Attachment #8530757 - Attachment is obsolete: true
Attachment #8536223 - Flags: review?(gerv) → review+
Target Milestone: --- → Bugzilla 6.0
To ssh://firstname.lastname@example.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.