Fix: Canonical redirects hijacking matched routes (#13)

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;
         }
 

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->assertCount(1, $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->assertCount(1, $matches);
+
+		$response = $this->get(home_url('/?p=' . $post_id));
+
+		$response->assertRedirect(get_permalink($post_id));
+	}
+
 	public function matchRoutes()
 	{
 		Routes::get_instance()->match_current_request();