Creating new posts/terms with non-latin characters is broken for tables with cp1251 collation in 4.1.5

[vloo]

The issue is identical to #32051.

Problematic code is in wp-includes/wp-db.php, as reverting it to 4.1.4 (just this file) solves the issue.

My guess (without having the time to check or prove it) is that changes in 4.2.2 were applied in 4.1.5 without considering that it was expected to be still tolerant to unorthodox collations.

P.S. Can't point out the version as it's still not in the list.

[vloo]

cp1251_bulgarian_ci is problematic, I tested it on a site with cp1251_general_ci and it was alright there. It'd be helpful if someone else could also try to reproduce it.

[SergeyBiryukov]

Confirmed. All branches from 3.7.8 to 4.2.2 appear to be affected.

Steps to reproduce:

1. Get a ​Bulgarian or ​Russian package.
2. Before installing, set DB_CHARSET to cp1251 in wp-config.php.
3. Proceed with installing.

When you try to view the site after installing:

• The default content is missing.
• Most of the options in wp_options are missing.

When you try to log in:

• It tells you that a DB upgrade is needed.
• During the upgrade, there's a bunch of errors:

  WordPress database error: [Table 'wordpress.wp422_categories' doesn't exist]
  SELECT cat_ID, cat_name, category_nicename FROM wp422_categories
  
  WordPress database error: [Table 'wordpress.wp422_post2cat' doesn't exist]
  SELECT DISTINCT post_id FROM wp422_post2cat
  
  WordPress database error: [Unknown column 'post_category' in 'field list']
  SELECT ID, post_category FROM wp422_posts WHERE post_category != '0'
  
  WordPress database error: [Table 'wordpress.wp422_categories' doesn't exist]
  ALTER TABLE `wp422_categories` ADD INDEX ( `category_nicename` )
  
  WordPress database error: [Key column 'link_category' doesn't exist in table]
  ALTER TABLE `wp422_links` ADD INDEX ( `link_category` )
  
  WordPress database error: [Unknown column 'user_nickname' in 'field list']
  SELECT ID, user_nickname, user_nicename FROM wp422_users
  
  WordPress database error: [Table 'wordpress.wp422_categories' doesn't exist]
  SELECT * FROM wp422_categories ORDER BY cat_ID
  
  WordPress database error: [Table 'wordpress.wp422_post2cat' doesn't exist]
  SELECT post_id, category_id FROM wp422_post2cat GROUP BY post_id, category_id
  
  WordPress database error: [Table 'wordpress.wp422_linkcategories' doesn't exist]
  SELECT cat_id, cat_name FROM wp422_linkcategories
  
  WordPress database error: [Unknown column 'link_category' in 'field list']
  SELECT link_id, link_category FROM wp422_links
  

Previous release in each branch (3.7.7 to 4.2.1) works correctly.

[pento]

Duplicate of #32165.

This is happening for the same reasons as described in 32165#comment:36.

[dd32]

I'm re-opening this as it seems like a different case than #32165 from what I can understand.

The issue at play here, from what I can see, is initially that the following query will always fail, as strip_text_from_query() will return '':

INSERT INTO table_with_cp1251_charset( `a` ) VALUES( 'safe data' )

The reason is that ultimately it performs this query:

LEFT( CONVERT( 'INSERT INTO table_with_cp1251_charset( `a` ) VALUES( \'safe data\' )' USING cp1251 ), 0 )

The LEFT( %s, 0 ) means it'll always fail, the 0 comes from the fact that strip_invalid_text() currently requires the length field be present when testing non-utf8/utf8mb4/latin1 strings, and strip_invalid_text_from_query() never passes that field.

Attached in 32279.diff​ is a unit test that demonstrates that, and a fix that passes the unit-test, but one that I'm not confident in - I'm struggling to find an example query that should fail.
As I understand it, the fix should be OK since the idea is that the CONVERT() query should fail, or return a different length of data, which causes the failure to get raised higher up in the chain.

I don't think this fixes all of the issues, I suspect #32165 will still kick in, but this allows the install & term creation to work again..

[obenland]

@dd32 is this something we should wait for pento to look at?

[dd32]

I've pinged @nbachiyski, @mdawaffe and @pento for a code review here, I haven't heard back yet from the first two. @pento suggested that it's likely that a bunch of the code in the function might not be needed, but couldn't give a yay or nay on the patch.

[netweb]

Related: #32917 Tests_DB_Charset tests don't fully cover wpdb::strip_invalid_text_for_column()

[mdawaffe]

Patch looks good to me. I fixed a bad variable assignment and added a couple more related (but slightly different) test cases in attachment:32279.2.diff​.

[mdawaffe]

Updated - clearer comment

[mdawaffe]

attachment:32279.3.diff​ is the same but clarifies a code comment in the tests.

[pento]

Let's go with 32279.3.diff​ for the moment. I recall there being some bigger issues with this area of code, related to #32165, but it's going to take me some time to get back up to speed. Worst case is that this is fixed in 4.2.3, and #32165 will be pushed back to 4.3.1.

[pento]

In 33310:

WPDB: ::strip_text_from_query() doesn't pass a length to ::strip_invalid_text(), which was causing queries to fail when they contained characters that needed to be sanity checked by MySQL.

Props dd32, mdawaffe, pento.

Fixes #32279.

[pento]

In 33311:

WPDB: ::strip_text_from_query() doesn't pass a length to ::strip_invalid_text(), which was causing queries to fail when they contained characters that needed to be sanity checked by MySQL.

Props dd32, mdawaffe, pento.

Merges [33310] to the 4.2 branch.

Fixes #32279.

[SergeyBiryukov]

I can still reproduce the issue in 4.2.3 with the steps from comment:3, and I'm seeing a lot of similar reports on support forums from those using the cp1251 charset.

Steps to reproduce:

1. Get a ​Bulgarian or ​Russian package.
2. Before installing, set DB_CHARSET to cp1251 in wp-config.php.
3. Proceed with installing.

• When you try to view the site after installing, the default content is missing.
• If you try to create a post or page, you'll always see _wp_posts_page_notice() instead of the editor, because get_default_post_to_edit() never succeeds due to __( 'Auto Draft' ) being translated into Cyrillic.
• If installing with WP_DEBUG enabled, there's a bunch of notices in the process:

Notice: Undefined index: x_term_id in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_slug in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_term_group in wp-includes/wp-db.php on line 2751

Notice: Undefined index: x_post_author in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_date in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_date_gmt in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_content in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_excerpt in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_name in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_modified in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_modified_gmt in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_guid in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_comment_count in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_to_ping in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_pinged in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_content_filtered in wp-includes/wp-db.php on line 2751

Notice: Undefined index: x_comment_post_ID in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_comment_author_email in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_comment_author_url in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_comment_date in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_comment_date_gmt in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_comment_content in wp-includes/wp-db.php on line 2751

Notice: Undefined index: x_post_author in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_date in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_date_gmt in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_content in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_excerpt in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_name in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_modified in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_modified_gmt in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_guid in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_type in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_to_ping in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_pinged in wp-includes/wp-db.php on line 2751
Notice: Undefined index: x_post_content_filtered wp-includes/wp-db.php on line 2751

[pento]

In 33455:

WPDB: When checking the encoding of strings against the database, make sure we're only relying on the return value of strings that were sent to the database. Also make sure that we're not trying to sanity check strings that've been marked as not needing sanity checking.

Fixes #32279.

[pento]

Re-opening for potential inclusion in 4.2.4.

[SergeyBiryukov]

[33455] works as expected, +1 for merging into 4.2.4.

[SergeyBiryukov]

FWIW, 3.7.9 to 4.1.6 are also broken with the same results as in comment:3.

Can we merge both [33310] and [33455] there?

[pento]

Given that it was a bug in a security patch, I'm okay with going back to 4.1, as some people are still stuck there due to this bug.

I'm inclined to not patch 3.x, because the lack of this fix doesn't introduce new security issues.

[pento]

In 33476:

WPDB: When checking the encoding of strings against the database, make sure we're only relying on the return value of strings that were sent to the database. Also make sure that we're not trying to sanity check strings that've been marked as not needing sanity checking.

Merge of [33455] to the 4.2 branch.

See #32279.

[pento]

In 33479:

WPDB: ::strip_text_from_query() doesn't pass a length to ::strip_invalid_text(), which was causing queries to fail when they contained characters that needed to be sanity checked by MySQL.

Props dd32, mdawaffe, pento.

Merges [33310] to the 4.1 branch.

See #32279.

[pento]

In 33480:

WPDB: When checking the encoding of strings against the database, make sure we're only relying on the return value of strings that were sent to the database. Also make sure that we're not trying to sanity check strings that've been marked as not needing sanity checking.

Merge of [33455] to the 4.1 branch.

See #32279.