From e1349b469e41039a6145ffc2e0908997b095033d Mon Sep 17 00:00:00 2001 From: Erik van der Bas Date: Tue, 7 Jul 2026 22:56:11 +0200 Subject: [PATCH 1/3] fix: prevent WordPress canonical redirects from hijacking matched routes 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 --- Routes.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Routes.php b/Routes.php index 7f9aeea..1aeb8a0 100755 --- a/Routes.php +++ b/Routes.php @@ -255,6 +255,13 @@ function () use ($query) { $priority ); + // Prevent WordPress's own canonical redirect logic from hijacking a matched + // route. This most commonly happens when the requested slug also matches an + // attachment: WordPress will otherwise redirect straight to the raw upload + // file via `redirect_canonical()` before `template_include` ever runs. + // @see https://github.com/Upstatement/routes/issues/13 + add_filter('redirect_canonical', '__return_false'); + return true; } From a6f21b52043716a348c2c64db2b364824af7d2fb Mon Sep 17 00:00:00 2001 From: Erik van der Bas Date: Tue, 7 Jul 2026 22:56:16 +0200 Subject: [PATCH 2/3] test: cover issue #13 attachment-redirect fix and its edge cases - 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. --- tests/RoutesTest.php | 92 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/RoutesTest.php b/tests/RoutesTest.php index 1a66342..4d69e6f 100644 --- a/tests/RoutesTest.php +++ b/tests/RoutesTest.php @@ -392,6 +392,98 @@ function ($params) { $this->assertCount(1, $matches); } + public function testRouteSurvivesAttachmentRedirect() + { + // Regression test for issue #13: WordPress redirects any request that resolves + // to an attachment straight to the raw upload file, since attachment pages are + // disabled by default (`wp_attachment_pages_enabled`). If the requested slug + // also matches a mapped route, that redirect must not hijack the route. + $attachment_id = $this->factory->attachment->with_image()->create( + [ + 'post_name' => 'books', + 'post_title' => 'Books', + ] + ); + + global $matches; + $matches = []; + + Routes::map( + 'books', + function () use ($attachment_id) { + global $matches; + $matches = []; + $matches[] = true; + + // Force the main query to resolve as the attachment, mirroring what + // WordPress's own request parsing can produce for a colliding slug. + Routes::load( + __DIR__ . '/Supports/single.php', + false, + [ + 'attachment' => 'books', + 'attachment_id' => $attachment_id, + ] + ); + } + ); + + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_SERVER['REQUEST_URI'] = '/books/'; + $this->matchRoutes(); + + $response = $this->get(home_url('/books/')); + + $response->assertOk(); + $this->assertEquals(1, count($matches)); + } + + public function testUnmatchedRouteDoesNotAffectCanonicalRedirects() + { + // Sanity check for the fix above: Routes must only cancel WordPress's canonical + // redirect when a route has actually matched and rendered a template. Regular + // WordPress routing -- like redirecting a legacy ?p=123 link to its canonical + // permalink -- must keep working untouched when no route matches at all. + $post_id = $this->factory->post->create(['post_title' => 'Hello World']); + + // No Routes::map() call in this test, so match_current_request() is a no-op. + $this->matchRoutes(); + + $response = $this->get(home_url('/?p=' . $post_id)); + + $response->assertRedirect(get_permalink($post_id)); + } + + public function testMatchedRouteWithoutLoadDoesNotAffectCanonicalRedirects() + { + // A route can match and run its callback without ever calling Routes::load() + // (e.g. it only performs a side effect). Since no template was set, the fix + // must not touch WordPress's canonical redirect handling for that request. + $post_id = $this->factory->post->create(['post_title' => 'Hello World']); + + global $matches; + $matches = []; + + Routes::map( + 'no-op', + function () { + global $matches; + $matches = []; + $matches[] = true; + } + ); + + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_SERVER['REQUEST_URI'] = '/no-op/'; + $this->matchRoutes(); + + $this->assertEquals(1, count($matches)); + + $response = $this->get(home_url('/?p=' . $post_id)); + + $response->assertRedirect(get_permalink($post_id)); + } + public function matchRoutes() { Routes::get_instance()->match_current_request(); From 491daf11a13cb18a05404322cb2bc0fe906533f0 Mon Sep 17 00:00:00 2001 From: Jared Novack Date: Fri, 10 Jul 2026 21:08:08 -0400 Subject: [PATCH 3/3] test: use assertCount() for match assertions in canonical-redirect tests 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) --- tests/RoutesTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/RoutesTest.php b/tests/RoutesTest.php index 4d69e6f..9e4e1a2 100644 --- a/tests/RoutesTest.php +++ b/tests/RoutesTest.php @@ -435,7 +435,7 @@ function () use ($attachment_id) { $response = $this->get(home_url('/books/')); $response->assertOk(); - $this->assertEquals(1, count($matches)); + $this->assertCount(1, $matches); } public function testUnmatchedRouteDoesNotAffectCanonicalRedirects() @@ -477,7 +477,7 @@ function () { $_SERVER['REQUEST_URI'] = '/no-op/'; $this->matchRoutes(); - $this->assertEquals(1, count($matches)); + $this->assertCount(1, $matches); $response = $this->get(home_url('/?p=' . $post_id));