WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#16228 closed defect (bug) (duplicate)

$wpdb->query and DROP TABLE

Reported by: elfin Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: Database Keywords:
Focuses: Cc:

Description

Using the shiny new admin Debug Bar plugin I noticed an issue on a plugin uninstall.

  1. WARNING: E:\htdocs\wpbeta\wp-includes\wp-db.php:1120 - mysql_num_fields() expects parameter 1 to be resource, boolean given
  2. WARNING: E:\htdocs\wpbeta\wp-includes\wp-db.php:1125 - mysql_fetch_object(): supplied argument is not a valid MySQL result resource
  3. WARNING: E:\htdocs\wpbeta\wp-includes\wp-db.php:1130 - mysql_free_result() expects parameter 1 to be resource, boolean given

looking at line 1110 of wp-db.php I see this line:

if ( preg_match( "/^\\s*(insert|delete|update|replace|alter) /i", $query ) ) {

adding in drop to the mix like this:

if ( preg_match( "/^\\s*(insert|delete|update|replace|alter|drop) /i", $query ) ) {

does seem to fix it, but I am unaware of the security issues associated. It should be noticed that this error does not appear on page, or in the debug.log despite having the following set:

define('WP_DEBUG', true);
define( 'WP_DEBUG_LOG', true );

Change History (6)

comment:1 mdawaffe3 years ago

  • Resolution set to invalid
  • Status changed from new to closed

Those warnings are all @-suppressed, so it's a bug in the Debug Bar's custom error handler. Not with WP.

Debug Bar should be checking the return value of error_reporting().

Closing as invalid (technically) and pinging the plugin authors.

comment:2 ocean903 years ago

  • Milestone Awaiting Review deleted

comment:3 mdawaffe3 years ago

  • Milestone set to Future Release
  • Resolution invalid deleted
  • Status changed from closed to reopened

westi says it's intentional on the part of the Debug Bar. This is the old @-suppression is evil argument (which I mostly agree with).

So I guess we should reopen and push to Future.

The @ suppression should be ditched and the code tweaked to handle more cases.

wpdb::query( DROP ) should maybe just return true/false instead?

comment:4 elfin3 years ago

and looking at my suggested fix it isn't really suitable. Could a new method be added in for DROPping tables?

comment:5 solarissmoke3 years ago

Related #11372 - same fix for both

comment:6 scribu3 years ago

  • Milestone Future Release deleted
  • Resolution set to duplicate
  • Status changed from reopened to closed

Closing as dup of #11372 then.

Note: See TracTickets for help on using tickets.