Skip to content

Bad Requests: Fix Pattern Directory URI check to match request path correctly#585

Closed
obenland wants to merge 1 commit intoWordPress:trunkfrom
obenland:fix/pattern-directory-uri-check
Closed

Bad Requests: Fix Pattern Directory URI check to match request path correctly#585
obenland wants to merge 1 commit intoWordPress:trunkfrom
obenland:fix/pattern-directory-uri-check

Conversation

@obenland
Copy link
Copy Markdown
Member

@obenland obenland commented Apr 1, 2026

Summary

  • Replace str_contains( $_SERVER['REQUEST_URI'], 'wordpress.org/patterns/' ) with str_starts_with( $_SERVER['REQUEST_URI'], '/patterns/' ) for a more precise URI match
  • REQUEST_URI is a path (e.g. /patterns/...), not a full URL, so matching against wordpress.org/patterns/ was relying on a substring that wouldn't actually appear in the value

Test plan

  • Verify Pattern Directory pages at /patterns/ still have non-scalar query parameters blocked
  • Verify unrelated pages are not affected by the check

🤖 Generated with Claude Code

The previous str_contains() check could match unrelated URLs that happen
to contain 'wordpress.org/patterns/' in the path. Using str_starts_with()
with '/patterns/' is more precise and correct for matching the request URI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 1, 2026 14:02
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props obenland.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@obenland obenland changed the title Bad Requests: Use str_starts_with() for Pattern Directory URI check Bad Requests: Fix Pattern Directory URI check to match request path correctly Apr 1, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Pattern Directory request detection used to block non-scalar query parameters, switching from a substring match to a stricter URI prefix check.

Changes:

  • Replace str_contains( $_SERVER['REQUEST_URI'], 'wordpress.org/patterns/' ) with str_starts_with( $_SERVER['REQUEST_URI'], '/patterns/' ) for Pattern Directory detection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +212 to 215
if ( ! str_starts_with( $_SERVER['REQUEST_URI'], '/patterns/' ) ) {
return;
}

Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str_starts_with( $_SERVER['REQUEST_URI'], '/patterns/' ) will not match requests like /patterns?curation[$in][]=all (no trailing slash before the query string) or /patterns (no trailing slash). That creates a bypass for the non-scalar query-parameter blocking this hook is meant to enforce. Consider normalizing to the path component (e.g., via parse_url(..., PHP_URL_PATH)) and checking for the /patterns prefix with an optional trailing slash, while still avoiding matches like /patterns-foo.

Suggested change
if ( ! str_starts_with( $_SERVER['REQUEST_URI'], '/patterns/' ) ) {
return;
}
$path = parse_url( $_SERVER['REQUEST_URI'] ?? '', PHP_URL_PATH );
if ( ! is_string( $path ) ) {
return;
}
// Match /patterns and /patterns/... but not /patterns-foo.
if ( $path !== '/patterns' && ! str_starts_with( $path, '/patterns/' ) ) {
return;
}

Copilot uses AI. Check for mistakes.
@bazza bazza closed this in 556ba52 Apr 1, 2026
@obenland obenland deleted the fix/pattern-directory-uri-check branch April 1, 2026 16:49
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.

2 participants