Page MenuHomePhabricator

[MySQL] Possibility for confusing error messages upon opening connection, while background connection is open
Closed, ResolvedPublic

Description

Correct MySQL connection error messages

When creating a DatabaseMysql object "dbB" (whose connection fails), while in the background another MySQL connection's previous action resulted in an error, dbB's creation fails with the background connection's error message.

Consider

------8<---Begin---
$dbA=new DatabaseMysql($wgDBserver,$wgDBuser,$wgDBpassword,$wgDBname);
$dbA->selectDB( "non_existing_database" );

wfDebug( "----------------------------\n" );

$wgDBuser = $wgDBuser."someText";
$dbB = wfGetDB( DB_MASTER );
------8<---End---

Connection for $dbA succeeds. Then a non existing database is selected.
Connection for $dbB fails, as the credentials are wrong on purpose.

The log messages for the $dbB connection failure however show:

------8<---Begin---
DB connection error
Server: localhost, User: wikisvnsomeText, Password: wik..., error: Access denied for user 'wikisvn'@'localhost' to database 'non_existing_database'
------8<---End---

Note the part /after/ the "error:".

Same for the DB log:
------8<---Begin---
Wed Feb 29 23:11:13 UTC 2012 spencer wikisvn Error connecting to localhost: Access denied for user 'wikisvn'@'localhost' to database 'non_existing_database' (localhost)
------8<---End---

The attached patch uses mysql_error only as fallback and produces the following log:

------8<---Begin---
DB connection error
Server: localhost, User: wikisvnsomeText, Password: wik..., error: Access denied for user 'wikisvnsomeText'@'localhost' (using password: YES)
------8<---End---

and the following DB log:

------8<---Begin---
Wed Feb 29 23:14:01 UTC 2012 spencer wikisvn Error connecting to localhost: Access denied for user 'wikisvnsomeText'@'localhost' (using password: YES)
------8<---End---

Both, log and DB log, show the expected error message after applying the patch.


Version: 1.20.x
Severity: trivial

Attached:

Details

Reference
bz34819

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:12 AM
bzimport set Reference to bz34819.
bzimport added a subscriber: Unknown Object (MLST).

adding Chad to ask him to look at this patch

I understand switching from mysql_error() to $error, but why the swapping of the priorities between the slurped PHP error and the checking for immediate errors? It seems a bit unclear to me.

(In reply to comment #2)

but why the swapping of
the priorities between the slurped PHP error and the checking for immediate
errors?

This switch arranges that:

  • dbA reports only the errors of dbA, and
  • dbB reports only the errors of dbB.

Without this switch, dbB's failed connection attempt would yield dbA's "database selection" error

The culprit for this mismatch is lastError().
dbB does not have a proper mConn set, when lastError() is called on it (See the guarding "if" condition). Hence, this lastError() call comes down to calling mysql_error() without parameters. However, calling mysql_error() without parameters refers to the previously (successfully) opened connection (In our case: dbA).

So without the switch, dbB reports the error message of dbA instead of it's own error message (see the logs in bug description).

sumanah wrote:

Hi, Christian! Thanks for the patch. Maybe you could use Developer
access https://www.mediawiki.org/wiki/Developer_access to submit your patch
directly into Git and Gerrit, so it'll get reviewed faster. Thanks.

(In reply to comment #4)

Maybe you could [...] to submit your patch
directly into Git and Gerrit [...]

Done.
Gerrit: https://gerrit.wikimedia.org/r/#/c/9202/1