From 89d588fab6dadc904c76859b4a8a86a3e8d11189 Mon Sep 17 00:00:00 2001 From: MGatner Date: Tue, 4 May 2021 18:14:19 +0000 Subject: [PATCH 1/2] Improve URI handling --- system/HTTP/IncomingRequest.php | 41 +++++++-------- system/HTTP/URI.php | 55 ++++++++++++--------- tests/system/HTTP/URITest.php | 10 ++-- user_guide_src/source/changelogs/v4.1.2.rst | 1 + 4 files changed, 58 insertions(+), 49 deletions(-) diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index 513bc580679c..e3de4aed3e0a 100755 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -14,6 +14,7 @@ use CodeIgniter\HTTP\Exceptions\HTTPException; use CodeIgniter\HTTP\Files\FileCollection; use CodeIgniter\HTTP\Files\UploadedFile; +use CodeIgniter\HTTP\URI; use Config\App; use Config\Services; use InvalidArgumentException; @@ -149,7 +150,7 @@ public function __construct($config, URI $uri = null, $body = 'php://input', Use $this->populateHeaders(); - // Get our current URI. + // Determine the current URI // NOTE: This WILL NOT match the actual URL in the browser since for // everything this cares about (and the router, etc) is the portion // AFTER the script name. So, if hosted in a sub-folder this will @@ -158,6 +159,12 @@ public function __construct($config, URI $uri = null, $body = 'php://input', Use $this->detectURI($config->uriProtocol, $config->baseURL); + // Check if the baseURL scheme needs to be coerced into its secure version + if ($config->forceGlobalSecureRequests && $this->uri->getScheme() === 'http') + { + $this->uri->setScheme('https'); + } + $this->validLocales = $config->supportedLocales; $this->detectLocale($config); @@ -610,11 +617,11 @@ protected function detectURI(string $protocol, string $baseURL) // It's possible the user forgot a trailing slash on their // baseURL, so let's help them out. - $baseURL = ! empty($baseURL) ? rtrim($baseURL, '/ ') . '/' : $baseURL; + $baseURL = $baseURL === '' ? $baseURL : rtrim($baseURL, '/ ') . '/'; // Based on our baseURL provided by the developer // set our current domain name, scheme - if (! empty($baseURL)) + if ($baseURL !== '') { $this->uri->setScheme(parse_url($baseURL, PHP_URL_SCHEME)); $this->uri->setHost(parse_url($baseURL, PHP_URL_HOST)); @@ -758,12 +765,9 @@ protected function parseRequestURI(): string parse_str($_SERVER['QUERY_STRING'], $_GET); - if ($uri === '/' || $uri === '') - { - return '/'; - } + $uri = URI::removeDotSegments($uri); - return $this->removeRelativeDirectory($uri); + return ($uri === '/' || $uri === '') ? '/' : ltrim($uri, '/'); } //-------------------------------------------------------------------- @@ -793,7 +797,9 @@ protected function parseQueryString(): string parse_str($_SERVER['QUERY_STRING'], $_GET); - return $this->removeRelativeDirectory($uri); + $uri = URI::removeDotSegments($uri); + + return ($uri === '/' || $uri === '') ? '/' : ltrim($uri, '/'); } //-------------------------------------------------------------------- @@ -806,22 +812,13 @@ protected function parseQueryString(): string * @param string $uri * * @return string + * + * @deprecated Use URI::removeDotSegments() directly */ protected function removeRelativeDirectory(string $uri): string { - $uris = []; - $tok = strtok($uri, '/'); - while ($tok !== false) - { - if ((! empty($tok) || $tok === '0') && $tok !== '..') - { - $uris[] = $tok; - } - $tok = strtok('/'); - } + $uri = URI::removeDotSegments($uri); - return implode('/', $uris); + return $uri === '/' ? $uri : ltrim($uri, '/'); } - - // -------------------------------------------------------------------- } diff --git a/system/HTTP/URI.php b/system/HTTP/URI.php index 0ec29286319e..4eb1f39f0e73 100644 --- a/system/HTTP/URI.php +++ b/system/HTTP/URI.php @@ -12,7 +12,6 @@ namespace CodeIgniter\HTTP; use CodeIgniter\HTTP\Exceptions\HTTPException; -use Config\App; use InvalidArgumentException; /** @@ -166,7 +165,7 @@ public static function createURIString(string $scheme = null, string $authority $uri .= $authority; } - if ($path !== '') + if (isset($path) && $path !== '') { $uri .= substr($uri, -1, 1) !== '/' ? '/' . ltrim($path, '/') : ltrim($path, '/'); } @@ -465,7 +464,7 @@ public function showPassword(bool $val = true) */ public function getHost(): string { - return $this->host; + return $this->host ?? ''; } //-------------------------------------------------------------------- @@ -665,33 +664,45 @@ public function getTotalSegments(): int //-------------------------------------------------------------------- /** - * Allow the URI to be output as a string by simply casting it to a string - * or echoing out. + * Formats the URI as a string. + * + * Warning: For backwards-compatability this method + * assumes URIs with the same host as baseURL should + * be relative to the project's configuration. + * This aspect of __toString() is deprecated and should be avoided. + * + * @return string */ public function __toString(): string { - // If hosted in a sub-folder, we will have additional - // segments that show up prior to the URI path we just - // grabbed from the request, so add it on if necessary. - $config = config(App::class); - $baseUri = new self($config->baseURL); - $basePath = trim($baseUri->getPath(), '/') . '/'; - $path = $this->getPath(); - $trimPath = ltrim($path, '/'); - - if ($basePath !== '/' && strpos($trimPath, $basePath) !== 0) - { - $path = $basePath . $trimPath; - } + $path = $this->getPath(); + $scheme = $this->getScheme(); + + // Check if this is an internal URI + $config = config('App'); + $baseUri = new self($config->baseURL); - // force https if needed - if ($config->forceGlobalSecureRequests) + // If the hosts matches then assume this should be relative to baseURL + if ($this->getHost() === $baseUri->getHost()) { - $this->setScheme('https'); + // Check for additional segments + $basePath = trim($baseUri->getPath(), '/') . '/'; + $trimPath = ltrim($path, '/'); + + if ($basePath !== '/' && strpos($trimPath, $basePath) !== 0) + { + $path = $basePath . $trimPath; + } + + // Check for forced HTTPS + if ($config->forceGlobalSecureRequests) + { + $scheme = 'https'; + } } return static::createURIString( - $this->getScheme(), $this->getAuthority(), $path, // Absolute URIs should use a "/" for an empty path + $scheme, $this->getAuthority(), $path, // Absolute URIs should use a "/" for an empty path $this->getQuery(), $this->getFragment() ); } diff --git a/tests/system/HTTP/URITest.php b/tests/system/HTTP/URITest.php index 2486195f463d..84c9e12d59ae 100644 --- a/tests/system/HTTP/URITest.php +++ b/tests/system/HTTP/URITest.php @@ -1007,19 +1007,19 @@ public function testForceGlobalSecureRequests() Factories::injectMock('config', 'App', $config); - $request = Services::request($config); - $request->uri = new URI('http://example.com/ci/v4/controller/method'); + $uri = new URI('http://example.com/ci/v4/controller/method'); + $request = new IncomingRequest($config, $uri, 'php://input', new UserAgent()); Services::injectMock('request', $request); - // going through request + // Detected by request $this->assertEquals('https://example.com/ci/v4/controller/method', (string) $request->uri); - // standalone + // Standalone $uri = new URI('http://example.com/ci/v4/controller/method'); $this->assertEquals('https://example.com/ci/v4/controller/method', (string) $uri); - $this->assertEquals($uri->getPath(), $request->uri->getPath()); + $this->assertEquals(trim($uri->getPath(), '/'), trim($request->uri->getPath(), '/')); } public function testZeroAsURIPath() diff --git a/user_guide_src/source/changelogs/v4.1.2.rst b/user_guide_src/source/changelogs/v4.1.2.rst index fe661ca930fe..76bad454c54e 100644 --- a/user_guide_src/source/changelogs/v4.1.2.rst +++ b/user_guide_src/source/changelogs/v4.1.2.rst @@ -41,6 +41,7 @@ Deprecations: - Deprecated ``ControllerTester`` to use the ``ControllerTestTrait`` instead. - Consolidated and deprecated ``ControllerResponse`` and ``FeatureResponse`` in favor of ``TestResponse``. - Deprecated ``Time::instance()``, use ``Time::createFromInstance()`` instead (now accepts ``DateTimeInterface``). +- Deprecated ``IncomingRequest::removeRelativeDirectory()``, use ``URI::removeDotSegments()`` instead Bugs Fixed: From 584bcd1aed9f076bb3e3b6400a10159afaa75f4b Mon Sep 17 00:00:00 2001 From: MGatner Date: Tue, 4 May 2021 18:54:41 +0000 Subject: [PATCH 2/2] Fix test config type --- system/HTTP/IncomingRequest.php | 2 +- tests/system/API/ResponseTraitTest.php | 23 +++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index e3de4aed3e0a..1c64b7568110 100755 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -124,7 +124,7 @@ class IncomingRequest extends Request /** * Constructor * - * @param object $config + * @param App $config * @param URI $uri * @param string|null $body * @param UserAgent $userAgent diff --git a/tests/system/API/ResponseTraitTest.php b/tests/system/API/ResponseTraitTest.php index 01daf3e66b90..692509f0f949 100644 --- a/tests/system/API/ResponseTraitTest.php +++ b/tests/system/API/ResponseTraitTest.php @@ -8,6 +8,7 @@ use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockIncomingRequest; use CodeIgniter\Test\Mock\MockResponse; +use Config\App; use stdClass; class ResponseTraitTest extends CIUnitTestCase @@ -30,7 +31,8 @@ protected function setUp(): void protected function makeController(array $userConfig = [], string $uri = 'http://example.com', array $userHeaders = []) { - $config = [ + $config = new App(); + foreach ([ 'baseURL' => 'http://example.com/', 'uriProtocol' => 'REQUEST_URI', 'defaultLocale' => 'en', @@ -44,9 +46,10 @@ protected function makeController(array $userConfig = [], string $uri = 'http:// 'cookieHTTPOnly' => false, 'proxyIPs' => [], 'cookieSameSite' => 'Lax', - ]; - - $config = array_merge($config, $userConfig); + ] as $key => $value) + { + $config->$key = $value; + } if (is_null($this->request)) { @@ -472,7 +475,8 @@ public function testXMLFormatter() public function testFormatByRequestNegotiateIfFormatIsNotJsonOrXML() { - $config = [ + $config = new App(); + foreach ([ 'baseURL' => 'http://example.com/', 'uriProtocol' => 'REQUEST_URI', 'defaultLocale' => 'en', @@ -486,10 +490,13 @@ public function testFormatByRequestNegotiateIfFormatIsNotJsonOrXML() 'cookieHTTPOnly' => false, 'proxyIPs' => [], 'cookieSameSite' => 'Lax', - ]; + ] as $key => $value) + { + $config->$key = $value; + } - $request = new MockIncomingRequest((object) $config, new URI($config['baseURL']), null, new UserAgent()); - $response = new MockResponse((object) $config); + $request = new MockIncomingRequest($config, new URI($config->baseURL), null, new UserAgent()); + $response = new MockResponse($config); $controller = new class($request, $response) {