Fix: Canonical redirects hijacking matched routes (#13)#53
Merged
Conversation
WordPress's redirect_canonical() runs on template_redirect and can send a 301 to the raw upload file whenever the resolved query is_attachment(), since attachment pages are disabled by default. This fires before template_include, so a route whose slug collides with a media attachment's slug gets silently overridden. Routes::load() now cancels the canonical redirect (via the documented redirect_canonical filter) whenever it successfully sets up a template, since a matched route should always take full control of the response. Fixes #13
- testRouteSurvivesAttachmentRedirect reproduces the original bug and verifies the route wins instead of being redirected to the attachment. - testUnmatchedRouteDoesNotAffectCanonicalRedirects and testMatchedRouteWithoutLoadDoesNotAffectCanonicalRedirects confirm regular WordPress canonical redirects are untouched when Routes isn't actively rendering a response.
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents WordPress canonical redirects from overriding matched custom routes that load templates.
Changes:
- Disables canonical redirects after
Routes::load()successfully selects a template. - Adds regression and scope-isolation tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
Routes.php |
Suppresses canonical redirects for loaded routes. |
tests/RoutesTest.php |
Tests attachment collisions and unaffected redirects. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Aligns the two new tests with the assertCount() convention established in 731fc92, matching the adjacent existing tests rather than the older assertEquals(1, count(...)) idiom. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jarednova
force-pushed
the
canolical-attachment-redirect-fix
branch
from
July 11, 2026 01:08
5024c6b to
491daf1
Compare
This was referenced Jul 11, 2026
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
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.
Problem
When a mapped route's slug happened to match the slug of a media attachment (e.g. an image), WordPress would redirect the request straight to the raw upload file instead of rendering the route.
Root cause: WordPress's
redirect_canonical()runs ontemplate_redirectand issues a301to the raw attachment file whenever the resolved main queryis_attachment()is true — this is expected behavior since attachment pages are disabled by default (wp_attachment_pages_enabled). The problem is that this redirect fires (andexits) before thetemplate_includefilter thatRoutes::load()registers ever runs, so a matched route gets silently overridden.Fix
Routes::load()now cancels WordPress's canonical redirect via the documentedredirect_canonicalfilter (add_filter('redirect_canonical', '__return_false')) whenever it successfully sets up a template. Once a route has matched and is rendering a response, it should always take full control — WordPress's own canonical/attachment redirect logic should no longer have a say.This only applies when a route actually resolves to a template via
Routes::load(); requests that don't match any route, or that match a route which never callsRoutes::load(), are completely unaffected.Tests
testRouteSurvivesAttachmentRedirect— reproduces the original bug: creates a colliding attachment, maps a route to the same slug, forces the main query to resolve as that attachment, and asserts the response is200(not a301to the raw file). Confirmed this test fails without the fix.testUnmatchedRouteDoesNotAffectCanonicalRedirects— sanity check that regular WordPress canonical redirects (e.g. legacy?p=123links) still work when no route matches at all.testMatchedRouteWithoutLoadDoesNotAffectCanonicalRedirects— sanity check that a route matching and running its callback without callingRoutes::load()doesn't affect canonical redirects either.All 25 tests pass, verified stable across multiple random test orderings.
Files changed
Routes.php— the fixtests/RoutesTest.php— regression + sanity testsFixes #13