From 6318c23a17de375fbd25caeb87f37521a9ae6282 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 15 Feb 2023 13:45:16 +0900 Subject: [PATCH 01/11] feat: add SiteURIFactory --- system/HTTP/IncomingRequest.php | 9 + system/HTTP/SiteURIFactory.php | 251 ++++++++++++++++++ .../SiteURIFactoryDetectRoutePathTest.php | 227 ++++++++++++++++ tests/system/HTTP/SiteURIFactoryTest.php | 91 +++++++ 4 files changed, 578 insertions(+) create mode 100644 system/HTTP/SiteURIFactory.php create mode 100644 tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php create mode 100644 tests/system/HTTP/SiteURIFactoryTest.php diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index 2554385b7ba6..99bbf223f405 100755 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -235,6 +235,8 @@ protected function detectURI(string $protocol, string $baseURL) /** * Detects the relative path based on * the URIProtocol Config setting. + * + * @deprecated Moved to SiteURIFactory. */ public function detectPath(string $protocol = ''): string { @@ -265,6 +267,8 @@ public function detectPath(string $protocol = ''): string * fixing the query string if necessary. * * @return string The URI it found. + * + * @deprecated Moved to SiteURIFactory. */ protected function parseRequestURI(): string { @@ -323,6 +327,8 @@ protected function parseRequestURI(): string * Parse QUERY_STRING * * Will parse QUERY_STRING and automatically detect the URI from it. + * + * @deprecated Moved to SiteURIFactory. */ protected function parseQueryString(): string { @@ -495,6 +501,9 @@ public function setPath(string $path, ?App $config = null) return $this; } + /** + * @deprecated Moved to SiteURIFactory. + */ private function determineHost(App $config, string $baseURL): string { $host = parse_url($baseURL, PHP_URL_HOST); diff --git a/system/HTTP/SiteURIFactory.php b/system/HTTP/SiteURIFactory.php new file mode 100644 index 000000000000..e96cc2781f85 --- /dev/null +++ b/system/HTTP/SiteURIFactory.php @@ -0,0 +1,251 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\HTTP; + +use CodeIgniter\HTTP\Exceptions\HTTPException; +use Config\App; + +class SiteURIFactory +{ + /** + * @var array Superglobal SERVER array + */ + private array $server; + + private App $appConfig; + + /** + * @param array $server Superglobal $_SERVER array + */ + public function __construct(array $server, App $appConfig) + { + $this->server = $server; + $this->appConfig = $appConfig; + } + + /** + * Create the current URI object from superglobals. + * + * This method updates superglobal $_SERVER and $_GET. + */ + public function createFromGlobals(): SiteURI + { + $routePath = $this->detectRoutePath(); + + return $this->createURIFromRoutePath($routePath); + } + + /** + * Create the SiteURI object from URI string. + * + * @internal Used for testing purposes only. + */ + public function createFromString(string $uri): SiteURI + { + // Validate URI + if (filter_var($uri, FILTER_VALIDATE_URL) === false) { + throw HTTPException::forUnableToParseURI($uri); + } + + $parts = parse_url($uri); + + if ($parts === false) { + throw HTTPException::forUnableToParseURI($uri); + } + + $query = $fragment = ''; + if (isset($parts['query'])) { + $query = '?' . $parts['query']; + } + if (isset($parts['fragment'])) { + $fragment = '#' . $parts['fragment']; + } + + $relativePath = $parts['path'] . $query . $fragment; + + return new SiteURI($this->appConfig, $relativePath, $parts['host'], $parts['scheme']); + } + + /** + * Detects the current URI path relative to baseURL based on the URIProtocol + * Config setting. + * + * @param string $protocol URIProtocol + * + * @return string The route path + * + * @internal Used for testing purposes only. + */ + public function detectRoutePath(string $protocol = ''): string + { + if ($protocol === '') { + $protocol = $this->appConfig->uriProtocol; + } + + switch ($protocol) { + case 'REQUEST_URI': + $routePath = $this->parseRequestURI(); + break; + + case 'QUERY_STRING': + $routePath = $this->parseQueryString(); + break; + + case 'PATH_INFO': + default: + $routePath = $this->server[$protocol] ?? $this->parseRequestURI(); + break; + } + + return ($routePath === '/' || $routePath === '') ? '/' : ltrim($routePath, '/'); + } + + /** + * Will parse the REQUEST_URI and automatically detect the URI from it, + * fixing the query string if necessary. + * + * This method updates superglobal $_SERVER and $_GET. + * + * @return string The route path (before normalization). + */ + private function parseRequestURI(): string + { + if (! isset($this->server['REQUEST_URI'], $this->server['SCRIPT_NAME'])) { + return ''; + } + + // parse_url() returns false if no host is present, but the path or query + // string contains a colon followed by a number. So we attach a dummy + // host since REQUEST_URI does not include the host. This allows us to + // parse out the query string and path. + $parts = parse_url('http://dummy' . $this->server['REQUEST_URI']); + $query = $parts['query'] ?? ''; + $path = $parts['path'] ?? ''; + + // Strip the SCRIPT_NAME path from the URI + if ( + $path !== '' && isset($this->server['SCRIPT_NAME'][0]) + && pathinfo($this->server['SCRIPT_NAME'], PATHINFO_EXTENSION) === 'php' + ) { + // Compare each segment, dropping them until there is no match + $segments = $keep = explode('/', $path); + + foreach (explode('/', $this->server['SCRIPT_NAME']) as $i => $segment) { + // If these segments are not the same then we're done + if (! isset($segments[$i]) || $segment !== $segments[$i]) { + break; + } + + array_shift($keep); + } + + $path = implode('/', $keep); + } + + // This section ensures that even on servers that require the URI to + // contain the query string (Nginx) a correct URI is found, and also + // fixes the QUERY_STRING Server var and $_GET array. + if (trim($path, '/') === '' && strncmp($query, '/', 1) === 0) { + $parts = explode('?', $query, 2); + $path = $parts[0]; + $newQuery = $query[1] ?? ''; + + $this->server['QUERY_STRING'] = $newQuery; + $this->updateServer('QUERY_STRING', $newQuery); + } else { + $this->server['QUERY_STRING'] = $query; + $this->updateServer('QUERY_STRING', $query); + } + + // Update our global GET for values likely to have been changed + parse_str($this->server['QUERY_STRING'], $get); + $this->updateGetArray($get); + + return URI::removeDotSegments($path); + } + + private function updateServer(string $key, string $value): void + { + $_SERVER[$key] = $value; + } + + private function updateGetArray(array $array): void + { + $_GET = $array; + } + + /** + * Will parse QUERY_STRING and automatically detect the URI from it. + * + * This method updates superglobal $_SERVER and $_GET. + * + * @return string The route path (before normalization). + */ + private function parseQueryString(): string + { + $query = $this->server['QUERY_STRING'] ?? @getenv('QUERY_STRING'); + + if (trim($query, '/') === '') { + return '/'; + } + + if (strncmp($query, '/', 1) === 0) { + $parts = explode('?', $query, 2); + $path = $parts[0]; + $newQuery = $parts[1] ?? ''; + + $this->server['QUERY_STRING'] = $newQuery; + $this->updateServer('QUERY_STRING', $newQuery); + } else { + $path = $query; + } + + // Update our global GET for values likely to have been changed + parse_str($this->server['QUERY_STRING'], $get); + $this->updateGetArray($get); + + return URI::removeDotSegments($path); + } + + /** + * Create current URI object. + * + * @param string $routePath URI path relative to baseURL + */ + private function createURIFromRoutePath(string $routePath): SiteURI + { + $query = $this->server['QUERY_STRING'] ?? ''; + + $relativePath = $query !== '' ? $routePath . '?' . $query : $routePath; + + return new SiteURI($this->appConfig, $relativePath, $this->getHost()); + } + + /** + * @return string|null The current hostname. Returns null if no host header. + */ + private function getHost(): ?string + { + $host = null; + + $httpHostPort = $this->server['HTTP_HOST'] ?? null; + if ($httpHostPort !== null) { + [$httpHost] = explode(':', $httpHostPort, 2); + + if (in_array($httpHost, $this->appConfig->allowedHostnames, true)) { + $host = $httpHost; + } + } + + return $host; + } +} diff --git a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php new file mode 100644 index 000000000000..3b7715049a5c --- /dev/null +++ b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php @@ -0,0 +1,227 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\HTTP; + +use CodeIgniter\Test\CIUnitTestCase; +use Config\App; + +/** + * @backupGlobals enabled + * + * @internal + * + * @group Others + */ +final class SiteURIFactoryDetectRoutePathTest extends CIUnitTestCase +{ + protected function setUp(): void + { + parent::setUp(); + + $_GET = $_SERVER = []; + } + + private function createSiteURIFactory(array $server, ?App $appConfig = null): SiteURIFactory + { + $appConfig ??= new App(); + + return new SiteURIFactory($server, $appConfig); + } + + public function testDefault() + { + // /index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath()); + } + + public function testDefaultEmpty() + { + // / + $_SERVER['REQUEST_URI'] = '/'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = '/'; + $this->assertSame($expected, $factory->detectRoutePath()); + } + + public function testRequestURI() + { + // /index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURINested() + { + // I'm not sure but this is a case of Apache config making such SERVER + // values? + // The current implementation doesn't use the value of the URI object. + // So I removed the code to set URI. Therefore, it's exactly the same as + // the method above as a test. + // But it may be changed in the future to use the value of the URI object. + // So I don't remove this test case. + + // /ci/index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURISubfolder() + { + // /ci/index.php/popcorn/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/ci/index.php/popcorn/woot'; + $_SERVER['SCRIPT_NAME'] = '/ci/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'popcorn/woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURINoIndex() + { + // /sub/example + $_SERVER['REQUEST_URI'] = '/sub/example'; + $_SERVER['SCRIPT_NAME'] = '/sub/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'example'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURINginx() + { + // /ci/index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot?code=good'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURINginxRedirecting() + { + // /?/ci/index.php/woot + $_SERVER['REQUEST_URI'] = '/?/ci/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'ci/woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURISuppressed() + { + // /woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/woot'; + $_SERVER['SCRIPT_NAME'] = '/'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testQueryString() + { + // /index.php?/ci/woot + $_SERVER['REQUEST_URI'] = '/index.php?/ci/woot'; + $_SERVER['QUERY_STRING'] = '/ci/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $_GET['/ci/woot'] = ''; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'ci/woot'; + $this->assertSame($expected, $factory->detectRoutePath('QUERY_STRING')); + } + + public function testQueryStringWithQueryString() + { + // /index.php?/ci/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php?/ci/woot?code=good'; + $_SERVER['QUERY_STRING'] = '/ci/woot?code=good'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $_GET['/ci/woot?code'] = 'good'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'ci/woot'; + $this->assertSame($expected, $factory->detectRoutePath('QUERY_STRING')); + $this->assertSame('code=good', $_SERVER['QUERY_STRING']); + $this->assertSame(['code' => 'good'], $_GET); + } + + public function testQueryStringEmpty() + { + // /index.php? + $_SERVER['REQUEST_URI'] = '/index.php?'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = '/'; + $this->assertSame($expected, $factory->detectRoutePath('QUERY_STRING')); + } + + public function testPathInfoUnset() + { + // /index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('PATH_INFO')); + } + + public function testPathInfoSubfolder() + { + $appConfig = new App(); + $appConfig->baseURL = 'http://localhost:8888/ci431/public/'; + + // http://localhost:8888/ci431/public/index.php/woot?code=good#pos + $_SERVER['PATH_INFO'] = '/woot'; + $_SERVER['REQUEST_URI'] = '/ci431/public/index.php/woot?code=good'; + $_SERVER['SCRIPT_NAME'] = '/ci431/public/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER, $appConfig); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('PATH_INFO')); + } +} diff --git a/tests/system/HTTP/SiteURIFactoryTest.php b/tests/system/HTTP/SiteURIFactoryTest.php new file mode 100644 index 000000000000..294ae9e9844b --- /dev/null +++ b/tests/system/HTTP/SiteURIFactoryTest.php @@ -0,0 +1,91 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\HTTP; + +use CodeIgniter\Test\CIUnitTestCase; +use Config\App; + +/** + * @backupGlobals enabled + * + * @internal + * + * @group Others + */ +final class SiteURIFactoryTest extends CIUnitTestCase +{ + protected function setUp(): void + { + parent::setUp(); + + $_GET = $_SERVER = []; + } + + public function testCreateFromGlobals() + { + // http://localhost:8080/index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot?code=good'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + $_SERVER['QUERY_STRING'] = 'code=good'; + $_SERVER['HTTP_HOST'] = 'localhost:8080'; + $_SERVER['PATH_INFO'] = '/woot'; + + $_GET['code'] = 'good'; + + $factory = new SiteURIFactory($_SERVER, new App()); + + $uri = $factory->createFromGlobals(); + + $this->assertInstanceOf(SiteURI::class, $uri); + $this->assertSame('http://localhost:8080/index.php/woot?code=good', (string) $uri); + $this->assertSame('/index.php/woot', $uri->getPath()); + $this->assertSame('woot', $uri->getRoutePath()); + } + + public function testCreateFromGlobalsAllowedHost() + { + // http://users.example.jp/index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot?code=good'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + $_SERVER['QUERY_STRING'] = 'code=good'; + $_SERVER['HTTP_HOST'] = 'users.example.jp'; + $_SERVER['PATH_INFO'] = '/woot'; + + $_GET['code'] = 'good'; + + $config = new App(); + $config->baseURL = 'http://example.jp/'; + $config->allowedHostnames = ['users.example.jp']; + + $factory = new SiteURIFactory($_SERVER, $config); + + $uri = $factory->createFromGlobals(); + + $this->assertInstanceOf(SiteURI::class, $uri); + $this->assertSame('http://users.example.jp/index.php/woot?code=good', (string) $uri); + $this->assertSame('/index.php/woot', $uri->getPath()); + $this->assertSame('woot', $uri->getRoutePath()); + } + + public function testCreateFromString() + { + $factory = new SiteURIFactory($_SERVER, new App()); + + $uriString = 'http://invalid.example.jp/foo/bar?page=3'; + $uri = $factory->createFromString($uriString); + + $this->assertInstanceOf(SiteURI::class, $uri); + $this->assertSame('http://localhost:8080/index.php/foo/bar?page=3', (string) $uri); + $this->assertSame('/index.php/foo/bar', $uri->getPath()); + $this->assertSame('foo/bar', $uri->getRoutePath()); + } +} From 09a96abb01315a8f8a0d1c2097f2870c6a893eef Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 23 Feb 2023 16:06:03 +0900 Subject: [PATCH 02/11] fix: createFromString() returns URI with invalid hostname --- system/HTTP/SiteURIFactory.php | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/system/HTTP/SiteURIFactory.php b/system/HTTP/SiteURIFactory.php index e96cc2781f85..0115ef5df9d5 100644 --- a/system/HTTP/SiteURIFactory.php +++ b/system/HTTP/SiteURIFactory.php @@ -71,8 +71,9 @@ public function createFromString(string $uri): SiteURI } $relativePath = $parts['path'] . $query . $fragment; + $host = $this->getValidHost($parts['host']); - return new SiteURI($this->appConfig, $relativePath, $parts['host'], $parts['scheme']); + return new SiteURI($this->appConfig, $relativePath, $host, $parts['scheme']); } /** @@ -231,21 +232,30 @@ private function createURIFromRoutePath(string $routePath): SiteURI } /** - * @return string|null The current hostname. Returns null if no host header. + * @return string|null The current hostname. Returns null if no valid host. */ private function getHost(): ?string { - $host = null; - $httpHostPort = $this->server['HTTP_HOST'] ?? null; + if ($httpHostPort !== null) { [$httpHost] = explode(':', $httpHostPort, 2); - if (in_array($httpHost, $this->appConfig->allowedHostnames, true)) { - $host = $httpHost; - } + return $this->getValidHost($httpHost); + } + + return null; + } + + /** + * @return string|null The valid hostname. Returns null if not valid. + */ + private function getValidHost(string $host): ?string + { + if (in_array($host, $this->appConfig->allowedHostnames, true)) { + return $host; } - return $host; + return null; } } From 37772e910e7ddaab91185b8a8813d2977fd2678c Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 12 Jul 2023 08:20:45 +0900 Subject: [PATCH 03/11] docs: add version to @deprecated --- system/HTTP/IncomingRequest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index 99bbf223f405..bf9e01a8a603 100755 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -236,7 +236,7 @@ protected function detectURI(string $protocol, string $baseURL) * Detects the relative path based on * the URIProtocol Config setting. * - * @deprecated Moved to SiteURIFactory. + * @deprecated 4.4.0 Moved to SiteURIFactory. */ public function detectPath(string $protocol = ''): string { @@ -268,7 +268,7 @@ public function detectPath(string $protocol = ''): string * * @return string The URI it found. * - * @deprecated Moved to SiteURIFactory. + * @deprecated 4.4.0 Moved to SiteURIFactory. */ protected function parseRequestURI(): string { @@ -328,7 +328,7 @@ protected function parseRequestURI(): string * * Will parse QUERY_STRING and automatically detect the URI from it. * - * @deprecated Moved to SiteURIFactory. + * @deprecated 4.4.0 Moved to SiteURIFactory. */ protected function parseQueryString(): string { @@ -502,7 +502,7 @@ public function setPath(string $path, ?App $config = null) } /** - * @deprecated Moved to SiteURIFactory. + * @deprecated 4.4.0 Moved to SiteURIFactory. */ private function determineHost(App $config, string $baseURL): string { From 9937a0308a4a288a43f1c53d36a18407a5ecf9f3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 12 Jul 2023 09:32:19 +0900 Subject: [PATCH 04/11] refactor: extract class Superglobals and use it --- system/HTTP/SiteURIFactory.php | 64 +++++++------------ system/Superglobals.php | 35 ++++++++++ .../SiteURIFactoryDetectRoutePathTest.php | 6 +- tests/system/HTTP/SiteURIFactoryTest.php | 10 ++- 4 files changed, 71 insertions(+), 44 deletions(-) create mode 100644 system/Superglobals.php diff --git a/system/HTTP/SiteURIFactory.php b/system/HTTP/SiteURIFactory.php index 0115ef5df9d5..dbe29546963e 100644 --- a/system/HTTP/SiteURIFactory.php +++ b/system/HTTP/SiteURIFactory.php @@ -12,24 +12,18 @@ namespace CodeIgniter\HTTP; use CodeIgniter\HTTP\Exceptions\HTTPException; +use CodeIgniter\Superglobals; use Config\App; class SiteURIFactory { - /** - * @var array Superglobal SERVER array - */ - private array $server; - + private Superglobals $superglobals; private App $appConfig; - /** - * @param array $server Superglobal $_SERVER array - */ - public function __construct(array $server, App $appConfig) + public function __construct(Superglobals $superglobals, App $appConfig) { - $this->server = $server; - $this->appConfig = $appConfig; + $this->superglobals = $superglobals; + $this->appConfig = $appConfig; } /** @@ -103,7 +97,7 @@ public function detectRoutePath(string $protocol = ''): string case 'PATH_INFO': default: - $routePath = $this->server[$protocol] ?? $this->parseRequestURI(); + $routePath = $this->superglobals->server($protocol) ?? $this->parseRequestURI(); break; } @@ -120,7 +114,10 @@ public function detectRoutePath(string $protocol = ''): string */ private function parseRequestURI(): string { - if (! isset($this->server['REQUEST_URI'], $this->server['SCRIPT_NAME'])) { + if ( + $this->superglobals->server('REQUEST_URI') === null + || $this->superglobals->server('SCRIPT_NAME') === null + ) { return ''; } @@ -128,19 +125,19 @@ private function parseRequestURI(): string // string contains a colon followed by a number. So we attach a dummy // host since REQUEST_URI does not include the host. This allows us to // parse out the query string and path. - $parts = parse_url('http://dummy' . $this->server['REQUEST_URI']); + $parts = parse_url('http://dummy' . $this->superglobals->server('REQUEST_URI')); $query = $parts['query'] ?? ''; $path = $parts['path'] ?? ''; // Strip the SCRIPT_NAME path from the URI if ( - $path !== '' && isset($this->server['SCRIPT_NAME'][0]) - && pathinfo($this->server['SCRIPT_NAME'], PATHINFO_EXTENSION) === 'php' + $path !== '' && isset($this->superglobals->server('SCRIPT_NAME')[0]) + && pathinfo($this->superglobals->server('SCRIPT_NAME'), PATHINFO_EXTENSION) === 'php' ) { // Compare each segment, dropping them until there is no match $segments = $keep = explode('/', $path); - foreach (explode('/', $this->server['SCRIPT_NAME']) as $i => $segment) { + foreach (explode('/', $this->superglobals->server('SCRIPT_NAME')) as $i => $segment) { // If these segments are not the same then we're done if (! isset($segments[$i]) || $segment !== $segments[$i]) { break; @@ -160,30 +157,18 @@ private function parseRequestURI(): string $path = $parts[0]; $newQuery = $query[1] ?? ''; - $this->server['QUERY_STRING'] = $newQuery; - $this->updateServer('QUERY_STRING', $newQuery); + $this->superglobals->setServer('QUERY_STRING', $newQuery); } else { - $this->server['QUERY_STRING'] = $query; - $this->updateServer('QUERY_STRING', $query); + $this->superglobals->setServer('QUERY_STRING', $query); } // Update our global GET for values likely to have been changed - parse_str($this->server['QUERY_STRING'], $get); - $this->updateGetArray($get); + parse_str($this->superglobals->server('QUERY_STRING'), $get); + $this->superglobals->setGetArray($get); return URI::removeDotSegments($path); } - private function updateServer(string $key, string $value): void - { - $_SERVER[$key] = $value; - } - - private function updateGetArray(array $array): void - { - $_GET = $array; - } - /** * Will parse QUERY_STRING and automatically detect the URI from it. * @@ -193,7 +178,7 @@ private function updateGetArray(array $array): void */ private function parseQueryString(): string { - $query = $this->server['QUERY_STRING'] ?? @getenv('QUERY_STRING'); + $query = $this->superglobals->server('QUERY_STRING') ?? @getenv('QUERY_STRING'); if (trim($query, '/') === '') { return '/'; @@ -204,15 +189,14 @@ private function parseQueryString(): string $path = $parts[0]; $newQuery = $parts[1] ?? ''; - $this->server['QUERY_STRING'] = $newQuery; - $this->updateServer('QUERY_STRING', $newQuery); + $this->superglobals->setServer('QUERY_STRING', $newQuery); } else { $path = $query; } // Update our global GET for values likely to have been changed - parse_str($this->server['QUERY_STRING'], $get); - $this->updateGetArray($get); + parse_str($this->superglobals->server('QUERY_STRING'), $get); + $this->superglobals->setGetArray($get); return URI::removeDotSegments($path); } @@ -224,7 +208,7 @@ private function parseQueryString(): string */ private function createURIFromRoutePath(string $routePath): SiteURI { - $query = $this->server['QUERY_STRING'] ?? ''; + $query = $this->superglobals->server('QUERY_STRING') ?? ''; $relativePath = $query !== '' ? $routePath . '?' . $query : $routePath; @@ -236,7 +220,7 @@ private function createURIFromRoutePath(string $routePath): SiteURI */ private function getHost(): ?string { - $httpHostPort = $this->server['HTTP_HOST'] ?? null; + $httpHostPort = $this->superglobals->server('HTTP_HOST') ?? null; if ($httpHostPort !== null) { [$httpHost] = explode(':', $httpHostPort, 2); diff --git a/system/Superglobals.php b/system/Superglobals.php new file mode 100644 index 000000000000..65ab800e98f3 --- /dev/null +++ b/system/Superglobals.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter; + +/** + * Superglobals manipulation. + * + * @internal + */ +final class Superglobals +{ + public function server(string $key): ?string + { + return $_SERVER[$key] ?? null; + } + + public function setServer(string $key, string $value): void + { + $_SERVER[$key] = $value; + } + + public function setGetArray(array $array): void + { + $_GET = $array; + } +} diff --git a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php index 3b7715049a5c..12d111f33390 100644 --- a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php +++ b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php @@ -11,6 +11,7 @@ namespace CodeIgniter\HTTP; +use CodeIgniter\Superglobals; use CodeIgniter\Test\CIUnitTestCase; use Config\App; @@ -34,7 +35,10 @@ private function createSiteURIFactory(array $server, ?App $appConfig = null): Si { $appConfig ??= new App(); - return new SiteURIFactory($server, $appConfig); + $_SERVER = $server; + $superglobals = new Superglobals(); + + return new SiteURIFactory($superglobals, $appConfig); } public function testDefault() diff --git a/tests/system/HTTP/SiteURIFactoryTest.php b/tests/system/HTTP/SiteURIFactoryTest.php index 294ae9e9844b..12ee43757579 100644 --- a/tests/system/HTTP/SiteURIFactoryTest.php +++ b/tests/system/HTTP/SiteURIFactoryTest.php @@ -11,6 +11,7 @@ namespace CodeIgniter\HTTP; +use CodeIgniter\Superglobals; use CodeIgniter\Test\CIUnitTestCase; use Config\App; @@ -41,7 +42,8 @@ public function testCreateFromGlobals() $_GET['code'] = 'good'; - $factory = new SiteURIFactory($_SERVER, new App()); + $superglobals = new Superglobals(); + $factory = new SiteURIFactory($superglobals, new App()); $uri = $factory->createFromGlobals(); @@ -66,7 +68,8 @@ public function testCreateFromGlobalsAllowedHost() $config->baseURL = 'http://example.jp/'; $config->allowedHostnames = ['users.example.jp']; - $factory = new SiteURIFactory($_SERVER, $config); + $superglobals = new Superglobals(); + $factory = new SiteURIFactory($superglobals, $config); $uri = $factory->createFromGlobals(); @@ -78,7 +81,8 @@ public function testCreateFromGlobalsAllowedHost() public function testCreateFromString() { - $factory = new SiteURIFactory($_SERVER, new App()); + $superglobals = new Superglobals(); + $factory = new SiteURIFactory($superglobals, new App()); $uriString = 'http://invalid.example.jp/foo/bar?page=3'; $uri = $factory->createFromString($uriString); From 8522e992208ca795f16254156cbed21a55ea82b1 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 12 Jul 2023 09:43:19 +0900 Subject: [PATCH 05/11] test: extract createSiteURIFactory() --- tests/system/HTTP/SiteURIFactoryTest.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/system/HTTP/SiteURIFactoryTest.php b/tests/system/HTTP/SiteURIFactoryTest.php index 12ee43757579..e211e7167db5 100644 --- a/tests/system/HTTP/SiteURIFactoryTest.php +++ b/tests/system/HTTP/SiteURIFactoryTest.php @@ -31,6 +31,14 @@ protected function setUp(): void $_GET = $_SERVER = []; } + private function createSiteURIFactory(?App $config = null, ?Superglobals $superglobals = null): SiteURIFactory + { + $config ??= new App(); + $superglobals ??= new Superglobals(); + + return new SiteURIFactory($superglobals, $config); + } + public function testCreateFromGlobals() { // http://localhost:8080/index.php/woot?code=good#pos @@ -42,8 +50,7 @@ public function testCreateFromGlobals() $_GET['code'] = 'good'; - $superglobals = new Superglobals(); - $factory = new SiteURIFactory($superglobals, new App()); + $factory = $this->createSiteURIFactory(); $uri = $factory->createFromGlobals(); @@ -68,8 +75,7 @@ public function testCreateFromGlobalsAllowedHost() $config->baseURL = 'http://example.jp/'; $config->allowedHostnames = ['users.example.jp']; - $superglobals = new Superglobals(); - $factory = new SiteURIFactory($superglobals, $config); + $factory = $this->createSiteURIFactory($config); $uri = $factory->createFromGlobals(); @@ -81,8 +87,7 @@ public function testCreateFromGlobalsAllowedHost() public function testCreateFromString() { - $superglobals = new Superglobals(); - $factory = new SiteURIFactory($superglobals, new App()); + $factory = $this->createSiteURIFactory(); $uriString = 'http://invalid.example.jp/foo/bar?page=3'; $uri = $factory->createFromString($uriString); From be7ca15611603c7ff7f9558e0f5fae03061c8ab5 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 12 Jul 2023 09:44:44 +0900 Subject: [PATCH 06/11] refactor: change parameter order --- system/HTTP/SiteURIFactory.php | 6 +++--- tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php | 2 +- tests/system/HTTP/SiteURIFactoryTest.php | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/system/HTTP/SiteURIFactory.php b/system/HTTP/SiteURIFactory.php index dbe29546963e..c8996038116f 100644 --- a/system/HTTP/SiteURIFactory.php +++ b/system/HTTP/SiteURIFactory.php @@ -17,13 +17,13 @@ class SiteURIFactory { - private Superglobals $superglobals; private App $appConfig; + private Superglobals $superglobals; - public function __construct(Superglobals $superglobals, App $appConfig) + public function __construct(App $appConfig, Superglobals $superglobals) { - $this->superglobals = $superglobals; $this->appConfig = $appConfig; + $this->superglobals = $superglobals; } /** diff --git a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php index 12d111f33390..b207d1134dc4 100644 --- a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php +++ b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php @@ -38,7 +38,7 @@ private function createSiteURIFactory(array $server, ?App $appConfig = null): Si $_SERVER = $server; $superglobals = new Superglobals(); - return new SiteURIFactory($superglobals, $appConfig); + return new SiteURIFactory($appConfig, $superglobals); } public function testDefault() diff --git a/tests/system/HTTP/SiteURIFactoryTest.php b/tests/system/HTTP/SiteURIFactoryTest.php index e211e7167db5..138ccad0b48d 100644 --- a/tests/system/HTTP/SiteURIFactoryTest.php +++ b/tests/system/HTTP/SiteURIFactoryTest.php @@ -36,7 +36,7 @@ private function createSiteURIFactory(?App $config = null, ?Superglobals $superg $config ??= new App(); $superglobals ??= new Superglobals(); - return new SiteURIFactory($superglobals, $config); + return new SiteURIFactory($config, $superglobals); } public function testCreateFromGlobals() From 1a95c89421e9ea25282e201c7f1c3c39f1eb84f2 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 12 Jul 2023 09:50:40 +0900 Subject: [PATCH 07/11] feat: allow Superglobals state changes from the constructor --- system/Superglobals.php | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/system/Superglobals.php b/system/Superglobals.php index 65ab800e98f3..4ffa51033bac 100644 --- a/system/Superglobals.php +++ b/system/Superglobals.php @@ -18,18 +18,37 @@ */ final class Superglobals { + private array $server; + private array $get; + + public function __construct(?array $server = null, ?array $get = null) + { + $this->server = $server ?? $_SERVER; + $this->get = $get ?? $_GET; + } + public function server(string $key): ?string { - return $_SERVER[$key] ?? null; + return $this->server[$key] ?? null; } public function setServer(string $key, string $value): void { - $_SERVER[$key] = $value; + $this->server[$key] = $value; + $_SERVER[$key] = $value; + } + + /** + * @return array|string|null + */ + public function get(string $key) + { + return $this->get[$key] ?? null; } public function setGetArray(array $array): void { - $_GET = $array; + $this->get = $array; + $_GET = $array; } } From 78b0ac6ef9d1fed2a48d16d74b1389a550146d7e Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 24 Jul 2023 09:32:33 +0900 Subject: [PATCH 08/11] refactore: remove `@` that does not make sense, and cast to string Because it might be false. --- system/HTTP/SiteURIFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/HTTP/SiteURIFactory.php b/system/HTTP/SiteURIFactory.php index c8996038116f..49ee3bf3ed73 100644 --- a/system/HTTP/SiteURIFactory.php +++ b/system/HTTP/SiteURIFactory.php @@ -178,7 +178,7 @@ private function parseRequestURI(): string */ private function parseQueryString(): string { - $query = $this->superglobals->server('QUERY_STRING') ?? @getenv('QUERY_STRING'); + $query = $this->superglobals->server('QUERY_STRING') ?? (string) getenv('QUERY_STRING'); if (trim($query, '/') === '') { return '/'; From fde3216efdb84392aeb7c53df1f2df2c0a6bd74f Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 24 Jul 2023 09:37:34 +0900 Subject: [PATCH 09/11] refactor: remove string array access It is difficult to understand. --- system/HTTP/SiteURIFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/HTTP/SiteURIFactory.php b/system/HTTP/SiteURIFactory.php index 49ee3bf3ed73..fcf295c82b02 100644 --- a/system/HTTP/SiteURIFactory.php +++ b/system/HTTP/SiteURIFactory.php @@ -131,7 +131,7 @@ private function parseRequestURI(): string // Strip the SCRIPT_NAME path from the URI if ( - $path !== '' && isset($this->superglobals->server('SCRIPT_NAME')[0]) + $path !== '' && $this->superglobals->server('SCRIPT_NAME') !== '' && pathinfo($this->superglobals->server('SCRIPT_NAME'), PATHINFO_EXTENSION) === 'php' ) { // Compare each segment, dropping them until there is no match From 6b3dd7af0da55785d6342883634738db8e0779fe Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 24 Jul 2023 09:40:55 +0900 Subject: [PATCH 10/11] refactor: make SiteURIFactory final --- system/HTTP/SiteURIFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/HTTP/SiteURIFactory.php b/system/HTTP/SiteURIFactory.php index fcf295c82b02..f0b846a44457 100644 --- a/system/HTTP/SiteURIFactory.php +++ b/system/HTTP/SiteURIFactory.php @@ -15,7 +15,7 @@ use CodeIgniter\Superglobals; use Config\App; -class SiteURIFactory +final class SiteURIFactory { private App $appConfig; private Superglobals $superglobals; From 84b4a31535cd767225a0cdcdeb40c7170fefc1c5 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 24 Jul 2023 09:50:00 +0900 Subject: [PATCH 11/11] feat: add Superglobals::setGet() --- system/Superglobals.php | 6 ++++++ tests/system/SuperglobalsTest.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 tests/system/SuperglobalsTest.php diff --git a/system/Superglobals.php b/system/Superglobals.php index 4ffa51033bac..b126af130f34 100644 --- a/system/Superglobals.php +++ b/system/Superglobals.php @@ -46,6 +46,12 @@ public function get(string $key) return $this->get[$key] ?? null; } + public function setGet(string $key, string $value): void + { + $this->get[$key] = $value; + $_GET[$key] = $value; + } + public function setGetArray(array $array): void { $this->get = $array; diff --git a/tests/system/SuperglobalsTest.php b/tests/system/SuperglobalsTest.php new file mode 100644 index 000000000000..f530145da766 --- /dev/null +++ b/tests/system/SuperglobalsTest.php @@ -0,0 +1,31 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter; + +use CodeIgniter\Test\CIUnitTestCase; + +/** + * @internal + * + * @group Others + */ +final class SuperglobalsTest extends CIUnitTestCase +{ + public function testSetGet() + { + $globals = new Superglobals([], []); + + $globals->setGet('test', 'value1'); + + $this->assertSame('value1', $globals->get('test')); + } +}