wpdb::prepare() should reject invalid schema object names

[dmsnell]

MySQL imposes certain constraints on schema object names (table names, column names, etc… what Core calls “identifiers” in some places). Queries with invalid values for these names will always fail, but sending them to the database incurs network traffic and latency.

If Core knows that a parameter cannot be valid then it could reject early and skip sending the query to the database. It could even avoid performing costly escaping of string values which also involve separate database calls, if it knows that the query can never succeed.

One example is that schema object names cannot end in a space character. Currently wpdb->prepare( "SELECT * FROM %i", "test " ) will produce the following table name:

`test ` 

…and this will always fail when run as a query.

Trac ticket: Core-63734
See Core-52506.

Rejects schema object names (column names, table names, etc…) provided to wpdb::prepare() if they are expected to be rejected by MySQL, because not all possible strings can be escaped.

This change short-circuits a call to the database which would fail anyway, thus failing faster and reducing database traffic. It adds a _doing_it_wrong() message to notify developers of improper use.

[vladimiraus]

Some PHP coding standards errors in Pull Request.

Is this a common problem? Is there a performance impact with these new checks?

Is this a common problem? Is there a performance impact with these new checks?

I’m not sure @johnbillion, and it might be hard getting this data because I would expect it mostly to impact a small percentage of plugins, but I was coming to this from an unrelated bug and in the vein of wanting to close any parsing gaps we might have. Every gap is an opportunity for corruption; or worse.

Performance-wise I don’t know how to measure it. The string-parsing should add a negligible overhead (which we can measure to confirm it’s not above the threshold of detectability), but the very first time it catches an invalid schema object name it should more than make up for that performance hit because of skipping a network round-trip to MySQL.

From the original code and unit tests added in Core-52506 we can see that the intent was to more-properly recognize the SQL syntax, but we stoped short at escaping backticks. This work follows that start in applying the rules set by MySQL itself.