From db3b2f72ace49c29b015b3cd2656556bb2134e22 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 9 Feb 2023 08:21:22 +0900 Subject: [PATCH 01/17] fix: deprecate $request and $response in Exceptions::__construct() When registering Exception Handler, Request object was generated. But it was too early, and difficult to understand. --- system/Config/Services.php | 6 +++++- system/Debug/Exceptions.php | 12 ++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/system/Config/Services.php b/system/Config/Services.php index a2d13c40e80c..2358e87bb1a5 100644 --- a/system/Config/Services.php +++ b/system/Config/Services.php @@ -250,6 +250,8 @@ public static function encrypter(?EncryptionConfig $config = null, $getShared = * - register_shutdown_function * * @return Exceptions + * + * @deprecated The parameter $request and $response are deprecated. */ public static function exceptions( ?ExceptionsConfig $config = null, @@ -262,7 +264,9 @@ public static function exceptions( } $config ??= config('Exceptions'); - $request ??= AppServices::request(); + /** @var ExceptionsConfig $config */ + + // @TODO remove instantiation of Response in the future. $response ??= AppServices::response(); return new Exceptions($config, $request, $response); diff --git a/system/Debug/Exceptions.php b/system/Debug/Exceptions.php index 6dc098d4af10..d4e80f28f2e9 100644 --- a/system/Debug/Exceptions.php +++ b/system/Debug/Exceptions.php @@ -21,6 +21,7 @@ use CodeIgniter\HTTP\ResponseInterface; use Config\Exceptions as ExceptionsConfig; use Config\Paths; +use Config\Services; use ErrorException; use Psr\Log\LogLevel; use Throwable; @@ -71,15 +72,15 @@ class Exceptions private ?Throwable $exceptionCaughtByExceptionHandler = null; /** - * @param CLIRequest|IncomingRequest $request + * @param CLIRequest|IncomingRequest|null $request + * + * @deprecated The parameter $request and $response are deprecated. No longer used. */ - public function __construct(ExceptionsConfig $config, $request, ResponseInterface $response) + public function __construct(ExceptionsConfig $config, $request, ResponseInterface $response) /** @phpstan-ignore-line */ { $this->ob_level = ob_get_level(); $this->viewPath = rtrim($config->errorViewPath, '\\/ ') . DIRECTORY_SEPARATOR; $this->config = $config; - $this->request = $request; - $this->response = $response; // workaround for upgraded users // This causes "Deprecated: Creation of dynamic property" in PHP 8.2. @@ -119,6 +120,9 @@ public function exceptionHandler(Throwable $exception) [$statusCode, $exitCode] = $this->determineCodes($exception); + $this->request = Services::request(); + $this->response = Services::response(); + if ($this->config->log === true && ! in_array($statusCode, $this->config->ignoreCodes, true)) { log_message('critical', "{message}\nin {exFile} on line {exLine}.\n{trace}", [ 'message' => $exception->getMessage(), From 1fbcd4e4bf7a5bb51005b8881492047c7a3c1a44 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 9 Feb 2023 08:46:23 +0900 Subject: [PATCH 02/17] chore: add rule to skip --- rector.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rector.php b/rector.php index 797387ba4aba..0473a5d8cfdb 100644 --- a/rector.php +++ b/rector.php @@ -28,6 +28,7 @@ use Rector\CodingStyle\Rector\ClassMethod\MakeInheritedMethodVisibilitySameAsParentRector; use Rector\CodingStyle\Rector\FuncCall\CountArrayToEmptyArrayComparisonRector; use Rector\Config\RectorConfig; +use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedConstructorParamRector; use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector; use Rector\DeadCode\Rector\If_\UnwrapFutureCompatibleIfPhpVersionRector; use Rector\DeadCode\Rector\MethodCall\RemoveEmptyMethodCallRector; @@ -89,6 +90,11 @@ __DIR__ . '/tests/system/Test/ReflectionHelperTest.php', ], + RemoveUnusedConstructorParamRector::class => [ + // there are deprecated parameters + __DIR__ . '/system/Debug/Exceptions.php', + ], + // call on purpose for nothing happen check RemoveEmptyMethodCallRector::class => [ __DIR__ . '/tests', From 9fa70300dbb08c180f6f633770cb4fd1e39298eb Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 9 Feb 2023 09:23:23 +0900 Subject: [PATCH 03/17] test: fix incorrect setup baseURL should not be empty. --- tests/system/Helpers/FormHelperTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/system/Helpers/FormHelperTest.php b/tests/system/Helpers/FormHelperTest.php index 9a525adf7ad2..b45329c8663c 100644 --- a/tests/system/Helpers/FormHelperTest.php +++ b/tests/system/Helpers/FormHelperTest.php @@ -39,7 +39,6 @@ private function setRequest(): void Services::injectMock('uri', $uri); $config = new App(); - $config->baseURL = ''; $config->indexPage = 'index.php'; $request = Services::request($config); From e983ff616034218cab57124239448df0329db060 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 12:00:25 +0900 Subject: [PATCH 04/17] feat: add URI::setRoutePath() method --- system/HTTP/URI.php | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/system/HTTP/URI.php b/system/HTTP/URI.php index 587e441ffc17..ece593be3af5 100644 --- a/system/HTTP/URI.php +++ b/system/HTTP/URI.php @@ -97,6 +97,16 @@ class URI */ protected $path; + /** + * URI path relative to baseURL. + * + * If the baseURL contains sub folders, this value will be different from + * the current URI path. + * + * @var string + */ + protected $routePath; + /** * The name of any fragment. * @@ -757,6 +767,22 @@ public function setPath(string $path) return $this; } + /** + * Sets the route path. + * + * @return $this + */ + public function setRoutePath(string $path) + { + $this->routePath = $this->filterPath($path); + + $tempPath = trim($this->routePath, '/'); + + $this->segments = ($tempPath === '') ? [] : explode('/', $tempPath); + + return $this; + } + /** * Sets the current baseURL. * From e75844737e798a04a015950b0bb22bdbc0502e6b Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 12:01:00 +0900 Subject: [PATCH 05/17] docs: add description --- system/HTTP/URI.php | 1 + 1 file changed, 1 insertion(+) diff --git a/system/HTTP/URI.php b/system/HTTP/URI.php index ece593be3af5..6b01c79b6126 100644 --- a/system/HTTP/URI.php +++ b/system/HTTP/URI.php @@ -43,6 +43,7 @@ class URI /** * List of URI segments. + * URI Segments mean only the URI path part relative to the baseURL. * * Starts at 1 instead of 0 * From 40dc59a2921add4eb08fcfb64e4ca20304b825be Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 11:38:34 +0900 Subject: [PATCH 06/17] feat: add URIFactory and copy methods from IncomingRequest This class does not work yet. --- system/HTTP/URIFactory.php | 248 +++++++++++++++++++++++++++++++++++++ 1 file changed, 248 insertions(+) create mode 100644 system/HTTP/URIFactory.php diff --git a/system/HTTP/URIFactory.php b/system/HTTP/URIFactory.php new file mode 100644 index 000000000000..38f718ef6598 --- /dev/null +++ b/system/HTTP/URIFactory.php @@ -0,0 +1,248 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\HTTP; + +use Config\App; + +class URIFactory +{ + /** + * @var array Superglobal SERVER array + */ + private array $server; + + /** + * @var array Superglobal GET array + */ + private array $get; + + private App $appConfig; + + /** + * @param array $server Superglobal $_SERVER array + * @param array $get Superglobal $_GET array + */ + public function __construct(array &$server, array &$get, App $appConfig) + { + $this->server = &$server; + $this->get = &$get; + $this->appConfig = $appConfig; + } + + /** + * Create the current URI object. + * + * This method updates superglobal $_SERVER and $_GET. + */ + public function createCurrentURI(): URI + { + $routePath = $this->detectRoutePath(); + + return $this->createURIFromRoutePath($routePath); + } + + /** + * Detects the relative path based on + * the URIProtocol Config setting. + */ + public function detectPath(string $protocol = ''): string + { + if (empty($protocol)) { + $protocol = 'REQUEST_URI'; + } + + switch ($protocol) { + case 'REQUEST_URI': + $this->path = $this->parseRequestURI(); + break; + + case 'QUERY_STRING': + $this->path = $this->parseQueryString(); + break; + + case 'PATH_INFO': + default: + $this->path = $this->fetchGlobal('server', $protocol) ?? $this->parseRequestURI(); + break; + } + + return $this->path; + } + + /** + * Will parse the REQUEST_URI and automatically detect the URI from it, + * fixing the query string if necessary. + * + * @return string The URI it found. + */ + protected function parseRequestURI(): string + { + if (! isset($_SERVER['REQUEST_URI'], $_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' . $_SERVER['REQUEST_URI']); + $query = $parts['query'] ?? ''; + $uri = $parts['path'] ?? ''; + + // Strip the SCRIPT_NAME path from the URI + if ( + $uri !== '' && isset($_SERVER['SCRIPT_NAME'][0]) + && pathinfo($_SERVER['SCRIPT_NAME'], PATHINFO_EXTENSION) === 'php' + ) { + // Compare each segment, dropping them until there is no match + $segments = $keep = explode('/', $uri); + + foreach (explode('/', $_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); + } + + $uri = 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($uri, '/') === '' && strncmp($query, '/', 1) === 0) { + $query = explode('?', $query, 2); + $uri = $query[0]; + $_SERVER['QUERY_STRING'] = $query[1] ?? ''; + } else { + $_SERVER['QUERY_STRING'] = $query; + } + + // Update our globals for values likely to been have changed + parse_str($_SERVER['QUERY_STRING'], $_GET); + $this->populateGlobals('server'); + $this->populateGlobals('get'); + + $uri = URI::removeDotSegments($uri); + + return ($uri === '/' || $uri === '') ? '/' : ltrim($uri, '/'); + } + + /** + * Parse QUERY_STRING + * + * Will parse QUERY_STRING and automatically detect the URI from it. + */ + protected function parseQueryString(): string + { + $uri = $_SERVER['QUERY_STRING'] ?? @getenv('QUERY_STRING'); + + if (trim($uri, '/') === '') { + return '/'; + } + + if (strncmp($uri, '/', 1) === 0) { + $uri = explode('?', $uri, 2); + $_SERVER['QUERY_STRING'] = $uri[1] ?? ''; + $uri = $uri[0]; + } + + // Update our globals for values likely to been have changed + parse_str($_SERVER['QUERY_STRING'], $_GET); + $this->populateGlobals('server'); + $this->populateGlobals('get'); + + $uri = URI::removeDotSegments($uri); + + return ($uri === '/' || $uri === '') ? '/' : ltrim($uri, '/'); + } + + /** + * Sets the relative path and updates the URI object. + * + * Note: Since current_url() accesses the shared request + * instance, this can be used to change the "current URL" + * for testing. + * + * @param string $path URI path relative to baseURL + * @param App|null $config Optional alternate config to use + * + * @return $this + */ + public function setPath(string $path, ?App $config = null) + { + $this->path = $path; + + // @TODO remove this. The path of the URI object should be a full URI path, + // not a URI path relative to baseURL. + $this->uri->setPath($path); + + $config ??= $this->config; + + // It's possible the user forgot a trailing slash on their + // baseURL, so let's help them out. + $baseURL = ($config->baseURL === '') ? $config->baseURL : rtrim($config->baseURL, '/ ') . '/'; + + // Based on our baseURL and allowedHostnames provided by the developer + // and HTTP_HOST, set our current domain name, scheme. + if ($baseURL !== '') { + $host = $this->determineHost($config, $baseURL); + + // Set URI::$baseURL + $uri = new URI($baseURL); + $currentBaseURL = (string) $uri->setHost($host); + $this->uri->setBaseURL($currentBaseURL); + + $this->uri->setScheme(parse_url($baseURL, PHP_URL_SCHEME)); + $this->uri->setHost($host); + $this->uri->setPort(parse_url($baseURL, PHP_URL_PORT)); + + // Ensure we have any query vars + $this->uri->setQuery($_SERVER['QUERY_STRING'] ?? ''); + + // Check if the scheme needs to be coerced into its secure version + if ($config->forceGlobalSecureRequests && $this->uri->getScheme() === 'http') { + $this->uri->setScheme('https'); + } + } elseif (! is_cli()) { + // Do not change exit() to exception; Request is initialized before + // setting the exception handler, so if an exception is raised, an + // error will be displayed even if in the production environment. + // @codeCoverageIgnoreStart + exit('You have an empty or invalid baseURL. The baseURL value must be set in app/Config/App.php, or through the .env file.'); + // @codeCoverageIgnoreEnd + } + + return $this; + } + + private function determineHost(App $config, string $baseURL): string + { + $host = parse_url($baseURL, PHP_URL_HOST); + + if (empty($config->allowedHostnames)) { + return $host; + } + + // Update host if it is valid. + $httpHostPort = $this->getServer('HTTP_HOST'); + if ($httpHostPort !== null) { + [$httpHost] = explode(':', $httpHostPort, 2); + + if (in_array($httpHost, $config->allowedHostnames, true)) { + $host = $httpHost; + } + } + + return $host; + } +} From 84193901daf4016d869502bc88f39192ade40dd6 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 11:46:33 +0900 Subject: [PATCH 07/17] feat: make URIFactory work --- system/HTTP/URIFactory.php | 149 +++++++++++++++++++------------------ 1 file changed, 77 insertions(+), 72 deletions(-) diff --git a/system/HTTP/URIFactory.php b/system/HTTP/URIFactory.php index 38f718ef6598..a4bafdfdeb0a 100644 --- a/system/HTTP/URIFactory.php +++ b/system/HTTP/URIFactory.php @@ -51,61 +51,68 @@ public function createCurrentURI(): URI } /** - * Detects the relative path based on - * the URIProtocol Config setting. + * Detects the current URI path relative to baseURL based on the URIProtocol + * Config setting. + * + * @param string $protocol URIProtocol + * + * @return string The route path */ - public function detectPath(string $protocol = ''): string + public function detectRoutePath(string $protocol = ''): string { - if (empty($protocol)) { - $protocol = 'REQUEST_URI'; + if ($protocol === '') { + $protocol = $this->appConfig->uriProtocol; } switch ($protocol) { case 'REQUEST_URI': - $this->path = $this->parseRequestURI(); + $routePath = $this->parseRequestURI(); break; case 'QUERY_STRING': - $this->path = $this->parseQueryString(); + $routePath = $this->parseQueryString(); break; case 'PATH_INFO': default: - $this->path = $this->fetchGlobal('server', $protocol) ?? $this->parseRequestURI(); + $routePath = $this->server[$protocol] ?? $this->parseRequestURI(); break; } - return $this->path; + return $routePath; } /** * Will parse the REQUEST_URI and automatically detect the URI from it, * fixing the query string if necessary. * - * @return string The URI it found. + * This method updates superglobal $_SERVER and $_GET. + * + * @return string The route path. */ - protected function parseRequestURI(): string + private function parseRequestURI(): string { - if (! isset($_SERVER['REQUEST_URI'], $_SERVER['SCRIPT_NAME'])) { + 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' . $_SERVER['REQUEST_URI']); + // 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'] ?? ''; $uri = $parts['path'] ?? ''; // Strip the SCRIPT_NAME path from the URI if ( - $uri !== '' && isset($_SERVER['SCRIPT_NAME'][0]) - && pathinfo($_SERVER['SCRIPT_NAME'], PATHINFO_EXTENSION) === 'php' + $uri !== '' && 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('/', $uri); - foreach (explode('/', $_SERVER['SCRIPT_NAME']) as $i => $segment) { + 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; @@ -117,20 +124,19 @@ protected function parseRequestURI(): string $uri = 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. + // 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($uri, '/') === '' && strncmp($query, '/', 1) === 0) { - $query = explode('?', $query, 2); - $uri = $query[0]; - $_SERVER['QUERY_STRING'] = $query[1] ?? ''; + $query = explode('?', $query, 2); + $uri = $query[0]; + $this->server['QUERY_STRING'] = $query[1] ?? ''; } else { - $_SERVER['QUERY_STRING'] = $query; + $this->server['QUERY_STRING'] = $query; } - // Update our globals for values likely to been have changed - parse_str($_SERVER['QUERY_STRING'], $_GET); - $this->populateGlobals('server'); - $this->populateGlobals('get'); + // Update our globals for values likely to have been changed + parse_str($this->server['QUERY_STRING'], $this->get); $uri = URI::removeDotSegments($uri); @@ -138,28 +144,28 @@ protected function parseRequestURI(): string } /** - * Parse QUERY_STRING - * * Will parse QUERY_STRING and automatically detect the URI from it. + * + * This method updates superglobal $_SERVER and $_GET. + * + * @return string The route path. */ - protected function parseQueryString(): string + private function parseQueryString(): string { - $uri = $_SERVER['QUERY_STRING'] ?? @getenv('QUERY_STRING'); + $uri = $this->server['QUERY_STRING'] ?? @getenv('QUERY_STRING'); if (trim($uri, '/') === '') { return '/'; } if (strncmp($uri, '/', 1) === 0) { - $uri = explode('?', $uri, 2); - $_SERVER['QUERY_STRING'] = $uri[1] ?? ''; - $uri = $uri[0]; + $uri = explode('?', $uri, 2); + $this->server['QUERY_STRING'] = $uri[1] ?? ''; + $uri = $uri[0]; } - // Update our globals for values likely to been have changed - parse_str($_SERVER['QUERY_STRING'], $_GET); - $this->populateGlobals('server'); - $this->populateGlobals('get'); + // Update our globals for values likely to have been changed + parse_str($this->server['QUERY_STRING'], $this->get); $uri = URI::removeDotSegments($uri); @@ -167,53 +173,49 @@ protected function parseQueryString(): string } /** - * Sets the relative path and updates the URI object. - * - * Note: Since current_url() accesses the shared request - * instance, this can be used to change the "current URL" - * for testing. + * Create current URI object. * - * @param string $path URI path relative to baseURL - * @param App|null $config Optional alternate config to use - * - * @return $this + * @param string $routePath URI path relative to baseURL */ - public function setPath(string $path, ?App $config = null) + private function createURIFromRoutePath(string $routePath): URI { - $this->path = $path; - - // @TODO remove this. The path of the URI object should be a full URI path, - // not a URI path relative to baseURL. - $this->uri->setPath($path); - - $config ??= $this->config; + $config = $this->appConfig; // It's possible the user forgot a trailing slash on their // baseURL, so let's help them out. - $baseURL = ($config->baseURL === '') ? $config->baseURL : rtrim($config->baseURL, '/ ') . '/'; + $baseURL = ($config->baseURL === '') + ? $config->baseURL + : rtrim($config->baseURL, '/ ') . '/'; // Based on our baseURL and allowedHostnames provided by the developer // and HTTP_HOST, set our current domain name, scheme. if ($baseURL !== '') { - $host = $this->determineHost($config, $baseURL); + $host = $this->determineHost($baseURL); // Set URI::$baseURL $uri = new URI($baseURL); $currentBaseURL = (string) $uri->setHost($host); - $this->uri->setBaseURL($currentBaseURL); + $uri->setBaseURL($currentBaseURL); + + $uri->setPath($routePath); + + $uri->setRoutePath($routePath); - $this->uri->setScheme(parse_url($baseURL, PHP_URL_SCHEME)); - $this->uri->setHost($host); - $this->uri->setPort(parse_url($baseURL, PHP_URL_PORT)); + $uri->setScheme(parse_url($baseURL, PHP_URL_SCHEME)); + $uri->setHost($host); + $uri->setPort(parse_url($baseURL, PHP_URL_PORT)); // Ensure we have any query vars - $this->uri->setQuery($_SERVER['QUERY_STRING'] ?? ''); + $uri->setQuery($this->server['QUERY_STRING'] ?? ''); // Check if the scheme needs to be coerced into its secure version - if ($config->forceGlobalSecureRequests && $this->uri->getScheme() === 'http') { - $this->uri->setScheme('https'); + if ($config->forceGlobalSecureRequests && $uri->getScheme() === 'http') { + $uri->setScheme('https'); } - } elseif (! is_cli()) { + + return $uri; + } + if (! is_cli()) { // Do not change exit() to exception; Request is initialized before // setting the exception handler, so if an exception is raised, an // error will be displayed even if in the production environment. @@ -222,23 +224,26 @@ public function setPath(string $path, ?App $config = null) // @codeCoverageIgnoreEnd } - return $this; + return new URI(); } - private function determineHost(App $config, string $baseURL): string + /** + * @return string The current hostname. + */ + private function determineHost(string $baseURL): string { $host = parse_url($baseURL, PHP_URL_HOST); - if (empty($config->allowedHostnames)) { + if (empty($this->appConfig->allowedHostnames)) { return $host; } // Update host if it is valid. - $httpHostPort = $this->getServer('HTTP_HOST'); + $httpHostPort = $this->server['HTTP_HOST'] ?? null; if ($httpHostPort !== null) { [$httpHost] = explode(':', $httpHostPort, 2); - if (in_array($httpHost, $config->allowedHostnames, true)) { + if (in_array($httpHost, $this->appConfig->allowedHostnames, true)) { $host = $httpHost; } } From 7445ae6618dec72a3e91dfac6d3fc10dcff63210 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 11:54:38 +0900 Subject: [PATCH 08/17] fix: normalize route path when using PATH_INFO --- system/HTTP/URIFactory.php | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/system/HTTP/URIFactory.php b/system/HTTP/URIFactory.php index a4bafdfdeb0a..67798d3b298a 100644 --- a/system/HTTP/URIFactory.php +++ b/system/HTTP/URIFactory.php @@ -79,7 +79,7 @@ public function detectRoutePath(string $protocol = ''): string break; } - return $routePath; + return ($routePath === '/' || $routePath === '') ? '/' : ltrim($routePath, '/'); } /** @@ -88,7 +88,7 @@ public function detectRoutePath(string $protocol = ''): string * * This method updates superglobal $_SERVER and $_GET. * - * @return string The route path. + * @return string The route path (before normalization). */ private function parseRequestURI(): string { @@ -138,9 +138,7 @@ private function parseRequestURI(): string // Update our globals for values likely to have been changed parse_str($this->server['QUERY_STRING'], $this->get); - $uri = URI::removeDotSegments($uri); - - return ($uri === '/' || $uri === '') ? '/' : ltrim($uri, '/'); + return URI::removeDotSegments($uri); } /** @@ -148,7 +146,7 @@ private function parseRequestURI(): string * * This method updates superglobal $_SERVER and $_GET. * - * @return string The route path. + * @return string The route path (before normalization). */ private function parseQueryString(): string { @@ -167,9 +165,7 @@ private function parseQueryString(): string // Update our globals for values likely to have been changed parse_str($this->server['QUERY_STRING'], $this->get); - $uri = URI::removeDotSegments($uri); - - return ($uri === '/' || $uri === '') ? '/' : ltrim($uri, '/'); + return URI::removeDotSegments($uri); } /** From fbf1591b96e8f286cdb1c1d2faea7512fd42d679 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 11:56:13 +0900 Subject: [PATCH 09/17] test: add URIFactoryDetectRoutePathTest --- .../HTTP/URIFactoryDetectRoutePathTest.php | 248 ++++++++++++++++++ 1 file changed, 248 insertions(+) create mode 100644 tests/system/HTTP/URIFactoryDetectRoutePathTest.php diff --git a/tests/system/HTTP/URIFactoryDetectRoutePathTest.php b/tests/system/HTTP/URIFactoryDetectRoutePathTest.php new file mode 100644 index 000000000000..b695cd6fbccd --- /dev/null +++ b/tests/system/HTTP/URIFactoryDetectRoutePathTest.php @@ -0,0 +1,248 @@ + + * + * 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 URIFactoryDetectRoutePathTest extends CIUnitTestCase +{ + private function createURIFactory(array &$server, array &$get, ?App $appConfig = null): URIFactory + { + $appConfig ??= new App(); + + return new URIFactory($server, $get, $appConfig); + } + + public function testDefault() + { + $_GET = $_SERVER = []; + + // /index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath()); + } + + public function testDefaultEmpty() + { + $_GET = $_SERVER = []; + + // / + $_SERVER['REQUEST_URI'] = '/'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = '/'; + $this->assertSame($expected, $factory->detectRoutePath()); + } + + public function testRequestURI() + { + $_GET = $_SERVER = []; + + // /index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURINested() + { + $_GET = $_SERVER = []; + + // 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->createURIFactory($_SERVER, $_GET); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURISubfolder() + { + $_GET = $_SERVER = []; + + // /ci/index.php/popcorn/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/ci/index.php/popcorn/woot'; + $_SERVER['SCRIPT_NAME'] = '/ci/index.php'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = 'popcorn/woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURINoIndex() + { + $_GET = $_SERVER = []; + + // /sub/example + $_SERVER['REQUEST_URI'] = '/sub/example'; + $_SERVER['SCRIPT_NAME'] = '/sub/index.php'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = 'example'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURINginx() + { + $_GET = $_SERVER = []; + + // /ci/index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot?code=good'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURINginxRedirecting() + { + $_GET = $_SERVER = []; + + // /?/ci/index.php/woot + $_SERVER['REQUEST_URI'] = '/?/ci/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = 'ci/woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURISuppressed() + { + $_GET = $_SERVER = []; + + // /woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/woot'; + $_SERVER['SCRIPT_NAME'] = '/'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); + } + + public function testQueryString() + { + $_GET = $_SERVER = []; + + // /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->createURIFactory($_SERVER, $_GET); + + $expected = 'ci/woot'; + $this->assertSame($expected, $factory->detectRoutePath('QUERY_STRING')); + } + + public function testQueryStringWithQueryString() + { + $_GET = $_SERVER = []; + + // /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->createURIFactory($_SERVER, $_GET); + + $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() + { + $_GET = $_SERVER = []; + + // /index.php? + $_SERVER['REQUEST_URI'] = '/index.php?'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = '/'; + $this->assertSame($expected, $factory->detectRoutePath('QUERY_STRING')); + } + + public function testPathInfoUnset() + { + $_GET = $_SERVER = []; + + // /index.php/woot?code=good#pos + $_SERVER['REQUEST_URI'] = '/index.php/woot'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createURIFactory($_SERVER, $_GET); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('PATH_INFO')); + } + + public function testPathInfoSubfolder() + { + $_GET = $_SERVER = []; + + $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->createURIFactory($_SERVER, $_GET, $appConfig); + + $expected = 'woot'; + $this->assertSame($expected, $factory->detectRoutePath('PATH_INFO')); + } +} From 97062a54a33d7c62f99b691c9b4fc2c5d09aa8e3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 12:04:02 +0900 Subject: [PATCH 10/17] refactor: replace exit() with throwing exception --- system/HTTP/URIFactory.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/system/HTTP/URIFactory.php b/system/HTTP/URIFactory.php index 67798d3b298a..72e680057ced 100644 --- a/system/HTTP/URIFactory.php +++ b/system/HTTP/URIFactory.php @@ -11,6 +11,7 @@ namespace CodeIgniter\HTTP; +use CodeIgniter\Exceptions\ConfigException; use Config\App; class URIFactory @@ -212,12 +213,9 @@ private function createURIFromRoutePath(string $routePath): URI return $uri; } if (! is_cli()) { - // Do not change exit() to exception; Request is initialized before - // setting the exception handler, so if an exception is raised, an - // error will be displayed even if in the production environment. - // @codeCoverageIgnoreStart - exit('You have an empty or invalid baseURL. The baseURL value must be set in app/Config/App.php, or through the .env file.'); - // @codeCoverageIgnoreEnd + throw new ConfigException( + 'You have an empty or invalid baseURL. The baseURL value must be set in app/Config/App.php, or through the .env file.' + ); } return new URI(); From 8105724c6420f913828aad8f3757698d0aec1ef5 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 12:38:00 +0900 Subject: [PATCH 11/17] docs: add @deprecated --- system/HTTP/IncomingRequest.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index 008a865bcdcd..692cdfaf05e9 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 URIFactory. */ 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 URIFactory. */ 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 URIFactory. */ protected function parseQueryString(): string { @@ -495,6 +501,9 @@ public function setPath(string $path, ?App $config = null) return $this; } + /** + * @deprecated Moved to URIFactory. + */ private function determineHost(App $config, string $baseURL): string { $host = parse_url($baseURL, PHP_URL_HOST); From 4c01195d0d8e6b92dd05dc184016bba899251c2a Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 12:55:01 +0900 Subject: [PATCH 12/17] refactor: change order of lines --- system/HTTP/URIFactory.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/system/HTTP/URIFactory.php b/system/HTTP/URIFactory.php index 72e680057ced..4746ca4650b1 100644 --- a/system/HTTP/URIFactory.php +++ b/system/HTTP/URIFactory.php @@ -129,8 +129,9 @@ private function parseRequestURI(): string // contain the query string (Nginx) a correct URI is found, and also // fixes the QUERY_STRING Server var and $_GET array. if (trim($uri, '/') === '' && strncmp($query, '/', 1) === 0) { - $query = explode('?', $query, 2); - $uri = $query[0]; + $query = explode('?', $query, 2); + $uri = $query[0]; + $this->server['QUERY_STRING'] = $query[1] ?? ''; } else { $this->server['QUERY_STRING'] = $query; @@ -158,9 +159,10 @@ private function parseQueryString(): string } if (strncmp($uri, '/', 1) === 0) { - $uri = explode('?', $uri, 2); + $uri = explode('?', $uri, 2); + $uri = $uri[0]; + $this->server['QUERY_STRING'] = $uri[1] ?? ''; - $uri = $uri[0]; } // Update our globals for values likely to have been changed From 6f1e5ea3ed157dc1f388cfad0529f8f61e23dae8 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 13:19:48 +0900 Subject: [PATCH 13/17] refactor: rename variable names --- system/HTTP/URIFactory.php | 40 +++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/system/HTTP/URIFactory.php b/system/HTTP/URIFactory.php index 4746ca4650b1..7a2d9a9d0be3 100644 --- a/system/HTTP/URIFactory.php +++ b/system/HTTP/URIFactory.php @@ -103,15 +103,15 @@ private function parseRequestURI(): string // parse out the query string and path. $parts = parse_url('http://dummy' . $this->server['REQUEST_URI']); $query = $parts['query'] ?? ''; - $uri = $parts['path'] ?? ''; + $path = $parts['path'] ?? ''; // Strip the SCRIPT_NAME path from the URI if ( - $uri !== '' && isset($this->server['SCRIPT_NAME'][0]) + $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('/', $uri); + $segments = $keep = explode('/', $path); foreach (explode('/', $this->server['SCRIPT_NAME']) as $i => $segment) { // If these segments are not the same then we're done @@ -122,25 +122,26 @@ private function parseRequestURI(): string array_shift($keep); } - $uri = implode('/', $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($uri, '/') === '' && strncmp($query, '/', 1) === 0) { - $query = explode('?', $query, 2); - $uri = $query[0]; + if (trim($path, '/') === '' && strncmp($query, '/', 1) === 0) { + $parts = explode('?', $query, 2); + $path = $parts[0]; + $newQuery = $query[1] ?? ''; - $this->server['QUERY_STRING'] = $query[1] ?? ''; + $this->server['QUERY_STRING'] = $newQuery; } else { $this->server['QUERY_STRING'] = $query; } - // Update our globals for values likely to have been changed + // Update our global GET for values likely to have been changed parse_str($this->server['QUERY_STRING'], $this->get); - return URI::removeDotSegments($uri); + return URI::removeDotSegments($path); } /** @@ -152,23 +153,26 @@ private function parseRequestURI(): string */ private function parseQueryString(): string { - $uri = $this->server['QUERY_STRING'] ?? @getenv('QUERY_STRING'); + $query = $this->server['QUERY_STRING'] ?? @getenv('QUERY_STRING'); - if (trim($uri, '/') === '') { + if (trim($query, '/') === '') { return '/'; } - if (strncmp($uri, '/', 1) === 0) { - $uri = explode('?', $uri, 2); - $uri = $uri[0]; + if (strncmp($query, '/', 1) === 0) { + $parts = explode('?', $query, 2); + $path = $parts[0]; + $newQuery = $parts[1] ?? ''; - $this->server['QUERY_STRING'] = $uri[1] ?? ''; + $this->server['QUERY_STRING'] = $newQuery; + } else { + $path = $query; } - // Update our globals for values likely to have been changed + // Update our global GET for values likely to have been changed parse_str($this->server['QUERY_STRING'], $this->get); - return URI::removeDotSegments($uri); + return URI::removeDotSegments($path); } /** From 4fb46afe789c957b17845892251f4d2334f20d92 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 13:31:48 +0900 Subject: [PATCH 14/17] feat: add URI::getRoutePath() --- system/HTTP/URI.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/system/HTTP/URI.php b/system/HTTP/URI.php index 6b01c79b6126..fcc90a2391ca 100644 --- a/system/HTTP/URI.php +++ b/system/HTTP/URI.php @@ -491,6 +491,20 @@ public function getPath(): string return $this->path ?? ''; } + /** + * Returns the URI path relative to baseURL. + * + * @return string The Route path. + */ + public function getRoutePath(): string + { + if ($this->routePath === null) { + throw new BadMethodCallException('The $routePath is not set.'); + } + + return $this->routePath; + } + /** * Retrieve the query string */ From d5d34ecce9ff505d3ca2486db92db0107012d9a7 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 13:33:04 +0900 Subject: [PATCH 15/17] test: add URIFactoryTest --- tests/system/HTTP/URIFactoryTest.php | 59 ++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 tests/system/HTTP/URIFactoryTest.php diff --git a/tests/system/HTTP/URIFactoryTest.php b/tests/system/HTTP/URIFactoryTest.php new file mode 100644 index 000000000000..f3eb0bcc6652 --- /dev/null +++ b/tests/system/HTTP/URIFactoryTest.php @@ -0,0 +1,59 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\HTTP; + +use CodeIgniter\Config\Factories; +use CodeIgniter\Test\CIUnitTestCase; +use Config\App; + +/** + * @backupGlobals enabled + * + * @internal + * + * @group Others + */ +final class URIFactoryTest extends CIUnitTestCase +{ + protected function setUp(): void + { + parent::setUp(); + + $_GET = $_SERVER = []; + } + + protected function tearDown(): void + { + Factories::reset('config'); + } + + public function testCreateCurrentURI() + { + // 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 URIFactory($_SERVER, $_GET, new App()); + + $uri = $factory->createCurrentURI(); + + $this->assertInstanceOf(URI::class, $uri); + $this->assertSame('http://localhost:8080/woot?code=good', (string) $uri); + $this->assertSame('woot', $uri->getPath()); + $this->assertSame('woot', $uri->getRoutePath()); + } +} From 0522efab82c337f65bad872c393207549adddce9 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 21:09:01 +0900 Subject: [PATCH 16/17] docs: add @internal --- system/HTTP/URIFactory.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/system/HTTP/URIFactory.php b/system/HTTP/URIFactory.php index 7a2d9a9d0be3..05cf3b10e7fc 100644 --- a/system/HTTP/URIFactory.php +++ b/system/HTTP/URIFactory.php @@ -58,6 +58,8 @@ public function createCurrentURI(): URI * @param string $protocol URIProtocol * * @return string The route path + * + * @internal Used for testing purposes only. */ public function detectRoutePath(string $protocol = ''): string { From 295a486943e0021cf2f207fdd2270f3414b9916e Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 10 Feb 2023 21:10:15 +0900 Subject: [PATCH 17/17] refactor: rename method --- system/HTTP/URIFactory.php | 4 ++-- tests/system/HTTP/URIFactoryTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/system/HTTP/URIFactory.php b/system/HTTP/URIFactory.php index 05cf3b10e7fc..4c423bde7a67 100644 --- a/system/HTTP/URIFactory.php +++ b/system/HTTP/URIFactory.php @@ -40,11 +40,11 @@ public function __construct(array &$server, array &$get, App $appConfig) } /** - * Create the current URI object. + * Create the current URI object from superglobals. * * This method updates superglobal $_SERVER and $_GET. */ - public function createCurrentURI(): URI + public function createFromGlobals(): URI { $routePath = $this->detectRoutePath(); diff --git a/tests/system/HTTP/URIFactoryTest.php b/tests/system/HTTP/URIFactoryTest.php index f3eb0bcc6652..e20d853aa9a2 100644 --- a/tests/system/HTTP/URIFactoryTest.php +++ b/tests/system/HTTP/URIFactoryTest.php @@ -49,7 +49,7 @@ public function testCreateCurrentURI() $factory = new URIFactory($_SERVER, $_GET, new App()); - $uri = $factory->createCurrentURI(); + $uri = $factory->createFromGlobals(); $this->assertInstanceOf(URI::class, $uri); $this->assertSame('http://localhost:8080/woot?code=good', (string) $uri);