Merge pull request #4646 from MGatner/uri-internal

MGatner · web-flow · commit 14728646d3b6 · 2021-05-05T11:14:14.000-04:00
Internal URI handling
diff --git 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;
@@ -123,7 +124,7 @@ class IncomingRequest extends Request
 	/**
 	 * Constructor
 	 *
-	 * @param object      $config
+	 * @param App         $config
 	 * @param URI         $uri
 	 * @param string|null $body
 	 * @param UserAgent   $userAgent
@@ -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
@@ -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/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)
 		{
diff --git 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
@@ -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:
 
