Make WordPress Core

Opened 13 hours ago

Last modified 13 hours ago

#66434 new defect (bug)

Widgets REST endpoint changes widget data for IDs that end in a newline or match no widget

Reported by: ekamran Owned by:
Priority: normal Milestone: Awaiting Review
Component: Widgets Version: 5.8
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses: rest-api

Description

wp_parse_widget_id() matches widget IDs with /^(.+)-(\d+)$/. In PHP, $ also matches before a final newline, so "text-1\n" is parsed as id_base text and number 1.

The widgets REST endpoint relies on that result. With a Text widget text-1 in sidebar-1 and an empty sidebar-2, as an administrator:

<?php
$request = new WP_REST_Request( 'POST', '/wp/v2/widgets' );
$request->set_body_params(
        array(
                'id'       => "text-1\n",
                'sidebar'  => 'sidebar-2',
                'instance' => array( 'raw' => array( 'text' => 'Updated text test' ) ),
        )
);
$response = rest_do_request( $request );

Result on trunk:

status 500, rest_invalid_widget
widget_text[1]['text']: Updated text test
sidebars_widgets: sidebar-1 => ["text-1"], sidebar-2 => ["text-1\n"]

The settings of text-1 change and "text-1\n" is added to sidebar-2, and then the request fails because no widget is registered under "text-1\n". The same request with "text-1" returns 201 and moves text-1 to sidebar-2.

A second check, in WP_REST_Widgets_Controller::update_item(), is related. When the widget is in no sidebar, it returns a 404 error only if the ID resolves to a WP_Widget object, so that registered widgets that are not WP_Widget objects can still be updated. An ID that matches no registered widget passes too. A PUT /wp/v2/widgets/unknown-3 request with only sidebar set to sidebar-1 adds unknown-3 to sidebar-1 in sidebars_widgets and then fails with a 500 rest_invalid_widget error.

The two need to be fixed together. With only the parser fixed, "text-1\n" no longer resolves to the Text widget and takes the second path: PUT /wp/v2/widgets/text-1 with id set to "text-1\n" and sidebar set to sidebar-2, which returns 404 on trunk, would add "text-1\n" to sidebar-2 and return 500.

Both requests need the edit_theme_options capability. I reproduced them on trunk with rest_do_request() as an administrator, with two test sidebars registered.

Proposed change:

  • In wp_parse_widget_id(), anchor the pattern with \z instead of $.
  • In update_item(), also return the 404 error when the widget is in no sidebar and its ID is not in $wp_registered_widgets. Registered widgets that are not WP_Widget objects keep working, as test_create_item_legacy_widget covers.

Related: WP_Customize_Widgets::parse_widget_id() uses the same pattern. I have not tested the Customizer path, so the proposed change leaves it alone. Should it get \z too for consistency, here or in a follow-up?

Change History (1)

This ticket was mentioned in ​PR #15314 on ​WordPress/wordpress-develop by ​@ekamran.


13 hours ago
#1

  • Keywords has-patch has-unit-tests added

wp_parse_widget_id() anchored its pattern with $, which also matches before a final newline, so "text-1\n" was parsed as the Text widget text-1. Creating a widget through POST /wp/v2/widgets with that ID saved the new settings to text-1 and added "text-1\n" to the sidebar, and then returned a 500 error.

WP_REST_Widgets_Controller::update_item() also let an ID that matches no registered widget through when the widget is in no sidebar, so PUT /wp/v2/widgets/{id} with only a sidebar parameter added that ID to the sidebar before returning a 500 error. Fixing only the parser would send "text-1\n" down this path, so both are fixed here.

What changes

  • wp_parse_widget_id() anchors its pattern with \z, so an ID that ends in a newline is no longer split into a base and a number.
  • update_item() returns the existing 404 rest_widget_not_found error when the widget is in no sidebar and its ID is not in $wp_registered_widgets. Registered widgets that are not WP_Widget objects can still be updated without a sidebar, as test_create_item_legacy_widget covers.

Testing

  • 4 new tests: test_does_not_parse_number_before_trailing_newline, test_create_item_rejects_id_with_trailing_newline, test_update_item_unknown_id_returns_404 and test_update_item_rejects_id_with_trailing_newline.
  • On trunk, the first 3 fail. The fourth passes on trunk and fails when only the parser is fixed, which is the case it guards. With only the update_item() change, the parser and create tests fail.
  • Locally with PHP 8.3 and MySQL 9.7: the REST widgets, sidebars and widget types controller tests and Tests_Widgets pass (507 tests) on single site and multisite, and the customize group passes (243 tests). PHPCS reports nothing on the changed files.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code and Codex
Used for: Investigation, code, tests, local testing and PR wording. I reviewed the reasoning, the code and the test results, and I take responsibility for the contribution.

Note: See TracTickets for help on using tickets.