Skip to content

8B. Read the request the way core wrote it at priority 5 - #67

Open
nikolaystrikhar wants to merge 1 commit into
49-merge-notice-tells-the-truthfrom
50-unslash-after-core-slashes
Open

8B. Read the request the way core wrote it at priority 5#67
nikolaystrikhar wants to merge 1 commit into
49-merge-notice-tells-the-truthfrom
50-unslash-after-core-slashes

Conversation

@nikolaystrikhar

Copy link
Copy Markdown
Contributor

What: drops wp_unslash() from the three reads that run at plugins_loaded priority 5 — Conflict\Resolver's $_SERVER['REQUEST_URI'], and Conflict\Gatekeeper's action arg and script-name candidate.

Usage: /wp-admin/edit.php?s=C:\projects now redirects back to that search rather than to one for C:projects, and ?action=\ is refused rather than resolved out from under core.

Why this way:

Core slashes after us, not before. wp_magic_quotes() is what wp_unslash() undoes, and wp-settings.php calls it after do_action( 'plugins_loaded' ) — so at priority 5 it is a plain stripslashes_deep() over the URL bar.

The gate was the expensive half. ?action=\ strips to '' and ?action2=-\1 to -1, which are exactly the two values the gate reads as asking for nothing — a bypass in the class whose job is refusing those requests.

Conflict\Rewriter keeps both of its uses. It runs from wp_admin_notice_markup at render time, after the slashing, where the idiom is right.

`wp_magic_quotes()` is what adds the slashes `wp_unslash()` takes off, and
wp-settings.php calls it *after* `do_action( 'plugins_loaded' )`. Both
conflict gates and the redirect run at priority 5, so every value they read
is exactly as the SAPI left it and `wp_unslash()` there is a plain
`stripslashes_deep()` over user input.

It costs a destination and a gate. An admin at
/wp-admin/edit.php?s=C:\projects who trips a conflict is sent back to a
search for "C:projects" -- re-rendering the screen the user asked for being
the entire point of the redirect -- and in the action gate `?action=\`
strips to '' while `?action2=-\1` strips to '-1', which are the two values
that gate reads as "nothing is being asked for". Both are then admitted,
resolved and exited out from under work core still goes on to dispatch, by
the class whose job is refusing exactly those requests.

`Conflict\Rewriter` keeps both of its uses: it runs from
`wp_admin_notice_markup` at admin render time, long after the slashing,
where the idiom is the correct one.

Where these can still run after the slashing is the inline fallback a
too-late boot reports, and the direction is safe there: a slashed action arg
only ever reads as more of an action than it is, and a slashed URI costs a
stray backslash in a re-encoded query arg rather than a deleted one.
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Enterprise

Run ID: 9f2014b9-36d4-41f7-b1b6-54b7c9c7084d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@nikolaystrikhar
nikolaystrikhar force-pushed the 50-unslash-after-core-slashes branch 2 times, most recently from 984dbc7 to f9e5122 Compare August 24, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant