Ticket #745 (closed defect: fixed)

Opened 1 year ago

Last modified 9 months ago

When using mysqli extension, db connect fails

Reported by: mdawaffe Assigned to: mdawaffe
Priority: high Milestone: 0.9
Component: Administration Version: 0.8.3
Severity: major Keywords:
Cc:

Description

http://bbpress.org/forums/topic/db-access-problem-in-the-first-installation-step http://bbpress.org/forums/topic/intergration-will-not-let-me-logon-using-my-wordpress-login

Warning: mysql_get_server_info() [function.mysql-get-server-info]:
Access denied for user 'root'@'localhost' (using password: NO)
in /bb-includes/db-mysqli.php on line 80

User and Password are supplied, but bbPress tries to connect with root (or equivalent) without a password.

Only effects mysqli connections, not mysql.

Overwrite db-mysqli.php with db.php, and everything works.

Attachments

745.diff (3.7 kB) - added by mdawaffe on 09/30/07 18:55:56.
745b.diff (3.7 kB) - added by mdawaffe on 09/30/07 19:00:01.
Better version
745c.diff (4.0 kB) - added by mdawaffe on 09/30/07 21:36:50.

Change History

09/30/07 06:46:36 changed by chrishajer

I just posted in the forum:

I think the problem is with <a href="http://trac.bbpress.org/browser/trunk/bb-includes/db-mysqli.php#L80">line 80 in db-mysqli.php</a>:

if ( !empty($this->charset) && version_compare(mysql_get_server_info(), '4.1.0', '>=') )

I think mysql_get_server_info() should be mysqli_get_server_info()

As it stands, the error produces a warning and prevents the charset from ever being set. Making that mysqli_get_server_info makes the warnings go away.

09/30/07 11:56:33 changed by sambauers

That call to mysqli_get_server_info() needs to contain the link to the mysqli object.

So perhaps line 80 should be:

if ( !empty($this->charset) && version_compare(mysqli_get_server_info($this->$dbhname), '4.1.0', '>=') )

09/30/07 13:26:52 changed by chrishajer

If mysql_get_server_info() is changed to mysqli_get_server_info(), it works fine since it uses the last link opened by mysqli_connect. To be explicit, you could put that parameter there, but if it fails to use the last link object, I think including the parameter explicitly would fail too. The link object is typically optional I think.

09/30/07 13:35:13 changed by chrishajer

Interesting, I just read this comment: http://bbpress.org/forums/topic/db-access-problem-in-the-first-installation-step?replies=10#post-11205

Warning: mysqli_get_server_info() expects exactly 1 parameter, 0 given

http://us.php.net/manual/en/function.mysqli-get-server-info.php

Apparently, mysqli_get_server_info requires the parameter, where mysql_get_server_info does not. So, I believe you're right Sam to require the $this->$dbhname (contradicting what I said earlier.) Apparently when I was testing this, I had added that object first, then tried the mysqli change, so I had already done it, but was going off information I read for mysql_get_server_info.

09/30/07 18:55:18 changed by mdawaffe

Try 745.diff out. It uses mysqli_ when appropriate. It's a little tricky in upgrade-schema.php since we could be looking at two different DBs, the normal one and the user one.

09/30/07 18:55:56 changed by mdawaffe

  • attachment 745.diff added.

09/30/07 19:00:01 changed by mdawaffe

  • attachment 745b.diff added.

Better version

09/30/07 21:36:50 changed by mdawaffe

  • attachment 745c.diff added.

09/30/07 21:42:42 changed by mdawaffe

I realized that BB_Query should be using mysql(i)_get_server_info() instead of mysql(i)_get_client_info(), so I decided version info was a useful enough thing to know that it should be in the DB class.

In 745c.diff

$bbdb->db_version( $dbh ) where $dbh can either be a string specifying the table name of a table in the database you're interested in, or a mysql(i)_connect() resource.

I chose table name over DB name for future compatibility with WordPress MU, HyperDB and BackPress.

Thoughts?

10/09/07 13:56:07 changed by baptiste

Diff 'C' works for me fine for a 0.8.2.1->0.8.3 upgrade.

10/10/07 17:33:16 changed by chrishajer

I tried to apply DiffC to an svn checkout of 0.8.3, but part of it failed:

server:~/bbpress-test > patch -p0 < diffc.patch
patching file bb-includes/classes.php
patching file bb-includes/db-mysqli.php
Hunk #2 FAILED at 79.
1 out of 3 hunks FAILED -- saving rejects to file bb-includes/db-mysqli.php.rej
patching file bb-includes/db.php
patching file bb-admin/upgrade-schema.php

The content of bb-includes/db-mysqli.php.rej:

***************
*** 77,83 ****

                $this->$dbhname = @mysqli_connect( $server->host, $server->user, $server->pass, null, $server->port );

-               if ( !empty($this->charset) && version_compare(mysql_get_server_info(), '4.1.0', '>=') )
                        $this->query("SET NAMES '$this->charset'");

                $this->select( $server->database, $this->$dbhname );
--- 79,85 ----

                $this->$dbhname = @mysqli_connect( $server->host, $server->user, $server->pass, null, $server->port );

+               if ( !empty($this->charset) && version_compare(mysqli_get_server_info($this->$dbhname), '4.1.0', '>=') )
                        $this->query("SET NAMES '$this->charset'");

                $this->select( $server->database, $this->$dbhname );

FYI

11/01/07 00:28:14 changed by mdawaffe

  • owner set to mdawaffe.
  • status changed from new to assigned.

WP has a slightly better way of doing this. I'll smoosh them together.

11/16/07 20:20:14 changed by mdawaffe

  • status changed from assigned to closed.
  • resolution set to fixed.

Fixed in [954].

11/16/07 20:21:52 changed by mdawaffe

Someone want to kick this in PHP 5?

Someone want to kick this using the mysqli extension?

11/17/07 00:51:15 changed by mdawaffe

(In [956]) db class structure changes. both mysql and mysqli classes inherit from base class. has_cap(), db_version(), set_prefix(), is_write_query(), insert(), update() methods. updated get_table_from_query() method. Fixes #745

11/17/07 03:56:47 changed by sambauers

Line 172 of db-mysqli.php doesn't work.

Using is_object instead of is_resource works.

Otherwise, apparently fine.

11/17/07 20:21:27 changed by mdawaffe

(In [957]) mysqli_connect() returns a mysqli object, not a mysql resource. Fixes #745 props sambauers