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\zinstead 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 notWP_Widgetobjects keep working, astest_create_item_legacy_widgetcovers.
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
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
wp_parse_widget_id()anchored its pattern with$, which also matches before a final newline, so"text-1\n"was parsed as the Text widgettext-1. Creating a widget throughPOST /wp/v2/widgetswith that ID saved the new settings totext-1and 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, soPUT /wp/v2/widgets/{id}with only asidebarparameter 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 404rest_widget_not_founderror when the widget is in no sidebar and its ID is not in$wp_registered_widgets. Registered widgets that are notWP_Widgetobjects can still be updated without a sidebar, astest_create_item_legacy_widgetcovers.Testing
test_does_not_parse_number_before_trailing_newline,test_create_item_rejects_id_with_trailing_newline,test_update_item_unknown_id_returns_404andtest_update_item_rejects_id_with_trailing_newline.update_item()change, the parser and create tests fail.Tests_Widgetspass (507 tests) on single site and multisite, and thecustomizegroup 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.