WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 21 months ago

Last modified 21 months ago

#35869 closed defect (bug) (fixed)

Preview of menu items includes unnecessary slashes for users without unfiltered_html

Reported by: ocean90 Owned by: westonruter
Milestone: 4.5 Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords:
Focuses: Cc:

Description

Noticed while testing #27355.

Assign an author the edit_theme_options cap and enter "foo'foo" into the input field for a menu item title: For some reasons the preview will have slashes.

Comment by westonruter:

I narrowed this down to the following line in WP_Customize_Nav_Menu_Item_Setting::sanitize():

<?php
$menu_item_value['title'] = apply_filters( 'title_save_pre', $menu_item_value['title'] );

The the wp_filter_kses function is adding the slash which applies on this title_save_pre filter. For some reason, the function does:

<?php
return addslashes( wp_kses( stripslashes( $data ), current_filter() ) );

Which is the reason for the slash being injected, because addslashes() adds slashes before apostrophes, even though there wasn't a slash that got stripped originally by stripslashes. So, to me this function and (wp_filter_post_kses like it) looks like it is doing the wrong thing. The easiest way I see to fix the issue is to bypass those KSES functions altogether with something like nav-menu-item-kses-filter-fix.diff.

Attachments (3)

35869.0.diff (2.3 KB) - added by westonruter 21 months ago.
35869.1.diff (5.6 KB) - added by westonruter 21 months ago.
Update unit tests to include \o/
35869.2.diff (5.6 KB) - added by westonruter 21 months ago.
Add apostrophes to test input

Download all attachments as: .zip

Change History (10)

#1 @westonruter
21 months ago

  • Milestone changed from Awaiting Review to 4.5
  • Owner set to westonruter
  • Status changed from new to accepted
  • Version set to 4.3

The problem with wp_filter_kses (and wp_filter_post_kses, see #1697) are that they both presume the data is already slashed. I suppose the simplest way to deal with this is to wp_slash() the data if we detect that title_save_pre has the wp_filter_kses filter, and likewise for the other filters.

This issue can be currently seen on any install where the user cannot unfiltered_html, for example on multisite installs.

#2 @westonruter
21 months ago

  • Keywords has-patch commit added; needs-patch removed

Turns out the issue goes a bit deeper. There is more needed than just doing wp_slash() before passing into wp_filter_kses filter, and then calling wp_unslash() on the return value of the filter. This handles it for the WP_Customize_Nav_Menu_Item_Setting::sanitize() logic and previewing the change. But when update is called, any slashes used in the content, e.g. “Yay! \o/” would get saved as “Yay! o/”. So the WP_Customize_Nav_Menu_Item_Setting::update() method also needs to be updated to ensure the setting is passed through wp_slash() in its way into wp_update_nav_menu_item(), and this function needs to be updated to note that it expects pre-slashed input (sadly).

This is all done in 35869.0.diff.

@westonruter
21 months ago

Update unit tests to include \o/

@westonruter
21 months ago

Add apostrophes to test input

#3 @westonruter
21 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 36608:

Customize: Fix previewing and updating of nav menu items containing slashed/slashable characters.

Prevents slashes from being added when a user without unfiltered_html previews a nav menu item containing an apostrophe or some other slashable character, and prevents the loss of an intentional slash (e.g. "\o/") when saving a nav menu item, regardless of capability.

Fixes #35869.

#4 @pento
21 months ago

  • Keywords has-patch commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

#5 @westonruter
21 months ago

  • Status changed from reopened to accepted

#6 @westonruter
21 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 36610:

Customize: Update unit test for WP_Customize_Nav_Menu_Item_Setting::value_as_wp_post_nav_menu_item() to account for slashing if user can't unfiltered_html.

Fixes unit tests which broke under multisite config after [36608].

Fixes #35869.

#7 @westonruter
21 months ago

In 36645:

Docs: Use markdown instead of HTML for code formatting.

Fixes phpdoc usage in [36622], [36608], [35724], [35307].

See #35898.
See #35869.
See #34738.
See #33552.

Note: See TracTickets for help on using tickets.