Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#38751 closed defect (bug) (fixed)

WPDB: get_table_from_query leaves SHOW results LIKE-escaped

Reported by: andy's profile andy Owned by: pento's profile pento
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.7
Component: Database Keywords: has-patch has-unit-tests fixed-major
Focuses: Cc:

Description

Consider the statement SHOW TABLES LIKE 'wp_123_%'. The percent symbol is an unbounded wildcard. What might be less obvious is that the underscore is a wildcard matching one character. So this statement matches wp_123_posts as well as wp_1234_posts, wp_1234. The underscores in that LIKE string should be escaped.

The correct pattern is SHOW TABLES LIKE 'wp\_123\_%'.

However, all wpdb::get_table_from_query() gets from that statement is 'wp' since its subpattern stops matching at the first backslash. From the unescaped version it gets 'wp_123_' which is more useful. In fact, the latter is what hyperdb has always used to correctly map the table to a server when such a query was encountered. The liability of this workaround is that unwanted tables might be included in the results, as shown above.

To be more useful, wpdb::get_table_from_query() should unescape underscores when the match is being used with LIKE.

Attachments (4)

38751.diff (1.3 KB) - added by andy 8 years ago.
38751.2.diff (1.3 KB) - added by pento 8 years ago.
38751.3.diff (2.1 KB) - added by pento 8 years ago.
38751.4.diff (3.2 KB) - added by andy 8 years ago.

Download all attachments as: .zip

Change History (10)

@andy
8 years ago

@pento
8 years ago

#1 @pento
8 years ago

  • Keywords has-patch reporter-feedback has-unit-tests added
  • Milestone changed from Awaiting Review to 4.6.2
  • Owner set to pento
  • Status changed from new to assigned

Thanks, @andy!

I rejigged the patch a bit in 38751.2.diff, and added a test. Does this patch still work for your case?

@pento
8 years ago

#2 @pento
8 years ago

38751.3.diff pulls in one of the fixes I missed from 38751.diff - WHERE Name LIKE syntax is now supported. It also adds a bunch more unit tests.

I'm not wild about this part of the regex: (?:LIKE\s+|WHERE\s+Name(?:\s*=\s*|\s+LIKE\s+)). It feels more complicated than it needs to be, but I suspect the alternatives are to start using lookarounds (I really don't want to add that complexity to the regex), or to allow invalid SQL syntax (ie, (?:WHERE\s+Name)?(?:\s*=\s*|\s+LIKE\s+) will work, but will also match the invalid query SHOW TABLES = 'foo').

@andy
8 years ago

#3 @andy
8 years ago

  • Keywords reporter-feedback removed

@pento Thank you! I got a lot from your patches and our chat. Patch 4 explains the code changes and adds tests based on your patch.

#4 @pento
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 39275:

Database: Add support for LIKE-escaped tables in ::get_table_from_query().

The SHOW TABLES LIKE query can be used to search for tables that match a pattern, wp\_123\_%, for example. While this isn't the name of an actual table, the wp_123_ prefix can be used by database drop-ins to direct the query correctly. This change removes the escaping and % modifier, to provide this usable prefix.

Props andy, pento.
Fixes #38751.

#5 @pento
8 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Thanks for the updated patch, @andy!

#6 @helen
8 years ago

  • Milestone changed from 4.7.1 to 4.7
  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.