diff --git a/docs/changelogs/v4.7.7.md b/docs/changelogs/v4.7.7.md index ac104dc1bb40..dc864f20d111 100644 --- a/docs/changelogs/v4.7.7.md +++ b/docs/changelogs/v4.7.7.md @@ -17,6 +17,7 @@ Release Date: Unreleased ## Bugs Fixed - **CORS:** Fixed a bug where `Cors::hasResponseHeaders()` matched header substrings via `str_contains()`, causing false positives when existing exposed headers shared a prefix with configured exposed headers. +- **HTTP:** Fixed a bug where `SiteURIFactory::parseRequestURI()` truncated the query string to a single character when using query-based routing (`/?route?param=val`). - **HTTP:** Fixed a bug where `UploadedFile::move()` and `UploadedFile::store()` on Windows could store a file under a different name than `getName()` reported, or write its contents to an NTFS alternate data stream. Filenames ending in a period or space or containing a colon are now rejected, including explicitly supplied names. - **Security:** Fixed a bug where `Security::__construct()` threw a `TypeError` when the CSRF cookie was an array, causing an HTTP 500 response for affected requests. - **Worker Mode:** Fixed a bug where every request that ended with an uncaught exception, including a `404 Page Not Found` without a 404 override, called `exit()` in the exception handler. This ended the worker script, so FrankenPHP booted a new one, and the worker loop skipped its cleanup after the request. The exception handler now returns to the worker loop. diff --git a/system/HTTP/SiteURIFactory.php b/system/HTTP/SiteURIFactory.php index 3533b8383c6a..56fa6dacea6f 100644 --- a/system/HTTP/SiteURIFactory.php +++ b/system/HTTP/SiteURIFactory.php @@ -160,7 +160,7 @@ private function parseRequestURI(): string if (trim($path, '/') === '' && str_starts_with($query, '/')) { $parts = explode('?', $query, 2); $path = $parts[0]; - $newQuery = $query[1] ?? ''; + $newQuery = $parts[1] ?? ''; $this->superglobals->setServer('QUERY_STRING', $newQuery); } else { diff --git a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php index a61f19a2fefe..43a80e58d1e1 100644 --- a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php +++ b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php @@ -258,6 +258,73 @@ public function testQueryStringWithQueryString(): void $this->assertSame(['code' => 'good'], $_GET); } + public function testRequestURIWithQueryBasedRoutingAndQueryParameters(): void + { + // /index.php?/ci/woot?code=good#pos + service('superglobals') + ->setServer('REQUEST_URI', '/index.php?/ci/woot?code=good') + ->setServer('SCRIPT_NAME', '/index.php') + ->setGet('/ci/woot?code', 'good'); + + $factory = $this->createSiteURIFactory(service('superglobals')->getServerArray()); + + $expected = 'ci/woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + $this->assertSame('code=good', $_SERVER['QUERY_STRING']); // @phpstan-ignore codeigniter.superglobalsOffsetAccess (checks the live superglobal, not the snapshot service) + $this->assertSame(['code' => 'good'], $_GET); + } + + public function testRequestURIWithQueryBasedRoutingWithoutAdditionalQuery(): void + { + // /index.php?/ci/woot + service('superglobals') + ->setServer('REQUEST_URI', '/index.php?/ci/woot') + ->setServer('SCRIPT_NAME', '/index.php') + ->setGet('/ci/woot', ''); + + $factory = $this->createSiteURIFactory(service('superglobals')->getServerArray()); + + $expected = 'ci/woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + $this->assertSame('', $_SERVER['QUERY_STRING']); // @phpstan-ignore codeigniter.superglobalsOffsetAccess (checks the live superglobal, not the snapshot service) + $this->assertSame([], $_GET); + } + + public function testRequestURIWithQueryBasedRoutingPreservesSubsequentQuestionMarks(): void + { + // /index.php?/ci/woot?redirect=/a?b=1 + service('superglobals') + ->setServer('REQUEST_URI', '/index.php?/ci/woot?redirect=/a?b=1') + ->setServer('SCRIPT_NAME', '/index.php') + ->setGet('/ci/woot?redirect', '/a?b=1'); + + $factory = $this->createSiteURIFactory(service('superglobals')->getServerArray()); + + $expected = 'ci/woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + // Only the first '?' separates the path from the query string, so the + // remaining question marks must be kept inside the query string. + $this->assertSame('redirect=/a?b=1', $_SERVER['QUERY_STRING']); // @phpstan-ignore codeigniter.superglobalsOffsetAccess (checks the live superglobal, not the snapshot service) + $this->assertSame(['redirect' => '/a?b=1'], $_GET); + } + + public function testRequestURIWithQueryBasedRoutingWithTrailingQuestionMark(): void + { + // /index.php?/ci/woot? + service('superglobals') + ->setServer('REQUEST_URI', '/index.php?/ci/woot?') + ->setServer('SCRIPT_NAME', '/index.php') + ->setGet('/ci/woot?', ''); + + $factory = $this->createSiteURIFactory(service('superglobals')->getServerArray()); + + $expected = 'ci/woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + // A trailing '?' keeps an empty query string, not the '?' itself. + $this->assertSame('', $_SERVER['QUERY_STRING']); // @phpstan-ignore codeigniter.superglobalsOffsetAccess (checks the live superglobal, not the snapshot service) + $this->assertSame([], $_GET); + } + public function testQueryStringRefreshesRequestWithoutOverwritingCustomValues(): void { if (! $this->effectiveRequestOrderIncludesGet()) {