24: Read the two request values the way core wrote them - #38
Open
nikolaystrikhar wants to merge 1 commit into
Open
24: Read the two request values the way core wrote them#38nikolaystrikhar wants to merge 1 commit into
nikolaystrikhar wants to merge 1 commit into
Conversation
sanitize_text_field() ran over query values wp_parse_str() had already
url-decoded, and it deletes every %[a-f0-9]{2} match and strips markup. So a
conflict resolved on edit.php?s=100%25ab re-rendered the screen searching for
100. The point of the redirect is to re-request what the user asked for, and
http_build_query( …, PHP_QUERY_RFC3986 ) is what actually prevents breakout;
all the sanitize call added was the mangling. Line breaks are stripped on
their own now, which is the one property the Location header needs.
The rewriter verified its nonce against a sanitised basename, but core mints
that nonce from the raw wp_unslash( $_REQUEST['plugin'] ). Any basename
sanitizing alters -- a folder name with a percent sequence, a leading space --
failed verification, and the activation-error screen kept core's wording on
exactly the conflict this feature exists to explain. An explicit is_string()
does the refusing that sanitize_text_field() was incidentally doing.
Also anchors the screen-name regex with \z: PCRE $ matches before a trailing
newline too.
nikolaystrikhar
force-pushed
the
24-request-values
branch
from
August 13, 2026 13:32
1c5f360 to
bd1b5ac
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What:
Conflict\Redirectorstrips line breaks from query values instead of runningsanitize_text_field()over them, andConflict\Rewriterverifies_error_nonceagainst the value core actually signed.Usage:
Why this way:
The values were already decoded, and sanitizing them destroyed them.
wp_parse_str()decodes before the sanitize ran, and_sanitize_text_fields()deletes every%[a-f0-9]{2}match and strips markup. Re-requesting the screen the user asked for is the entire point of the redirect.http_build_query( …, PHP_QUERY_RFC3986 )is what prevents breakout; the sanitize call only added mangling. Line breaks are stripped on their own, which is the one property aLocationheader needs.Core signs the raw basename.
wp-admin/plugins.phpmintsplugin-activation-error_{$plugin}fromwp_unslash( $_REQUEST['plugin'] ), so a basename sanitizing altered failed verification and left core's wording on the one screen this feature exists to reword. An explicitis_string()now does the refusing thatsanitize_text_field()was doing by accident.