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 | Owned by: | 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)
Change History (10)
#1
@
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
#2
@
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'
).
#3
@
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.
Thanks, @andy!
I rejigged the patch a bit in 38751.2.diff, and added a test. Does this patch still work for your case?