Opened 18 months ago

Closed 9 months ago

#19324 closed feature request (fixed)

Allow more flexible mysql_connect configuration (compression, ssl, etc)

Reported by: Otto42 Owned by: nacin
Priority: normal Milestone: 3.5
Component: Database Version: 3.2
Severity: normal Keywords: has-patch commit
Cc: kobenews@…, chris@…, strategyoracle@…

Description

In wp-db.php, mysql is connected like so:

if ( WP_DEBUG ) {
	$this->dbh = mysql_connect( $this->dbhost, $this->dbuser, $this->dbpassword, true );
} else {
	$this->dbh = @mysql_connect( $this->dbhost, $this->dbuser, $this->dbpassword, true );
}

The "true" forces a new link to be established (not allowing connection reuse), and no fifth parameter means that one cannot use the mysql client constants, such as MYSQL_CLIENT_SSL for SSL support or MYSQL_CLIENT_COMPRESS for compression. Things like that.

Suggest adding filters or some kind of hooks here to allow these to be defined/overridden by plugins as well. SSL support especially would be nice for people who have separate DB servers and want a small extra smidge of security over that link.

Currently the only way to really do this and be semi-forward-compatible is to copy wp-db.php into wp-content/db.php and modify the file directly. Not exactly upgrade friendly.

Attachments (2)

19324.diff (985 bytes) - added by Otto42 13 months ago.
patch to add DB_NEW_LINK and DB_CLIENT_FLAGS defines
db.php (1.8 KB) - added by Otto42 11 months ago.
Custom db.php file that incorporates the patch as an extension to the class

Download all attachments as: .zip

Change History (22)

This is a *great* idea. I'd like to see this made a priority for 3.4, and failing that, 3.5 at least.

I especially concur with this:

<blockquote>SSL support especially would be nice for people who have separate DB servers and want a small extra smidge of security over that link.</blockquote>

Support for secure MySQL connections really needs to be in core. Security-minded WordPress users who do the work of getting their host to properly configure MySQL to accept secure connections shouldn't have to rewrite wp-db.php to get things working on the WordPress end.

Given the significant and growing popularity of WordPress, I'm sure there are many cases in which people are connecting to databases that are not on the same machines as their WordPress filesystem. It would be best if that traffic were encrypted.

Version 0, edited 18 months ago by avcascade (next)

Instead of copying the entire file, you can simply have that file require_once wpdb, followed by extending the class and replacing the function required.

Still not as clean as it could be, but works never-the-less.

comment:3 follow-up: ↓ 4   sirzooro18 months ago

The easiest way seems to add new define (e.g. WP_DB_CLIENT_FLAGS) with default value 0, and allow to specify custom value in wp-config.php.

comment:4 in reply to: ↑ 3   nacin18 months ago

Replying to sirzooro:

The easiest way seems to add new define (e.g. WP_DB_CLIENT_FLAGS) with default value 0, and allow to specify custom value in wp-config.php.

Something along those lines, yeah. The connection runs too early for plugins to run, so filters won't help much.

  • Cc kobenews@… added
  • Keywords needs-refresh needs-patch 2nd-opinion added
  • Version set to 3.3.1

I use MYSQL_CLIENT_COMPRESS, it reduces the amount of traffic between the web server and database significantly. Currently I have to modify core, and change /wp-includes/wp-db.php every release. It quite annoying. I propose a new configuration parameter:

    define(WP_MYSQL_CLIENT_COMPRESS, true);

Which then in wp-db.php I purpose the following change:

	if ( WP_DEBUG ) {
		$this->dbh = mysql_connect( $this->dbhost, $this->dbuser, $this->dbpassword, true, WP_MYSQL_CLIENT_COMPRESS ? WP_MYSQL_CLIENT_COMPRESS : 0 );
	} else {
		$this->dbh = @mysql_connect( $this->dbhost, $this->dbuser, $this->dbpassword, true, WP_MYSQL_CLIENT_COMPRESS ? WP_MYSQL_CLIENT_COMPRESS : 0 );
	}
Last edited 13 months ago by JustinK101 (previous) (diff)

patch to add DB_NEW_LINK and DB_CLIENT_FLAGS defines

Attached patch to add DB_NEW_LINK and DB_CLIENT_FLAGS defines to allow changing of these two settings.

  • Keywords needs-refresh needs-patch removed
  • Version changed from 3.3.1 to 3.4

Awesome, when will this make it into live?

  • Version changed from 3.4 to 3.3.1

The version number on the ticket should not be changed, it should reflect the original reported version.

  • Version changed from 3.3.1 to 3.2
  • Cc chris@… added

We have had this patch tested in production for several weeks now and have had no problems (running Ubuntu 12.04 on two Linode VPSs, using MYSQL_CLIENT_SSL to connect between the two). It works flawlessly. I'd love to see this fix make it in to 3.4.1, so we can stop running patched core!

How we can help make that happen?

Custom db.php file that incorporates the patch as an extension to the class

I attached a db.php file which you can drop into the wp-content directory. It basically extends the wpdb class to create a custom_wpdb class, which incorporates the patch.

This will let you run the same patch without hacking core. It's not a great solution, but I can't think of a better one because as nacin said, plugins don't have the ability to easily modify that section of code (making filters useless) and using defines are rather unpleasant at best.

Still, having a drop in like this to do that sort of thing is better than hacking core.

Last edited 11 months ago by Otto42 (previous) (diff)
  • Cc strategyoracle@… added

Thanks Otto! I still think this belongs in core but this is a nice way to make it work without patches/hacks until it makes it in.

  • Keywords has-patch added

A good patch. Let's do it.

  • Keywords commit added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to 3.5

Because these are MySQL-specific, I kind of want to call them MYSQL_* rather than DB_*. Any preference?

MYSQL_NEW_LINK and MYSQL_CLIENT_FLAGS works for me. These match the default parameter names for mysql_connect().

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [21609]:

Basic support for the mysql_connect() new_link and client_flags arguments. props Otto42, fixes #19324.

Note: See TracTickets for help on using tickets.