autofocus query string parameter in customizer url is broken in WP6.6-RC2

[jamesros161]

When using a query string parameter to autofocus in the customizer, it is broken by using the sanitize_text_field() function.

Example: customize.php?autofocus%5Bsection%5D=colors

Line 89 of /wp-admin/customize.php uses sanitize_text_field to sanitize the $_REQUEST['autofocus'] variable. However, this variable is an array, not a string. Therefore it will always return an empty string when being sanitized this way.

[dlh]

Welcome back to Trac, @jamesros161! Thanks for the report.

I can replicate the problem, which appears to have been introduced in [58069].

I'm adding this to the 6.6 milestone for visibility, but I think a fix is more likely a candidate for 6.6.1 at this point.

Trac Ticket - Core-61561

## Summary

Fixes an issue with the autofocus query string parameter in the WordPress Customizer (customize.php). Previously, sanitize_text_field() was used on $_REQUESTautofocus?, which is an array, resulting in an empty string.

## Description

### Problem

The Customizer's autofocus functionality was broken due to incorrect sanitization of the $_REQUESTautofocus? variable. When sanitize_text_field() is used on an array, it returns an empty string.

### Example

customize.php?autofocus%5Bsection%5D=colors

### Solution

To resolve this issue, the keys and values of the $_REQUESTautofocus? array are sanitized separately and then combined into a new array.

###Changes Made

• Sanitize the keys of the $_REQUESTautofocus? array using sanitize_key().
• Sanitize the values of the $_REQUESTautofocus? array using sanitize_text_field().
• Combine the sanitized keys and values into a new array using array_combine().

[audrasjb]

Given we're pretty close to 6.6 final release and as it's not a blocker for the release, I'm moving this ticket to 6.6.1.

[hellofromTonya]

Current status:

• Needs testing instructions.
• Needs a code review of the patch.
• Needs testing (test reports).

Likely a candidate for 6.6.2, rather than 6.6.1.

@hellofromtonya why me? 🤔

Ah, because of https://core.trac.wordpress.org/changeset/58069. Context would have been helpful.

Hi @swissspidy , I have addressed the feedback and made the necessary changes. Assigning you again for re-review.

[hellofromTonya]

Test Report

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/6974

Environment

• OS: macOS
• Web Server: Nginx
• PHP: 8.3
• WordPress: 6.6.1-alpha-58737-src
• Browser: Firefox 128.0
• Theme: Twenty Twenty-One
• Active Plugins: None.

Steps

1. Open Customizer.
2. Add the following URL params autofocus%5Bsection%5D=colors to the end of the URL, e.g. ​http://localhost:8889/wp-admin/customize.php?return=%2Fwp-admin%2F&autofocus%5Bsection%5D=colors).
3. Observe if the Colors section opens.

Actual Results

When reproducing a bug/defect (before applying the patch:

• ❌ Error occurs. Can reproduce.

When testing the bugfix patch (after applying the patch):

• ✅ Issue resolved.

[hellofromTonya]

Patch: ​https://github.com/WordPress/wordpress-develop/pull/6974

• PR is approved by @swissspidy
• Test report confirms before and after.

Moving this fix back into 6.6.1. Self-assigning for commit.

[hellofromTonya]

Tested this on 6.5.4. Can reproduce the issue there too. So this bug was not introduced in 6.6 cycle. Aha. Moving it to 6.7.

[ironprogrammer]

Test Report

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/6974 (I left a couple of suggestions on the PR so figured I'd drop in a test report as well).

Environment

• Hardware: MacBook Pro Apple M1 Pro
• OS: macOS 14.5
• Browser: Safari 17.5
• Server: nginx/1.27.0
• PHP: 8.3.9
• MySQL: 8.0.27
• WordPress: 6.7-alpha-58576-src
• Theme: twentytwenty v2.7

Actual Results

• ✅ Issue resolved with patch. E.g. /wp-admin/customize.php?return=%2Fwp-admin%2F +
  • Site Identity: &autofocus%5Bsection%5D=title_tagline
  • Colors: &autofocus%5Bsection%5D=colors
  • Additional CSS: &autofocus%5Bsection%5D=custom_css

[hellofromTonya]

Replying to hellofromTonya:

Tested this on 6.5.4. Can reproduce the issue there too. So this bug was not introduced in 6.6 cycle. Aha. Moving it to 6.7.

After evaluating the fix, [58069] introduced the issue for this fix by treating the autofocus as a string, rather than as a array. As such, the regression of changing the data type was introduced in the 6.6 cycle.

Moving this to 6.6.2 for only the fix that narrowly focuses on restoring the array data type.

Marking it as changes-requested, as the relocating of wp_unslash() was not introduced in the 6.6 cycle.

@hellofromtonya As I suggested that change, I disagree. https://core.trac.wordpress.org/changeset/58069 made an incorrect change in that it a) called sanitize_text_field on an array and b) called sanitize_text_field before calling wp_unslash() (rather than after). Both are introduced in the 6.6 cycle and it makes sense to fix this properly, _together_

Hi @hellofromtonya , what should be done, should I revert back the wp_unslash() changes?

As I suggested that change, I disagree. https://core.trac.wordpress.org/changeset/58069 made an incorrect change in that it a) called sanitize_text_field on an array and b) called sanitize_text_field before calling wp_unslash() (rather than after). Both are introduced in the 6.6 cycle and it makes sense to fix this properly, together

I didn't previously see that @swissspidy ​suggested the change to relocate `wp_unslash()` to occur _before_ sanitizing:

Also just noticing the other wp_unslash calls further below are incorrect, they should be happening before the sanitizing.

Noting, Core uses both approaches of wp_unslash() before and after sanitizing. For example, in wp-signup.php, $_POST['WPLANG'] is sanitized first and then unslashed.

What about this specific bugfix, which is better?
Unsplashing _before_ does not contribute to the bug itself. That said, I agree with @swissspidy that it's better to unsplash before sanitizing.

The reason for me asking to revert that change is to minimize the code changes to just the bug itself for the 6.6.2 minor cycle. But with this new understanding @swissspidy shared, I withdraw that suggestion and will move forward with committing the change.

Thank you everyone for your patience and contributions.

[hellofromTonya]

​As noted here in the PR, I now better understand the reasoning for relocating the wp_unslash() as part of this bug fix and have removed my suggestion to revert that change. Re-marking the patch for commit and prepping it now.

[hellofromTonya]

In 58804:

Customize: Sanitize autofocus URL parameter as an array.

[58069] introduced calling sanitize_text_field() with $_REQUEST['autofocus'] (which is an array) and setting its default to a string. This fix restores the array data type for autofocus.

The fix also relocates the unsplash for url, return, and autofocus before sanitizing.

Follow-up to [58069], [34269], [29026], [21028].

Props jamesros161, swissspidy, dlh, audrasjb, hellofromTonya, ironprogrammer.
Fixes #61561.

[hellofromTonya]

Reopening for 2nd committer review to backport [58804] to the 6.6 branch.

After committing, I noticed that I missed adding @debarghyabanerjee (authored the patch) to the commit Props. I manually added them to Core Props.

Committer Note: please add debarghyabanerjee in the props for the backport commit.

Committed via https://core.trac.wordpress.org/changeset/58804. Thank you everyone for your contributions ⭐

Hi @hellofromtonya , it seems that my ID is missing from the props. Can you please take a look into it. Thanks :-)

it seems that my ID is missing from the props. Can you please take a look into it. Thanks :-)

Yeah I saw that after I committed. Sorry about that. As I noted here in the Trac ticket, I manually added you in Core's Props and noted for the committer who does the backport to 6.6 branch to add you to the commit props.

[debarghyabanerjee]

Thanks @hellofromTonya , Looking forward to it :-)

Hi, @Debarghya-Banerjee -- You can refer to the "Unlinked Props" section of ​this comment, which has instructions to make sure you're automatically properly credited in the future. And thanks again for the PR!

Hi, @Debarghya-Banerjee -- You can refer to the "Unlinked Props" section of ​this comment, which has instructions to make sure you're automatically properly credited in the future. And thanks again for the PR!

Hi @ironprogrammer , I actually had some issues, but before the PR got approved, I fixed it, you can check my ID on the props as well. Thanks for youe response!

[joedolson]

#61780 was marked as a duplicate.

[sabernhardt]

#61803 was marked as a duplicate.

[hellofromTonya]

#61579 was marked as a duplicate.

[joedolson]

In 58973:

Customize: Sanitize autofocus URL parameter as an array.

[58069] introduced calling sanitize_text_field() with $_REQUESTautofocus? (which is an array) and setting its default to a string. This fix restores the array data type for autofocus.

The fix also relocates the unslash for url, return, and autofocus before sanitizing.

Follow-up to [58069], [34269], [29026], [21028].

Reviewed by joedolson.
Merges [58804] to the 6.6 branch.

Props jamesros161, swissspidy, dlh, audrasjb, hellofromTonya, ironprogrammer, debarghyabanerjee.
Fixes #61561.