From 42a60ce50059ec4f492fd72078b6b3a972853ee2 Mon Sep 17 00:00:00 2001 From: MGatner Date: Wed, 5 May 2021 01:38:32 +0000 Subject: [PATCH 1/4] Add URL class --- system/HTTP/URL.php | 177 +++++++++++++++++ tests/system/HTTP/URLTest.php | 364 ++++++++++++++++++++++++++++++++++ 2 files changed, 541 insertions(+) create mode 100644 system/HTTP/URL.php create mode 100644 tests/system/HTTP/URLTest.php diff --git a/system/HTTP/URL.php b/system/HTTP/URL.php new file mode 100644 index 000000000000..ba8cbf30b155 --- /dev/null +++ b/system/HTTP/URL.php @@ -0,0 +1,177 @@ + + * + * 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\IncomingRequest; +use Config\App; +use Config\Services; +use InvalidArgumentException; + +/** + * URI wrapper to work with complete project URLs. + * This class creates immutable instances. + */ +class URL +{ + /** + * Underlying URI instance + * + * @var URI + */ + private $uri; + + /** + * Path relative to baseURL + * + * @var string + */ + protected $relativePath; + + //-------------------------------------------------------------------- + + /** + * Returns an instance representing the current URL. + * + * @return static + */ + final public static function base() + { + return new static(''); + } + + /** + * Returns an instance representing the current URL. + * + * @param string $uri + * + * @return static + */ + final public static function to(string $uri) + { + // Check for a named or reverse-route + if ($route = Services::routes()->reverseRoute($uri)) + { + return new static($route); + } + + return new static($uri); + } + + /** + * Returns an instance representing the current URL. + * + * @return static + */ + final public static function current() + { + return static::fromRequest(Services::request()); + } + + /** + * Returns an instance representing the URL from + * an Incoming Request (as defined by Config\App). + * + * @param IncomingRequest $request + * + * @return static + */ + final public static function fromRequest(IncomingRequest $request) + { + $path = $request->detectPath($request->config->uriProtocol); + $query = isset($_SERVER['QUERY_STRING']) ? '?' . $_SERVER['QUERY_STRING'] : ''; + + return new static($path . $query, $request->config); + } + + //-------------------------------------------------------------------- + + /** + * Store the App configuration and create the URI + * instance from the relative path. + * + * @param string $relativePath + * @param App|null $config + */ + final public function __construct(string $relativePath = '', App $config = null) + { + $config = $config ?? config('App'); + + if ($config->baseURL === '') + { + throw new InvalidArgumentException('URL class requires a valid baseURL.'); + } + if (is_int(strpos($relativePath, '://'))) + { + throw new InvalidArgumentException('URL class only accepts relative paths.'); + } + + $this->relativePath = ltrim(URI::removeDotSegments($relativePath), '/'); + + // Build the full URL based on $config and $relativePath + $url = rtrim($config->baseURL, '/ ') . '/'; + + // Check for an index page + if ($config->indexPage !== '') + { + $url .= $config->indexPage; + + // If the relative path has anything other than ? (for query) we need a separator + if ($this->relativePath !== '' && strncmp($this->relativePath, '?', 1) !== 0) + { + $url .= '/'; + } + } + + $url .= $this->relativePath; + + $this->uri = new URI($url); + + // Check if the baseURL scheme needs to be coerced into its secure version + if ($config->forceGlobalSecureRequests && $this->uri->getScheme() === 'http') + { + $this->uri->setScheme('https'); + } + } + + /** + * Returns the path relative to baseUrl, loosely + * what was passed to the constructor. + * + * @return string + */ + public function getPath(): string + { + return $this->relativePath; + } + + /** + * Returns the underlying URI instance. + * In order for this instance to remain + * immutable a clone is returned. + * + * @return URI + */ + public function getUri(): URI + { + return clone $this->uri; + } + + /** + * Returns this URL as a string. + * + * @return string + */ + public function __toString(): string + { + return (string) $this->uri; + } +} diff --git a/tests/system/HTTP/URLTest.php b/tests/system/HTTP/URLTest.php new file mode 100644 index 000000000000..dcce022d7810 --- /dev/null +++ b/tests/system/HTTP/URLTest.php @@ -0,0 +1,364 @@ +config = new App(); + $this->config->baseURL = 'http://example.com/'; + $this->config->indexPage = 'index.php'; + $this->config->forceGlobalSecureRequests = false; + + Factories::injectMock('config', 'App', $this->config); + } + + //-------------------------------------------------------------------- + + /** + * Updates a single Config value. + * + * @param string $uri + * + * @return $this + */ + private function setConfig(string $key, string $value): self + { + $this->config->$key = $value; + + return $this; + } + + /** + * Fakes the current URL and re-initializes the Request. + * + * @param string $uri + * + * @return $this + */ + private function setCurrent(string $uri): self + { + $parts = parse_url($uri); + if ($parts === false) + { + throw InvalidArgumentException('Could not parse URI: ' . $uri); + } + + $_SERVER['REQUEST_URI'] = $parts['path'] ?? ''; + + if (! empty($parts['host'])) + { + $_SERVER['HTTP_HOST'] = $parts['host']; + } + if (! empty($parts['query'])) + { + $_SERVER['REQUEST_URI'] .= '?' . $parts['query']; + } + if (isset($parts['scheme'])) + { + $_SERVER['HTTPS'] = strtolower(rtrim($parts['scheme'], ':/')) === 'https' ? 'on' : 'off'; + } + + // Recreate the Incoming Request to force detection + $request = Services::request($this->config, false); + Services::injectMock('request', $request); + + return $this; + } + + //-------------------------------------------------------------------- + + public function testDefault() + { + $url = new URL(); + + $result = $this->getPrivateProperty($url, 'relativePath'); + $this->assertSame('', $result); + + $result = $this->getPrivateProperty($url, 'uri'); + $this->assertInstanceOf(URI::class, $result); + } + + public function testGetPath() + { + $result = (new URL('banana'))->getPath(); + + $this->assertSame('banana', $result); + } + + public function testGetUri() + { + $url = new URL('banana'); + $result = $url->getUri(); + + $this->assertInstanceOf(URI::class, $result); + $this->assertSame((string) $url, (string) $result); + } + + public function testFullUri() + { + $this->expectException('InvalidArgumentException'); + $this->expectExceptionMessage('URL class only accepts relative paths.'); + + $url = new URL('http://example.com/bam'); + } + + public function testInvalidBaseURL() + { + $this->config->baseURL = ''; + + $this->expectException('InvalidArgumentException'); + $this->expectExceptionMessage('URL class requires a valid baseURL.'); + + $url = new URL('banana', $this->config); + } + + /** + * This test mostly replicates URI::testRemoveDotSegments() + * but it demonstrates how input is cleaned up. + * + * @dataProvider pathProvider + */ + public function testConstructorPath(string $path, string $expected) + { + $result = (new URL($path))->getPath(); + + $this->assertSame($expected, $result); + } + + /** + * @dataProvider configProvider + */ + public function testConstructorConfig(array $configs, string $baseURL) + { + foreach ($configs as $key => $value) + { + $this->setConfig($key, $value); + } + + $url = new URL('testaroo', $this->config); + $expected = rtrim($baseURL, '/') . '/testaroo'; + + $this->assertSame($expected, (string) $url); + } + + /** + * @dataProvider currentProvider + */ + public function testCurrent(string $uri, string $expected = null) + { + $this->setCurrent($uri); + + $url = URL::current(); + + $this->assertInstanceOf(URL::class, $url); + $this->assertSame($expected ?? $uri, (string) $url); + } + + /** + * @dataProvider configProvider + */ + public function testBase(array $configs, string $expected) + { + foreach ($configs as $key => $value) + { + $this->setConfig($key, $value); + } + + Factories::injectMock('config', 'App', $this->config); + + $url = URL::base(); + + $this->assertSame($expected, (string) $url); + } + + public function testTo() + { + $url = URL::to('fruit/basket'); + + $this->assertSame('http://example.com/index.php/fruit/basket', (string) $url); + } + + public function testToNamedRoute() + { + Services::routes()->add('apples', 'Home::index', ['as' => 'home']); + + $url = URL::to('home'); + + $this->assertSame('http://example.com/index.php/apples', (string) $url); + } + + public function testToReverseRoute() + { + Services::routes()->add('oranges', 'Basket::fruit'); + + $url = URL::to('App\Controllers\Basket::fruit'); + + $this->assertSame('http://example.com/index.php/oranges', (string) $url); + } + + //-------------------------------------------------------------------- + + public function pathProvider(): array + { + return [ + [ + '', + '', + ], + [ + '/', + '', + ], + [ + '//', + '', + ], + [ + '/foo/..', + '', + ], + [ + '/foo', + 'foo', + ], + [ + 'foo', + 'foo', + ], + [ + 'foo/', + 'foo/', + ], + [ + '?bar=bam', + '?bar=bam', + ], + [ + 'foo?bar=bam', + 'foo?bar=bam', + ], + ]; + } + + public function configProvider(): array + { + return [ + [ + [ + 'baseURL' => 'http://bananas.com', + ], + 'http://bananas.com/index.php', + ], + [ + [ + 'baseURL' => 'http://bananas.com/', + ], + 'http://bananas.com/index.php', + ], + [ + [ + 'baseURL' => 'http://bananas.com/subfolder/', + ], + 'http://bananas.com/subfolder/index.php', + ], + [ + [ + 'indexPage' => '', + ], + 'http://example.com/', + ], + [ + [ + 'baseURL' => 'http://bananas.com/subfolder/', + 'indexPage' => '', + ], + 'http://bananas.com/subfolder/', + ], + [ + [ + 'forceGlobalSecureRequests' => true, + ], + 'https://example.com/index.php', + ], + [ + [ + 'baseURL' => 'http://bananas.com/', + 'forceGlobalSecureRequests' => true, + ], + 'https://bananas.com/index.php', + ], + [ + [ + 'baseURL' => 'https://bananas.com/subfolder/', + 'forceGlobalSecureRequests' => true, + ], + 'https://bananas.com/subfolder/index.php', + ], + ]; + } + + public function currentProvider(): array + { + return [ + [ + '', + 'http://example.com/index.php', + ], + [ + '/', + 'http://example.com/index.php', + ], + [ + 'http://example.com/index.php/sauce', + null, + ], + [ + 'http://example.com/index.php/sauce/', + null, + ], + [ + '/sauce/', + 'http://example.com/index.php/sauce/', + ], + [ + 'http://bananas.com/index.php', + 'http://example.com/index.php', + ], + [ + 'https://example.com/index.php', + 'http://example.com/index.php', + ], + [ + '?blahblah=true', + 'http://example.com/index.php?blahblah=true', + ], + [ + 'http://example.com/index.php?blahblah=true', + 'http://example.com/index.php?blahblah=true', + ], + ]; + } +} From 10b605b85bda3928a0014b0bd185d6b633a66a1c Mon Sep 17 00:00:00 2001 From: MGatner Date: Wed, 5 May 2021 02:14:09 +0000 Subject: [PATCH 2/4] Fix base URL logic --- system/HTTP/URL.php | 20 ++++++++++++------ tests/system/HTTP/URLTest.php | 38 ++++++++++++++++++++++++----------- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/system/HTTP/URL.php b/system/HTTP/URL.php index ba8cbf30b155..e97d92508c6b 100644 --- a/system/HTTP/URL.php +++ b/system/HTTP/URL.php @@ -39,26 +39,34 @@ class URL //-------------------------------------------------------------------- /** - * Returns an instance representing the current URL. + * Returns an instance representing the base URL. + * + * @param string $uri Additional URI string to include * * @return static */ - final public static function base() + final public static function base(string $uri = '') { - return new static(''); + // Base URLs never include the index page + $config = clone config('App'); + $config->indexPage = ''; + + return new static($uri, $config); } /** - * Returns an instance representing the current URL. + * Returns an instance representing a routed URL. * - * @param string $uri + * @param string $uri Named route, reverse route, or URI string * * @return static */ final public static function to(string $uri) { + $uri = rtrim($uri, '/ '); + // Check for a named or reverse-route - if ($route = Services::routes()->reverseRoute($uri)) + if ($uri !== '' && $route = Services::routes()->reverseRoute($uri)) { return new static($route); } diff --git a/tests/system/HTTP/URLTest.php b/tests/system/HTTP/URLTest.php index dcce022d7810..43afef947302 100644 --- a/tests/system/HTTP/URLTest.php +++ b/tests/system/HTTP/URLTest.php @@ -151,17 +151,16 @@ public function testConstructorPath(string $path, string $expected) /** * @dataProvider configProvider */ - public function testConstructorConfig(array $configs, string $baseURL) + public function testConstructorConfig(array $configs, string $baseURL, string $siteURL) { foreach ($configs as $key => $value) { $this->setConfig($key, $value); } - $url = new URL('testaroo', $this->config); - $expected = rtrim($baseURL, '/') . '/testaroo'; + $url = new URL('testaroo', $this->config); - $this->assertSame($expected, (string) $url); + $this->assertSame($siteURL, (string) $url); } /** @@ -180,7 +179,7 @@ public function testCurrent(string $uri, string $expected = null) /** * @dataProvider configProvider */ - public function testBase(array $configs, string $expected) + public function testBase(array $configs, string $baseURL, string $siteURL) { foreach ($configs as $key => $value) { @@ -191,7 +190,14 @@ public function testBase(array $configs, string $expected) $url = URL::base(); - $this->assertSame($expected, (string) $url); + $this->assertSame($baseURL, (string) $url); + } + + public function testBaseWithUri() + { + $url = URL::base('images/cat.gif'); + + $this->assertSame('http://example.com/images/cat.gif', (string) $url); } public function testTo() @@ -270,25 +276,29 @@ public function configProvider(): array [ 'baseURL' => 'http://bananas.com', ], - 'http://bananas.com/index.php', + 'http://bananas.com/', + 'http://bananas.com/index.php/testaroo', ], [ [ 'baseURL' => 'http://bananas.com/', ], - 'http://bananas.com/index.php', + 'http://bananas.com/', + 'http://bananas.com/index.php/testaroo', ], [ [ 'baseURL' => 'http://bananas.com/subfolder/', ], - 'http://bananas.com/subfolder/index.php', + 'http://bananas.com/subfolder/', + 'http://bananas.com/subfolder/index.php/testaroo', ], [ [ 'indexPage' => '', ], 'http://example.com/', + 'http://example.com/testaroo', ], [ [ @@ -296,26 +306,30 @@ public function configProvider(): array 'indexPage' => '', ], 'http://bananas.com/subfolder/', + 'http://bananas.com/subfolder/testaroo', ], [ [ 'forceGlobalSecureRequests' => true, ], - 'https://example.com/index.php', + 'https://example.com/', + 'https://example.com/index.php/testaroo', ], [ [ 'baseURL' => 'http://bananas.com/', 'forceGlobalSecureRequests' => true, ], - 'https://bananas.com/index.php', + 'https://bananas.com/', + 'https://bananas.com/index.php/testaroo', ], [ [ 'baseURL' => 'https://bananas.com/subfolder/', 'forceGlobalSecureRequests' => true, ], - 'https://bananas.com/subfolder/index.php', + 'https://bananas.com/subfolder/', + 'https://bananas.com/subfolder/index.php/testaroo', ], ]; } From 9fcab55fb10ffb9da3587da4bd67fd03f0bb911d Mon Sep 17 00:00:00 2001 From: MGatner Date: Wed, 5 May 2021 15:34:16 +0000 Subject: [PATCH 3/4] Refactor static methods --- system/HTTP/URL.php | 71 ++++++++++++++++++++++++----------- tests/system/HTTP/URLTest.php | 18 ++++----- 2 files changed, 58 insertions(+), 31 deletions(-) diff --git a/system/HTTP/URL.php b/system/HTTP/URL.php index e97d92508c6b..12e73c826327 100644 --- a/system/HTTP/URL.php +++ b/system/HTTP/URL.php @@ -12,6 +12,7 @@ namespace CodeIgniter\HTTP; use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\Router\Exceptions\RouterException; use Config\App; use Config\Services; use InvalidArgumentException; @@ -20,7 +21,7 @@ * URI wrapper to work with complete project URLs. * This class creates immutable instances. */ -class URL +final class URL { /** * Underlying URI instance @@ -34,18 +35,29 @@ class URL * * @var string */ - protected $relativePath; + private $relativePath; //-------------------------------------------------------------------- /** - * Returns an instance representing the base URL. + * Creates the base URL. + * + * @return static + */ + public static function base() + { + return static::public(''); + } + + /** + * Creates a URL to unrouted public files (typically assets). + * Similar to base_url('path/to/file') * * @param string $uri Additional URI string to include * * @return static */ - final public static function base(string $uri = '') + public static function public(string $uri) { // Base URLs never include the index page $config = clone config('App'); @@ -55,33 +67,45 @@ final public static function base(string $uri = '') } /** - * Returns an instance representing a routed URL. - * - * @param string $uri Named route, reverse route, or URI string + * Returns an instance representing the current URL. * * @return static */ - final public static function to(string $uri) + public static function current() { - $uri = rtrim($uri, '/ '); - - // Check for a named or reverse-route - if ($uri !== '' && $route = Services::routes()->reverseRoute($uri)) - { - return new static($route); - } + return static::fromRequest(Services::request()); + } - return new static($uri); + /** + * Creates a framework URL. + * + * @param string $uri + * + * @return static + */ + public static function to(string $uri) + { + return new static(rtrim($uri, '/ ')); } /** - * Returns an instance representing the current URL. + * Creates a URL to a named or reverse route. + * + * @param string $uri Named or reverse route * * @return static + * + * @throws RouterException */ - final public static function current() + public static function route(string $uri) { - return static::fromRequest(Services::request()); + // Check for a named or reverse-route + if ($route = Services::routes()->reverseRoute($uri)) + { + return new static($route); + } + + throw RouterException::forInvalidRoute($uri); } /** @@ -92,7 +116,7 @@ final public static function current() * * @return static */ - final public static function fromRequest(IncomingRequest $request) + public static function fromRequest(IncomingRequest $request) { $path = $request->detectPath($request->config->uriProtocol); $query = isset($_SERVER['QUERY_STRING']) ? '?' . $_SERVER['QUERY_STRING'] : ''; @@ -109,7 +133,7 @@ final public static function fromRequest(IncomingRequest $request) * @param string $relativePath * @param App|null $config */ - final public function __construct(string $relativePath = '', App $config = null) + public function __construct(string $relativePath = '', App $config = null) { $config = $config ?? config('App'); @@ -175,11 +199,14 @@ public function getUri(): URI /** * Returns this URL as a string. + * Since this is typically for routing and + * link purposes we strip any queries, but + * they can be accessed via getUri(). * * @return string */ public function __toString(): string { - return (string) $this->uri; + return (string) $this->getUri()->setQuery(''); } } diff --git a/tests/system/HTTP/URLTest.php b/tests/system/HTTP/URLTest.php index 43afef947302..5a459be2d86a 100644 --- a/tests/system/HTTP/URLTest.php +++ b/tests/system/HTTP/URLTest.php @@ -193,9 +193,9 @@ public function testBase(array $configs, string $baseURL, string $siteURL) $this->assertSame($baseURL, (string) $url); } - public function testBaseWithUri() + public function testPublic() { - $url = URL::base('images/cat.gif'); + $url = URL::public('images/cat.gif'); $this->assertSame('http://example.com/images/cat.gif', (string) $url); } @@ -207,20 +207,20 @@ public function testTo() $this->assertSame('http://example.com/index.php/fruit/basket', (string) $url); } - public function testToNamedRoute() + public function testNamedRoute() { - Services::routes()->add('apples', 'Home::index', ['as' => 'home']); + Services::routes()->add('apples', 'Home::index', ['as' => 'orchard']); - $url = URL::to('home'); + $url = URL::route('orchard'); $this->assertSame('http://example.com/index.php/apples', (string) $url); } - public function testToReverseRoute() + public function testReverseRoute() { Services::routes()->add('oranges', 'Basket::fruit'); - $url = URL::to('App\Controllers\Basket::fruit'); + $url = URL::route('App\Controllers\Basket::fruit'); $this->assertSame('http://example.com/index.php/oranges', (string) $url); } @@ -367,11 +367,11 @@ public function currentProvider(): array ], [ '?blahblah=true', - 'http://example.com/index.php?blahblah=true', + 'http://example.com/index.php', ], [ 'http://example.com/index.php?blahblah=true', - 'http://example.com/index.php?blahblah=true', + 'http://example.com/index.php', ], ]; } From 16c364e2dd3897c2e2e3c0ce378f12d221660fb2 Mon Sep 17 00:00:00 2001 From: MGatner Date: Wed, 5 May 2021 15:57:27 +0000 Subject: [PATCH 4/4] Add spoofing, fix tests --- system/HTTP/URL.php | 24 ++++++++++++++++++++ tests/system/HTTP/URLTest.php | 41 +++++++++++++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/system/HTTP/URL.php b/system/HTTP/URL.php index 12e73c826327..0e3286dd618d 100644 --- a/system/HTTP/URL.php +++ b/system/HTTP/URL.php @@ -23,6 +23,13 @@ */ final class URL { + /** + * Explicit URL to use for current. + * + * @var static|null + */ + private static $current; + /** * Underlying URI instance * @@ -73,6 +80,11 @@ public static function public(string $uri) */ public static function current() { + if (isset(self::$current)) + { + return self::$current; + } + return static::fromRequest(Services::request()); } @@ -124,6 +136,18 @@ public static function fromRequest(IncomingRequest $request) return new static($path . $query, $request->config); } + /** + * Injects a URL to use for the current. + * Useful for testing components whose behavior + * depends on the URL being visited. + * + * @param URL|null $url + */ + public static function setCurrent(?URL $url) + { + self::$current = $url; + } + //-------------------------------------------------------------------- /** diff --git a/tests/system/HTTP/URLTest.php b/tests/system/HTTP/URLTest.php index 5a459be2d86a..37d771567261 100644 --- a/tests/system/HTTP/URLTest.php +++ b/tests/system/HTTP/URLTest.php @@ -3,6 +3,7 @@ namespace CodeIgniter\HTTP; use CodeIgniter\Config\Factories; +use CodeIgniter\Router\Exceptions\RouterException; use CodeIgniter\Test\CIUnitTestCase; use Config\App; use Config\Services; @@ -35,6 +36,13 @@ protected function setUp(): void Factories::injectMock('config', 'App', $this->config); } + protected function tearDown(): void + { + parent::tearDown(); + + URL::setCurrent(null); + } + //-------------------------------------------------------------------- /** @@ -209,7 +217,7 @@ public function testTo() public function testNamedRoute() { - Services::routes()->add('apples', 'Home::index', ['as' => 'orchard']); + Services::routes()->add('apples', '\App\Controllers\Home::index', ['as' => 'orchard']); $url = URL::route('orchard'); @@ -218,13 +226,42 @@ public function testNamedRoute() public function testReverseRoute() { - Services::routes()->add('oranges', 'Basket::fruit'); + Services::routes()->add('oranges', '\App\Controllers\Basket::fruit'); $url = URL::route('App\Controllers\Basket::fruit'); $this->assertSame('http://example.com/index.php/oranges', (string) $url); } + public function testMissingRoute() + { + $this->expectException(RouterException::class); + $this->expectExceptionMessage('Foo\Bar\Baz::bam route cannot be found while reverse-routing'); + + URL::route('Foo\Bar\Baz::bam'); + } + + public function testSetCurrent() + { + $url = new URL('fruitcake'); + + URL::setCurrent($url); + $result = URL::current(); + + $this->assertSame($url, $result); + } + + public function testSetCurrentNull() + { + $url = new URL('fruitcake'); + + URL::setCurrent($url); + URL::setCurrent(null); + $result = URL::current(); + + $this->assertNotSame($url, $result); + } + //-------------------------------------------------------------------- public function pathProvider(): array