Make WordPress Core

Opened 2 years ago

Last modified 2 years ago

#57419 new defect (bug)

Adding terms to a taxonomy with non-latin characters results in PHP notice

Reported by: jorgeatorres's profile jorgeatorres Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: has-testing-info
Focuses: Cc:

Description

When adding terms to a taxonomy that was registered with a name that contains non-latin characters, adding any terms to the taxonomy will produce a PHP notice. This, due, to taxonomy names being light in sanitization vs screen code being too harsh.

We, on the WooCommerce team, encountered this in the context of product attributes, which are just a product taxonomy. We then confirmed this happens to all taxonomies having non-latin characters in their name.

A similar issue, but for quick edit of terms, was also reported. The code involved is different, but the core of the issue is again the discrepancy in sanitization of taxonomy names.

Steps to reproduce

  1. Register a taxonomy with non-latin characters. For example: register_taxonomy( 'tamaño', 'post', array( 'labels' => array( 'name' => 'tamaño' ) ) );
  2. Create a term in inside this new taxonomy.
  3. Term creation succeeds.
  4. Confirm that:
    • The error log contains a PHP notice along these lines: PHP Notice: Trying to get property 'show_ui' of non-object in [...]/wp-admin/includes/class-wp-terms-list-table.php on line 573.
    • Alternatively, test with the Query Monitor plugin active and confirm that the PHP notice is displayed in the JS console after the AJAX request.

Technical details

  1. Despite what the codex says, taxonomy names are actually not sanitized when registering taxonomies with register_taxonomy().
  2. When a term is added to a taxonomy, wp_ajax_add_tag(), which handles the AJAX request, uses the $_POST['screen'] to get an instance of the terms list table and initialize its screen to that value.
  3. _get_list_table() in turn calls convert_to_screen() on this arg to obtain the screen object.
  4. WP_Screen::get(), which is called by convert_to_screen(), sanitizes the passed value using sanitize_key() which removes all non-latin characters.
  5. Other checks in WP_Screen now fail as the sanitized taxonomy name obviously doesn't exist.

Change History (2)

This ticket was mentioned in Slack in #core-test by ironprogrammer. View the logs.


2 years ago

#2 @ironprogrammer
2 years ago

  • Keywords has-testing-info added

Welcome to Trac, and thank you for the report, @jorgeatorres!

This ticket has clear steps to reproduce, and it would be great if another contributor could submit a reproduction report.

Note: See TracTickets for help on using tickets.