From d01c783a6f99e851882902dba35dbfc9f8d013d2 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 20 Feb 2023 15:20:20 +0900 Subject: [PATCH 01/34] refactor: current URI creation --- system/Config/Services.php | 11 ++- system/HTTP/IncomingRequest.php | 59 +++-------- system/HTTP/URI.php | 5 - system/Test/FeatureTestTrait.php | 21 ++-- tests/_support/Config/Services.php | 10 ++ tests/system/HTTP/IncomingRequestTest.php | 97 ++++++------------- tests/system/HTTP/NegotiateTest.php | 3 +- tests/system/HTTP/RedirectResponseTest.php | 7 +- .../SiteURIFactoryDetectRoutePathTest.php | 81 ++++++++++++++++ tests/system/HTTP/URITest.php | 37 ++++--- 10 files changed, 186 insertions(+), 145 deletions(-) diff --git a/system/Config/Services.php b/system/Config/Services.php index ffca4fa804ff..2e1e7879c087 100644 --- a/system/Config/Services.php +++ b/system/Config/Services.php @@ -38,6 +38,7 @@ use CodeIgniter\HTTP\RequestInterface; use CodeIgniter\HTTP\Response; use CodeIgniter\HTTP\ResponseInterface; +use CodeIgniter\HTTP\SiteURIFactory; use CodeIgniter\HTTP\URI; use CodeIgniter\HTTP\UserAgent; use CodeIgniter\Images\Handlers\BaseHandler; @@ -52,6 +53,7 @@ use CodeIgniter\Session\Handlers\Database\PostgreHandler; use CodeIgniter\Session\Handlers\DatabaseHandler; use CodeIgniter\Session\Session; +use CodeIgniter\Superglobals; use CodeIgniter\Throttle\Throttler; use CodeIgniter\Typography\Typography; use CodeIgniter\Validation\Validation; @@ -744,7 +746,7 @@ public static function toolbar(?ToolbarConfig $config = null, bool $getShared = * * @param string $uri * - * @return URI + * @return URI The current URI if $uri is null. */ public static function uri(?string $uri = null, bool $getShared = true) { @@ -752,6 +754,13 @@ public static function uri(?string $uri = null, bool $getShared = true) return static::getSharedInstance('uri', $uri); } + if ($uri === null) { + $appConfig = config(App::class); + $factory = new SiteURIFactory($appConfig, new Superglobals()); + + return $factory->createFromGlobals(); + } + return new URI($uri); } diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index 7de4c37b4f15..4f0f1cb0d85d 100755 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -175,7 +175,12 @@ public function __construct($config, ?URI $uri = null, $body = 'php://input', ?U parent::__construct($config); - $this->detectURI($config->uriProtocol, $config->baseURL); + if ($uri instanceof SiteURI) { + $this->setPath($uri->getRoutePath()); + } else { + $this->setPath($uri->getPath()); + } + $this->detectLocale($config); } @@ -227,9 +232,9 @@ public function detectLocale($config) * either provided by the user in the baseURL Config setting, or * determined from the environment as needed. * - * @deprecated $protocol and $baseURL are deprecated. No longer used. - * * @return void + * + * @deprecated No longer used. */ protected function detectURI(string $protocol, string $baseURL) { @@ -447,7 +452,7 @@ public function isSecure(): bool } /** - * Sets the relative path and updates the URI object. + * Sets the URI path relative to baseURL. * * Note: Since current_url() accesses the shared request * instance, this can be used to change the "current URL" @@ -457,51 +462,13 @@ public function isSecure(): bool * @param App|null $config Optional alternate config to use * * @return $this + * + * @deprecated This method will be private. The parameter $config is deprecated. No longer used. */ 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; } @@ -535,10 +502,6 @@ private function determineHost(App $config, string $baseURL): string */ public function getPath(): string { - if ($this->path === null) { - $this->detectPath($this->config->uriProtocol); - } - return $this->path; } diff --git a/system/HTTP/URI.php b/system/HTTP/URI.php index 3eef2371943a..429aa73a7b7f 100644 --- a/system/HTTP/URI.php +++ b/system/HTTP/URI.php @@ -94,11 +94,6 @@ class URI /** * URI path. * - * Note: The constructor of the IncomingRequest class changes the path of - * the URI object held by the IncomingRequest class to a path relative - * to the baseURL. If the baseURL contains subfolders, this value - * will be different from the current URI path. - * * @var string */ protected $path; diff --git a/system/Test/FeatureTestTrait.php b/system/Test/FeatureTestTrait.php index bde29f1b03f5..ded49bd35011 100644 --- a/system/Test/FeatureTestTrait.php +++ b/system/Test/FeatureTestTrait.php @@ -15,6 +15,7 @@ use CodeIgniter\HTTP\Exceptions\RedirectException; use CodeIgniter\HTTP\IncomingRequest; use CodeIgniter\HTTP\Request; +use CodeIgniter\HTTP\SiteURI; use CodeIgniter\HTTP\URI; use Config\App; use Config\Services; @@ -265,15 +266,23 @@ public function options(string $path, ?array $params = null) */ protected function setupRequest(string $method, ?string $path = null): IncomingRequest { - $path = URI::removeDotSegments($path); - $config = config(App::class); - $request = Services::request($config, false); + $config = config(App::class); + $uri = new SiteURI($config); // $path may have a query in it - $parts = explode('?', $path); - $_SERVER['QUERY_STRING'] = $parts[1] ?? ''; + $path = URI::removeDotSegments($path); + $parts = explode('?', $path); + $path = $parts[0]; + $query = $parts[1] ?? ''; + + $_SERVER['QUERY_STRING'] = $query; + + $uri->setPath($path); + + Services::injectMock('uri', $uri); + + $request = Services::request($config, false); - $request->setPath($parts[0]); $request->setMethod($method); $request->setProtocolVersion('1.1'); diff --git a/tests/_support/Config/Services.php b/tests/_support/Config/Services.php index 4a942a053433..9a553d0116a0 100644 --- a/tests/_support/Config/Services.php +++ b/tests/_support/Config/Services.php @@ -11,7 +11,10 @@ namespace Tests\Support\Config; +use CodeIgniter\HTTP\SiteURIFactory; use CodeIgniter\HTTP\URI; +use CodeIgniter\Superglobals; +use Config\App; use Config\Services as BaseServices; use RuntimeException; @@ -41,6 +44,13 @@ public static function uri(?string $uri = null, bool $getShared = true) return static::getSharedInstance('uri', $uri); } + if ($uri === null) { + $appConfig = config(App::class); + $factory = new SiteURIFactory($appConfig, new Superglobals()); + + return $factory->createFromGlobals(); + } + return new URI($uri); } } diff --git a/tests/system/HTTP/IncomingRequestTest.php b/tests/system/HTTP/IncomingRequestTest.php index 0065ab430fc7..3855a4b85c94 100644 --- a/tests/system/HTTP/IncomingRequestTest.php +++ b/tests/system/HTTP/IncomingRequestTest.php @@ -35,11 +35,22 @@ protected function setUp(): void { parent::setUp(); - $this->request = new IncomingRequest(new App(), new URI(), null, new UserAgent()); + $config = new App(); + $this->request = $this->createRequest($config); $_POST = $_GET = $_SERVER = $_REQUEST = $_ENV = $_COOKIE = $_SESSION = []; } + private function createRequest(?App $config = null, $body = null, ?string $path = null): IncomingRequest + { + $config ??= new App(); + $path ??= ''; + + $uri = new SiteURI($config, $path); + + return new IncomingRequest($config, $uri, $body, new UserAgent()); + } + public function testCanGrabRequestVars(): void { $_REQUEST['TEST'] = 5; @@ -186,7 +197,7 @@ public function testSetLocaleSaves(): void $config->defaultLocale = 'es'; $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), null, new UserAgent()); + $request = $this->createRequest($config); $request->setLocale('en'); $this->assertSame('en', $request->getLocale()); @@ -199,7 +210,7 @@ public function testSetBadLocale(): void $config->defaultLocale = 'es'; $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), null, new UserAgent()); + $request = $this->createRequest($config); $request->setLocale('xx'); $this->assertSame('es', $request->getLocale()); @@ -232,7 +243,7 @@ public function testNegotiatesLocale(): void $config->supportedLocales = ['fr', 'en']; $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), null, new UserAgent()); + $request = $this->createRequest($config); $this->assertSame($config->defaultLocale, $request->getDefaultLocale()); $this->assertSame('fr', $request->getLocale()); @@ -247,7 +258,7 @@ public function testNegotiatesLocaleOnlyBroad(): void $config->supportedLocales = ['fr', 'en']; $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), null, new UserAgent()); + $request = $this->createRequest($config); $this->assertSame($config->defaultLocale, $request->getDefaultLocale()); $this->assertSame('fr', $request->getLocale()); @@ -306,7 +317,7 @@ public function testCanGrabGetRawJSON(): void $config = new App(); $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), $json, new UserAgent()); + $request = $this->createRequest($config, $json); $this->assertSame($expected, $request->getJSON(true)); } @@ -327,7 +338,7 @@ public function testCanGetAVariableFromJson(): void $config = new App(); $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), $json, new UserAgent()); + $request = $this->createRequest($config, $json); $this->assertSame('bar', $request->getJsonVar('foo')); $this->assertNull($request->getJsonVar('notExists')); @@ -361,7 +372,7 @@ public function testGetJsonVarAsArray(): void $config = new App(); $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), $json, new UserAgent()); + $request = $this->createRequest($config, $json); $jsonVar = $request->getJsonVar('baz', true); $this->assertIsArray($jsonVar); @@ -381,7 +392,7 @@ public function testGetJsonVarCanFilter(): void $config = new App(); $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), $json, new UserAgent()); + $request = $this->createRequest($config, $json); $this->assertFalse($request->getJsonVar('foo', false, FILTER_VALIDATE_INT)); } @@ -402,7 +413,7 @@ public function testGetJsonVarCanFilterArray(): void $config = new App(); $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), $json, new UserAgent()); + $request = $this->createRequest($config, $json); $request->setHeader('Content-Type', 'application/json'); $expected = [ @@ -446,7 +457,7 @@ public function testGetVarWorksWithJson(): void $config = new App(); $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), $json, new UserAgent()); + $request = $this->createRequest($config, $json); $request->setHeader('Content-Type', 'application/json'); $this->assertSame('bar', $request->getVar('foo')); @@ -473,12 +484,13 @@ public function testGetVarWorksWithJsonAndGetParams(): void $_REQUEST['foo'] = 'bar'; $_REQUEST['fizz'] = 'buzz'; - $request = new IncomingRequest($config, new URI('http://example.com/path?foo=bar&fizz=buzz'), 'php://input', new UserAgent()); + $request = $this->createRequest($config, null); $request = $request->withMethod('GET'); // JSON type $request->setHeader('Content-Type', 'application/json'); + // The body is null, so this works. $this->assertSame('bar', $request->getVar('foo')); $this->assertSame('buzz', $request->getVar('fizz')); @@ -501,7 +513,7 @@ public function testGetJsonVarReturnsNullFromNullBody(): void $config = new App(); $config->baseURL = 'http://example.com/'; $json = null; - $request = new IncomingRequest($config, new URI(), $json, new UserAgent()); + $request = $this->createRequest($config, $json); $this->assertNull($request->getJsonVar('myKey')); } @@ -511,7 +523,7 @@ public function testgetJSONReturnsNullFromNullBody(): void $config = new App(); $config->baseURL = 'http://example.com/'; $json = null; - $request = new IncomingRequest($config, new URI(), $json, new UserAgent()); + $request = $this->createRequest($config, $json); $this->assertNull($request->getJSON()); } @@ -529,7 +541,7 @@ public function testCanGrabGetRawInput(): void $config = new App(); $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), $rawstring, new UserAgent()); + $request = $this->createRequest($config, $rawstring); $this->assertSame($expected, $request->getRawInput()); } @@ -627,7 +639,7 @@ public function testCanGrabGetRawInputVar($rawstring, $var, $expected, $filter, $config = new App(); $config->baseURL = 'http://example.com/'; - $request = new IncomingRequest($config, new URI(), $rawstring, new UserAgent()); + $request = $this->createRequest($config, $rawstring); $this->assertSame($expected, $request->getRawInputVar($var, $filter, $flag)); } @@ -723,7 +735,7 @@ public function testUserAgent(): void $_SERVER['HTTP_USER_AGENT'] = 'Mozilla'; $config = new App(); - $request = new IncomingRequest($config, new URI(), null, new UserAgent()); + $request = $this->createRequest($config); $this->assertSame('Mozilla', $request->getUserAgent()->__toString()); } @@ -838,7 +850,7 @@ public function testGetPostSecondStreams(): void public function testWithFalseBody(): void { // Use `false` here to simulate file_get_contents returning a false value - $request = new IncomingRequest(new App(), new URI(), false, new UserAgent()); + $request = $this->createRequest(null, false); $this->assertNotFalse($request->getBody()); $this->assertNull($request->getBody()); @@ -888,49 +900,11 @@ public function testExtensionPHP($path, $detectPath): void public function testGetPath(): void { - $_SERVER['REQUEST_URI'] = '/index.php/fruits/banana'; - $_SERVER['SCRIPT_NAME'] = '/index.php'; - - $request = new IncomingRequest(new App(), new URI(), null, new UserAgent()); - - $this->assertSame('fruits/banana', $request->getPath()); - } - - public function testGetPathIsRelative(): void - { - $_SERVER['REQUEST_URI'] = '/sub/folder/index.php/fruits/banana'; - $_SERVER['SCRIPT_NAME'] = '/sub/folder/index.php'; - - $request = new IncomingRequest(new App(), new URI(), null, new UserAgent()); - - $this->assertSame('fruits/banana', $request->getPath()); - } - - public function testGetPathStoresDetectedValue(): void - { - $_SERVER['REQUEST_URI'] = '/fruits/banana'; - $_SERVER['SCRIPT_NAME'] = '/index.php'; - - $request = new IncomingRequest(new App(), new URI(), null, new UserAgent()); - - $_SERVER['REQUEST_URI'] = '/candy/snickers'; + $request = $this->createRequest(null, null, 'fruits/banana'); $this->assertSame('fruits/banana', $request->getPath()); } - public function testGetPathIsRediscovered(): void - { - $_SERVER['REQUEST_URI'] = '/fruits/banana'; - $_SERVER['SCRIPT_NAME'] = '/index.php'; - - $request = new IncomingRequest(new App(), new URI(), null, new UserAgent()); - - $_SERVER['REQUEST_URI'] = '/candy/snickers'; - $request->detectPath(); - - $this->assertSame('candy/snickers', $request->getPath()); - } - public function testSetPath(): void { $request = new IncomingRequest(new App(), new URI(), null, new UserAgent()); @@ -940,15 +914,6 @@ public function testSetPath(): void $this->assertSame('foobar', $request->getPath()); } - public function testSetPathUpdatesURI(): void - { - $request = new IncomingRequest(new App(), new URI(), null, new UserAgent()); - - $request->setPath('apples'); - - $this->assertSame('apples', $request->getUri()->getPath()); - } - public function testGetIPAddressNormal(): void { $expected = '123.123.123.123'; diff --git a/tests/system/HTTP/NegotiateTest.php b/tests/system/HTTP/NegotiateTest.php index 4323e80c2b42..0784bd562b54 100644 --- a/tests/system/HTTP/NegotiateTest.php +++ b/tests/system/HTTP/NegotiateTest.php @@ -29,7 +29,8 @@ protected function setUp(): void { parent::setUp(); - $this->request = new IncomingRequest(new App(), new URI(), null, new UserAgent()); + $config = new App(); + $this->request = new IncomingRequest($config, new SiteURI($config), null, new UserAgent()); $this->negotiate = new Negotiate($this->request); } diff --git a/tests/system/HTTP/RedirectResponseTest.php b/tests/system/HTTP/RedirectResponseTest.php index b6d74c442421..a15117e9324a 100644 --- a/tests/system/HTTP/RedirectResponseTest.php +++ b/tests/system/HTTP/RedirectResponseTest.php @@ -53,7 +53,7 @@ protected function setUp(): void $this->request = new MockIncomingRequest( $this->config, - new URI('http://example.com'), + new SiteURI($this->config), null, new UserAgent() ); @@ -186,7 +186,8 @@ public function testWith(): void public function testRedirectBack(): void { $_SERVER['HTTP_REFERER'] = 'http://somewhere.com'; - $this->request = new MockIncomingRequest($this->config, new URI('http://somewhere.com'), null, new UserAgent()); + + $this->request = new MockIncomingRequest($this->config, new SiteURI($this->config), null, new UserAgent()); Services::injectMock('request', $this->request); $response = new RedirectResponse(new App()); @@ -222,7 +223,7 @@ public function testRedirectRouteBaseUrl(): void $config->baseURL = 'http://example.com/test/'; Factories::injectMock('config', 'App', $config); - $request = new MockIncomingRequest($config, new URI('http://example.com/test/'), null, new UserAgent()); + $request = new MockIncomingRequest($config, new SiteURI($config), null, new UserAgent()); Services::injectMock('request', $request); $response = new RedirectResponse(new App()); diff --git a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php index b207d1134dc4..1e96bf065b92 100644 --- a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php +++ b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php @@ -157,6 +157,54 @@ public function testRequestURISuppressed() $this->assertSame($expected, $factory->detectRoutePath('REQUEST_URI')); } + public function testRequestURIGetPath() + { + // /index.php/fruits/banana + $_SERVER['REQUEST_URI'] = '/index.php/fruits/banana'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $this->assertSame('fruits/banana', $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURIPathIsRelative() + { + // /sub/folder/index.php/fruits/banana + $_SERVER['REQUEST_URI'] = '/sub/folder/index.php/fruits/banana'; + $_SERVER['SCRIPT_NAME'] = '/sub/folder/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $this->assertSame('fruits/banana', $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURIStoresDetectedPath() + { + // /fruits/banana + $_SERVER['REQUEST_URI'] = '/fruits/banana'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $_SERVER['REQUEST_URI'] = '/candy/snickers'; + + $this->assertSame('fruits/banana', $factory->detectRoutePath('REQUEST_URI')); + } + + public function testRequestURIPathIsNeverRediscovered() + { + $_SERVER['REQUEST_URI'] = '/fruits/banana'; + $_SERVER['SCRIPT_NAME'] = '/index.php'; + + $factory = $this->createSiteURIFactory($_SERVER); + + $_SERVER['REQUEST_URI'] = '/candy/snickers'; + $factory->detectRoutePath('REQUEST_URI'); + + $this->assertSame('fruits/banana', $factory->detectRoutePath('REQUEST_URI')); + } + public function testQueryString() { // /index.php?/ci/woot @@ -228,4 +276,37 @@ public function testPathInfoSubfolder() $expected = 'woot'; $this->assertSame($expected, $factory->detectRoutePath('PATH_INFO')); } + + /** + * @dataProvider providePathChecks + * + * @param string $path + * @param string $detectPath + */ + public function testExtensionPHP($path, $detectPath) + { + $config = new App(); + $config->baseURL = 'http://example.com/'; + + $_SERVER['REQUEST_URI'] = $path; + $_SERVER['SCRIPT_NAME'] = $path; + + $factory = $this->createSiteURIFactory($_SERVER, $config); + + $this->assertSame($detectPath, $factory->detectRoutePath()); + } + + public function providePathChecks(): iterable + { + return [ + 'not /index.php' => [ + '/test.php', + '/', + ], + '/index.php' => [ + '/index.php', + '/', + ], + ]; + } } diff --git a/tests/system/HTTP/URITest.php b/tests/system/HTTP/URITest.php index 7d753c1bedac..1e1fbc4d1a87 100644 --- a/tests/system/HTTP/URITest.php +++ b/tests/system/HTTP/URITest.php @@ -1028,8 +1028,11 @@ public function testSetBadSegmentSilent(): void public function testBasedNoIndex(): void { - $_SERVER['HTTP_HOST'] = 'example.com'; - $_SERVER['REQUEST_URI'] = '/ci/v4/controller/method'; + $_SERVER['REQUEST_URI'] = '/ci/v4/controller/method'; + $_SERVER['SCRIPT_NAME'] = '/ci/v4/index.php'; + $_SERVER['QUERY_STRING'] = ''; + $_SERVER['HTTP_HOST'] = 'example.com'; + $_SERVER['PATH_INFO'] = '/controller/method'; $this->resetServices(); @@ -1046,20 +1049,24 @@ public function testBasedNoIndex(): void 'http://example.com/ci/v4/controller/method', (string) $request->getUri() ); - $this->assertSame('ci/v4/controller/method', $request->getUri()->getPath()); + $this->assertSame('/ci/v4/controller/method', $request->getUri()->getPath()); + $this->assertSame('controller/method', $request->getUri()->getRoutePath()); // standalone $uri = new URI('http://example.com/ci/v4/controller/method'); $this->assertSame('http://example.com/ci/v4/controller/method', (string) $uri); $this->assertSame('/ci/v4/controller/method', $uri->getPath()); - $this->assertSame($uri->getPath(), '/' . $request->getUri()->getPath()); + $this->assertSame($uri->getPath(), $request->getUri()->getPath()); } public function testBasedWithIndex(): void { - $_SERVER['HTTP_HOST'] = 'example.com'; - $_SERVER['REQUEST_URI'] = '/ci/v4/index.php/controller/method'; + $_SERVER['REQUEST_URI'] = '/ci/v4/index.php/controller/method'; + $_SERVER['SCRIPT_NAME'] = '/ci/v4/index.php'; + $_SERVER['QUERY_STRING'] = ''; + $_SERVER['HTTP_HOST'] = 'example.com'; + $_SERVER['PATH_INFO'] = '/controller/method'; $this->resetServices(); @@ -1077,7 +1084,7 @@ public function testBasedWithIndex(): void (string) $request->getUri() ); $this->assertSame( - 'ci/v4/index.php/controller/method', + '/ci/v4/index.php/controller/method', $request->getUri()->getPath() ); @@ -1089,26 +1096,26 @@ public function testBasedWithIndex(): void ); $this->assertSame('/ci/v4/index.php/controller/method', $uri->getPath()); - $this->assertSame($uri->getPath(), '/' . $request->getUri()->getPath()); + $this->assertSame($uri->getPath(), $request->getUri()->getPath()); } public function testForceGlobalSecureRequests(): void { $this->resetServices(); - $_SERVER['HTTP_HOST'] = 'example.com'; - $_SERVER['REQUEST_URI'] = '/ci/v4/controller/method'; + $_SERVER['REQUEST_URI'] = '/ci/v4/controller/method'; + $_SERVER['SCRIPT_NAME'] = '/ci/v4/index.php'; + $_SERVER['QUERY_STRING'] = ''; + $_SERVER['HTTP_HOST'] = 'example.com'; + $_SERVER['PATH_INFO'] = '/controller/method'; $config = new App(); $config->baseURL = 'http://example.com/ci/v4'; - $config->indexPage = 'index.php'; + $config->indexPage = ''; $config->forceGlobalSecureRequests = true; - Factories::injectMock('config', 'App', $config); - $uri = new URI('http://example.com/ci/v4/controller/method'); - $request = new IncomingRequest($config, $uri, 'php://input', new UserAgent()); - + $request = Services::request($config); Services::injectMock('request', $request); // Detected by request From fc0a1b8f996360e98eab7a6d97cc2584418a75dd Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 20 Feb 2023 17:57:23 +0900 Subject: [PATCH 02/34] refactor: URL helper site_url(), base_url(), current_url(), uri_string() --- system/HTTP/SiteURI.php | 83 ++++++++++ system/Helpers/url_helper.php | 145 ++---------------- .../Helpers/URLHelper/CurrentUrlTest.php | 82 +++++----- .../system/Helpers/URLHelper/MiscUrlTest.php | 39 ++--- .../system/Helpers/URLHelper/SiteUrlTest.php | 58 +++++-- 5 files changed, 209 insertions(+), 198 deletions(-) diff --git a/system/HTTP/SiteURI.php b/system/HTTP/SiteURI.php index adf873bfb902..72196e1bc365 100644 --- a/system/HTTP/SiteURI.php +++ b/system/HTTP/SiteURI.php @@ -363,4 +363,87 @@ protected function applyParts(array $parts): void $this->password = $parts['pass']; } } + + /** + * For baser_url() helper. + * + * @param array|string $relativePath URI string or array of URI segments + * @param string|null $scheme URI scheme. E.g., http, ftp + */ + public function baseUrl($relativePath = '', ?string $scheme = null): string + { + $relativePath = $this->stringifyRelativePath($relativePath); + + $config = clone config(App::class); + $config->indexPage = ''; + + $host = $this->getHost(); + + $uri = new self($config, $relativePath, $host, $scheme); + + return URI::createURIString( + $uri->getScheme(), + $uri->getAuthority(), + $this->adjustPathTrailingSlash($uri, $relativePath), + $uri->getQuery(), + $uri->getFragment() + ); + } + + /** + * @param array|string $relativePath URI string or array of URI segments + */ + private function stringifyRelativePath($relativePath): string + { + if (is_array($relativePath)) { + $relativePath = implode('/', $relativePath); + } + + return $relativePath; + } + + /** + * For site_url() helper. + * + * @param array|string $relativePath URI string or array of URI segments + * @param string|null $scheme URI scheme. E.g., http, ftp + * @param App|null $config Alternate configuration to use + */ + public function siteUrl($relativePath = '', ?string $scheme = null, ?App $config = null): string + { + $relativePath = $this->stringifyRelativePath($relativePath); + + // Check current host. + $host = $config === null ? $this->getHost() : null; + + $config ??= config(App::class); + + $uri = new self($config, $relativePath, $host, $scheme); + + // Adjust path + $path = $this->adjustPathTrailingSlash($uri, $relativePath); + if ($config->indexPage !== '' && $relativePath === '') { + $path = rtrim($path, '/'); + } + + return URI::createURIString( + $uri->getScheme(), + $uri->getAuthority(), + $path, + $uri->getQuery(), + $uri->getFragment() + ); + } + + private function adjustPathTrailingSlash(self $uri, string $relativePath): string + { + $parts = parse_url($this->getBaseURL() . $relativePath); + $path = $parts['path'] ?? ''; + + if (substr($path, -1) === '/' && substr($uri->getPath(), -1) !== '/') { + return $uri->getPath() . '/'; + } + + return $uri->getPath(); + } } diff --git a/system/Helpers/url_helper.php b/system/Helpers/url_helper.php index c6c810b66da6..b8c1a687e6bb 100644 --- a/system/Helpers/url_helper.php +++ b/system/Helpers/url_helper.php @@ -10,8 +10,8 @@ */ use CodeIgniter\HTTP\CLIRequest; -use CodeIgniter\HTTP\Exceptions\HTTPException; use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\HTTP\SiteURI; use CodeIgniter\HTTP\URI; use CodeIgniter\Router\Exceptions\RouterException; use Config\App; @@ -19,92 +19,6 @@ // CodeIgniter URL Helpers -if (! function_exists('_get_uri')) { - /** - * Used by the other URL functions to build a framework-specific URI - * based on $request->getUri()->getBaseURL() and the App config. - * - * @internal Outside the framework this should not be used directly. - * - * @param array|string $relativePath URI string or array of URI segments. - * May include queries or fragments. - * @param App|null $config Alternative Config to use - * - * @throws HTTPException For invalid paths. - * @throws InvalidArgumentException For invalid config. - */ - function _get_uri($relativePath = '', ?App $config = null): URI - { - $appConfig = null; - if ($config === null) { - /** @var App $appConfig */ - $appConfig = config(App::class); - - if ($appConfig->baseURL === '') { - throw new InvalidArgumentException( - '_get_uri() requires a valid baseURL.' - ); - } - } elseif ($config->baseURL === '') { - throw new InvalidArgumentException( - '_get_uri() requires a valid baseURL.' - ); - } - - // Convert array of segments to a string - if (is_array($relativePath)) { - $relativePath = implode('/', $relativePath); - } - - // If a full URI was passed then convert it - if (strpos($relativePath, '://') !== false) { - $full = new URI($relativePath); - $relativePath = URI::createURIString( - null, - null, - $full->getPath(), - $full->getQuery(), - $full->getFragment() - ); - } - - $relativePath = URI::removeDotSegments($relativePath); - - $request = Services::request(); - - if ($config === null) { - $baseURL = $request instanceof CLIRequest - ? rtrim($appConfig->baseURL, '/ ') . '/' - // Use the current baseURL for multiple domain support - : $request->getUri()->getBaseURL(); - - $config = $appConfig; - } else { - $baseURL = rtrim($config->baseURL, '/ ') . '/'; - } - - // Check for an index page - $indexPage = ''; - if ($config->indexPage !== '') { - $indexPage = $config->indexPage; - - // Check if we need a separator - if ($relativePath !== '' && $relativePath[0] !== '/' && $relativePath[0] !== '?') { - $indexPage .= '/'; - } - } - - $uri = new URI($baseURL . $indexPage . $relativePath); - - // Check if the baseURL scheme needs to be coerced into its secure version - if ($config->forceGlobalSecureRequests && $uri->getScheme() === 'http') { - $uri->setScheme('https'); - } - - return $uri; - } -} - if (! function_exists('site_url')) { /** * Returns a site URL as defined by the App config. @@ -115,22 +29,12 @@ function _get_uri($relativePath = '', ?App $config = null): URI */ function site_url($relativePath = '', ?string $scheme = null, ?App $config = null): string { - $uri = _get_uri($relativePath, $config); - - $uriString = URI::createURIString( - $scheme ?? $uri->getScheme(), - $uri->getAuthority(), - $uri->getPath(), - $uri->getQuery(), - $uri->getFragment() - ); - - // For protocol-relative links - if ($scheme === '') { - $uriString = '//' . $uriString; - } + $currentURI = Services::request()->getUri(); + + assert($currentURI instanceof SiteURI); - return $uriString; + // @TODO supprot protocol-relative links + return $currentURI->siteUrl($relativePath, $scheme, $config); } } @@ -144,18 +48,11 @@ function site_url($relativePath = '', ?string $scheme = null, ?App $config = nul */ function base_url($relativePath = '', ?string $scheme = null): string { - /** @var App $config */ - $config = clone config(App::class); - - // Use the current baseURL for multiple domain support - $request = Services::request(); - $config->baseURL = $request instanceof CLIRequest - ? rtrim($config->baseURL, '/ ') . '/' - : $request->getUri()->getBaseURL(); + $currentURI = Services::request()->getUri(); - $config->indexPage = ''; + assert($currentURI instanceof SiteURI); - return site_url($relativePath, $scheme, $config); + return $currentURI->baseUrl($relativePath, $scheme); } } @@ -173,18 +70,7 @@ function current_url(bool $returnObject = false, ?IncomingRequest $request = nul { $request ??= Services::request(); /** @var CLIRequest|IncomingRequest $request */ - $routePath = $request->getPath(); - $currentURI = $request->getUri(); - - // Append queries and fragments - if ($query = $currentURI->getQuery()) { - $query = '?' . $query; - } - if ($fragment = $currentURI->getFragment()) { - $fragment = '#' . $fragment; - } - - $uri = _get_uri($routePath . $query . $fragment); + $uri = $request->getUri(); return $returnObject ? $uri : URI::createURIString($uri->getScheme(), $uri->getAuthority(), $uri->getPath()); } @@ -220,10 +106,13 @@ function previous_url(bool $returnObject = false) */ function uri_string(): string { - // The value of Services::request()->getUri()->getPath() is overridden - // by IncomingRequest constructor. If we use it here, the current tests - // in CurrentUrlTest will fail. - return ltrim(Services::request()->getPath(), '/'); + // The value of Services::request()->getUri()->getPath() returns + // full URI path. + $uri = Services::request()->getUri(); + + $path = $uri instanceof SiteURI ? $uri->getRoutePath() : $uri->getPath(); + + return ltrim($path, '/'); } } diff --git a/tests/system/Helpers/URLHelper/CurrentUrlTest.php b/tests/system/Helpers/URLHelper/CurrentUrlTest.php index 6f908053b6a6..fd5f7dda9ea7 100644 --- a/tests/system/Helpers/URLHelper/CurrentUrlTest.php +++ b/tests/system/Helpers/URLHelper/CurrentUrlTest.php @@ -13,7 +13,11 @@ use CodeIgniter\Config\Factories; use CodeIgniter\Config\Services; +use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\HTTP\SiteURIFactory; use CodeIgniter\HTTP\URI; +use CodeIgniter\HTTP\UserAgent; +use CodeIgniter\Superglobals; use CodeIgniter\Test\CIUnitTestCase; use Config\App; @@ -42,7 +46,6 @@ protected function setUp(): void $this->config = new App(); $this->config->baseURL = 'http://example.com/'; $this->config->indexPage = 'index.php'; - Factories::injectMock('config', 'App', $this->config); $_SERVER['HTTP_HOST'] = 'example.com'; $_SERVER['REQUEST_URI'] = '/'; @@ -63,9 +66,7 @@ public function testCurrentURLReturnsBasicURL(): void $this->config->baseURL = 'http://example.com/public'; - // URI object are updated in IncomingRequest constructor. - $request = Services::incomingrequest($this->config); - Services::injectMock('request', $request); + $this->createRequest($this->config); $this->assertSame('http://example.com/public/index.php/', current_url()); } @@ -79,9 +80,28 @@ public function testCurrentURLReturnsAllowedHostname(): void $this->config->baseURL = 'http://example.com/public'; $this->config->allowedHostnames = ['www.example.jp']; + $this->createRequest($this->config); + $this->assertSame('http://www.example.jp/public/index.php/', current_url()); } + private function createRequest(?App $config = null, $body = null, ?string $path = null): void + { + $config ??= new App(); + + $factory = new SiteURIFactory($config, new Superglobals()); + $uri = $factory->createFromGlobals(); + + if ($path !== null) { + $uri->setPath($path); + } + + $request = new IncomingRequest($config, $uri, $body, new UserAgent()); + Services::injectMock('request', $request); + + Factories::injectMock('config', 'App', $config); + } + public function testCurrentURLReturnsBaseURLIfNotAllowedHostname(): void { $_SERVER['HTTP_HOST'] = 'invalid.example.org'; @@ -91,6 +111,8 @@ public function testCurrentURLReturnsBaseURLIfNotAllowedHostname(): void $this->config->baseURL = 'http://example.com/public'; $this->config->allowedHostnames = ['www.example.jp']; + $this->createRequest($this->config); + $this->assertSame('http://example.com/public/index.php/', current_url()); } @@ -99,6 +121,8 @@ public function testCurrentURLReturnsObject(): void // Since we're on a CLI, we must provide our own URI $this->config->baseURL = 'http://example.com/public'; + $this->createRequest($this->config); + $url = current_url(true); $this->assertInstanceOf(URI::class, $url); @@ -111,11 +135,9 @@ public function testCurrentURLEquivalence(): void $_SERVER['REQUEST_URI'] = '/public'; $_SERVER['SCRIPT_NAME'] = '/index.php'; - // Since we're on a CLI, we must provide our own URI - Factories::injectMock('config', 'App', $this->config); + $this->config->indexPage = ''; - $request = Services::request($this->config); - Services::injectMock('request', $request); + $this->createRequest($this->config); $this->assertSame(site_url(uri_string()), current_url()); } @@ -128,16 +150,14 @@ public function testCurrentURLInSubfolder(): void // Since we're on a CLI, we must provide our own URI $this->config->baseURL = 'http://example.com/foo/public'; - Factories::injectMock('config', 'App', $this->config); - $request = Services::request($this->config); - Services::injectMock('request', $request); + $this->createRequest($this->config); $this->assertSame('http://example.com/foo/public/index.php/bar', current_url()); $this->assertSame('http://example.com/foo/public/index.php/bar?baz=quip', (string) current_url(true)); $uri = current_url(true); - $this->assertSame('foo', $uri->getSegment(1)); + $this->assertSame('bar', $uri->getSegment(1)); $this->assertSame('example.com', $uri->getHost()); $this->assertSame('http', $uri->getScheme()); } @@ -149,19 +169,16 @@ public function testCurrentURLWithPortInSubfolder(): void $_SERVER['REQUEST_URI'] = '/foo/public/bar?baz=quip'; $_SERVER['SCRIPT_NAME'] = '/foo/public/index.php'; - // Since we're on a CLI, we must provide our own URI $this->config->baseURL = 'http://example.com:8080/foo/public'; - Factories::injectMock('config', 'App', $this->config); - $request = Services::request($this->config); - Services::injectMock('request', $request); + $this->createRequest($this->config); $this->assertSame('http://example.com:8080/foo/public/index.php/bar', current_url()); $this->assertSame('http://example.com:8080/foo/public/index.php/bar?baz=quip', (string) current_url(true)); $uri = current_url(true); - $this->assertSame(['foo', 'public', 'index.php', 'bar'], $uri->getSegments()); - $this->assertSame('foo', $uri->getSegment(1)); + $this->assertSame(['bar'], $uri->getSegments()); + $this->assertSame('bar', $uri->getSegment(1)); $this->assertSame('example.com', $uri->getHost()); $this->assertSame('http', $uri->getScheme()); $this->assertSame(8080, $uri->getPort()); @@ -172,8 +189,9 @@ public function testUriString(): void $_SERVER['HTTP_HOST'] = 'example.com'; $_SERVER['REQUEST_URI'] = '/assets/image.jpg'; - $uri = 'http://example.com/assets/image.jpg'; - $this->setService($uri); + $this->config->indexPage = ''; + + $this->createRequest($this->config); $this->assertSame('assets/image.jpg', uri_string()); } @@ -192,18 +210,17 @@ public function testUriStringNoTrailingSlash(): void $_SERVER['HTTP_HOST'] = 'example.com'; $_SERVER['REQUEST_URI'] = '/assets/image.jpg'; - $this->config->baseURL = 'http://example.com'; + $this->config->baseURL = 'http://example.com'; + $this->config->indexPage = ''; - $uri = 'http://example.com/assets/image.jpg'; - $this->setService($uri); + $this->createRequest($this->config); $this->assertSame('assets/image.jpg', uri_string()); } public function testUriStringEmpty(): void { - $uri = 'http://example.com/'; - $this->setService($uri); + $this->createRequest($this->config); $this->assertSame('', uri_string()); } @@ -215,8 +232,7 @@ public function testUriStringSubfolderAbsolute(): void $this->config->baseURL = 'http://example.com/subfolder/'; - $uri = 'http://example.com/subfolder/assets/image.jpg'; - $this->setService($uri); + $this->createRequest($this->config); $this->assertSame('subfolder/assets/image.jpg', uri_string()); } @@ -229,8 +245,7 @@ public function testUriStringSubfolderRelative(): void $this->config->baseURL = 'http://example.com/subfolder/'; - $uri = 'http://example.com/subfolder/assets/image.jpg'; - $this->setService($uri); + $this->createRequest($this->config); $this->assertSame('assets/image.jpg', uri_string()); } @@ -284,8 +299,7 @@ public function testUrlIs(string $currentPath, string $testPath, bool $expected) $_SERVER['HTTP_HOST'] = 'example.com'; $_SERVER['REQUEST_URI'] = '/' . $currentPath; - $uri = 'http://example.com/' . $currentPath; - $this->setService($uri); + $this->createRequest($this->config); $this->assertSame($expected, url_is($testPath)); } @@ -300,8 +314,7 @@ public function testUrlIsNoIndex(string $currentPath, string $testPath, bool $ex $this->config->indexPage = ''; - $uri = 'http://example.com/' . $currentPath; - $this->setService($uri); + $this->createRequest($this->config); $this->assertSame($expected, url_is($testPath)); } @@ -317,8 +330,7 @@ public function testUrlIsWithSubfolder(string $currentPath, string $testPath, bo $this->config->baseURL = 'http://example.com/subfolder/'; - $uri = 'http://example.com/subfolder/' . $currentPath; - $this->setService($uri); + $this->createRequest($this->config); $this->assertSame($expected, url_is($testPath)); } diff --git a/tests/system/Helpers/URLHelper/MiscUrlTest.php b/tests/system/Helpers/URLHelper/MiscUrlTest.php index 7af14d169f16..c26e9e7fbf9b 100644 --- a/tests/system/Helpers/URLHelper/MiscUrlTest.php +++ b/tests/system/Helpers/URLHelper/MiscUrlTest.php @@ -13,7 +13,9 @@ use CodeIgniter\Config\Factories; use CodeIgniter\Config\Services; -use CodeIgniter\HTTP\URI; +use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\HTTP\SiteURIFactory; +use CodeIgniter\HTTP\UserAgent; use CodeIgniter\Router\Exceptions\RouterException; use CodeIgniter\Test\CIUnitTestCase; use Config\App; @@ -40,7 +42,6 @@ protected function setUp(): void $this->config = new App(); $this->config->baseURL = 'http://example.com/'; $this->config->indexPage = 'index.php'; - Factories::injectMock('config', 'App', $this->config); } protected function tearDown(): void @@ -61,19 +62,21 @@ public function testPreviousURLUsesSessionFirst(): void $this->config->baseURL = 'http://example.com/public'; $uri = 'http://example.com/public'; - $this->setRequest($uri); + $this->createRequest($uri); $this->assertSame($uri2, previous_url()); } - private function setRequest(string $uri): void + private function createRequest(string $uri): void { - $uri = new URI($uri); - Services::injectMock('uri', $uri); + $factory = new SiteURIFactory($_SERVER, $this->config); + + $uri = $factory->createFromString($uri); - // Since we're on a CLI, we must provide our own URI - $request = Services::request($this->config); + $request = new IncomingRequest($this->config, $uri, null, new UserAgent()); Services::injectMock('request', $request); + + Factories::injectMock('config', 'App', $this->config); } public function testPreviousURLUsesRefererIfNeeded(): void @@ -85,7 +88,7 @@ public function testPreviousURLUsesRefererIfNeeded(): void $this->config->baseURL = 'http://example.com/public'; $uri = 'http://example.com/public'; - $this->setRequest($uri); + $this->createRequest($uri); $this->assertSame($uri1, previous_url()); } @@ -95,7 +98,7 @@ public function testPreviousURLUsesRefererIfNeeded(): void public function testIndexPage(): void { $uri = 'http://example.com/'; - $this->setRequest($uri); + $this->createRequest($uri); $this->assertSame('index.php', index_page()); } @@ -105,7 +108,7 @@ public function testIndexPageAlt(): void $this->config->indexPage = 'banana.php'; $uri = 'http://example.com/'; - $this->setRequest($uri); + $this->createRequest($uri); $this->assertSame('banana.php', index_page($this->config)); } @@ -166,7 +169,7 @@ public static function provideAnchor(): iterable public function testAnchor($expected = '', $uri = '', $title = '', $attributes = ''): void { $uriString = 'http://example.com/'; - $this->setRequest($uriString); + $this->createRequest($uriString); $this->assertSame($expected, anchor($uri, $title, $attributes, $this->config)); } @@ -233,7 +236,7 @@ public function testAnchorNoindex($expected = '', $uri = '', $title = '', $attri $this->config->indexPage = ''; $uriString = 'http://example.com/'; - $this->setRequest($uriString); + $this->createRequest($uriString); $this->assertSame($expected, anchor($uri, $title, $attributes, $this->config)); } @@ -290,7 +293,7 @@ public function testAnchorTargetted($expected = '', $uri = '', $title = '', $att $this->config->indexPage = ''; $uriString = 'http://example.com/'; - $this->setRequest($uriString); + $this->createRequest($uriString); $this->assertSame($expected, anchor($uri, $title, $attributes, $this->config)); } @@ -334,7 +337,7 @@ public static function provideAnchorExamples(): iterable public function testAnchorExamples($expected = '', $uri = '', $title = '', $attributes = ''): void { $uriString = 'http://example.com/'; - $this->setRequest($uriString); + $this->createRequest($uriString); $this->assertSame($expected, anchor($uri, $title, $attributes, $this->config)); } @@ -392,7 +395,7 @@ public static function provideAnchorPopup(): iterable public function testAnchorPopup($expected = '', $uri = '', $title = '', $attributes = false): void { $uriString = 'http://example.com/'; - $this->setRequest($uriString); + $this->createRequest($uriString); $this->assertSame($expected, anchor_popup($uri, $title, $attributes, $this->config)); } @@ -431,7 +434,7 @@ public static function provideMailto(): iterable public function testMailto($expected = '', $email = '', $title = '', $attributes = ''): void { $uriString = 'http://example.com/'; - $this->setRequest($uriString); + $this->createRequest($uriString); $this->assertSame($expected, mailto($email, $title, $attributes)); } @@ -470,7 +473,7 @@ public static function provideSafeMailto(): iterable public function testSafeMailto($expected = '', $email = '', $title = '', $attributes = ''): void { $uriString = 'http://example.com/'; - $this->setRequest($uriString); + $this->createRequest($uriString); $this->assertSame($expected, safe_mailto($email, $title, $attributes)); } diff --git a/tests/system/Helpers/URLHelper/SiteUrlTest.php b/tests/system/Helpers/URLHelper/SiteUrlTest.php index cafcb2f3ee6a..a28a588cbc5f 100644 --- a/tests/system/Helpers/URLHelper/SiteUrlTest.php +++ b/tests/system/Helpers/URLHelper/SiteUrlTest.php @@ -13,7 +13,10 @@ use CodeIgniter\Config\Factories; use CodeIgniter\Config\Services; +use CodeIgniter\HTTP\IncomingRequest; +use CodeIgniter\HTTP\SiteURIFactory; use CodeIgniter\HTTP\URI; +use CodeIgniter\HTTP\UserAgent; use CodeIgniter\Test\CIUnitTestCase; use Config\App; @@ -39,7 +42,6 @@ protected function setUp(): void Services::reset(true); $this->config = new App(); - Factories::injectMock('config', 'App', $this->config); } protected function tearDown(): void @@ -49,6 +51,23 @@ protected function tearDown(): void $_SERVER = []; } + private function createRequest(?App $config = null, $body = null, ?string $path = null): void + { + $config ??= new App(); + + $factory = new SiteURIFactory($_SERVER, $config); + $uri = $factory->createFromGlobals(); + + if ($path !== null) { + $uri->setPath($path); + } + + $request = new IncomingRequest($config, $uri, $body, new UserAgent()); + Services::injectMock('request', $request); + + Factories::injectMock('config', 'App', $config); + } + /** * Takes a multitude of various config input and verifies * that base_url() and site_url() return the expected result. @@ -77,6 +96,8 @@ public function testUrls( $this->config->indexPage = $indexPage; $this->config->forceGlobalSecureRequests = $secure; + $this->createRequest($this->config); + $this->assertSame($expectedSiteUrl, site_url($path, $scheme, $this->config)); $this->assertSame($expectedBaseUrl, base_url($path, $scheme)); } @@ -333,11 +354,15 @@ public function testBaseURLDiscovery(): void $_SERVER['HTTP_HOST'] = 'example.com'; $_SERVER['REQUEST_URI'] = '/test'; + $this->createRequest($this->config); + $this->assertSame('http://example.com/', base_url()); $_SERVER['HTTP_HOST'] = 'example.com'; $_SERVER['REQUEST_URI'] = '/test/page'; + $this->createRequest($this->config); + $this->assertSame('http://example.com/', base_url()); $this->assertSame('http://example.com/profile', base_url('profile')); } @@ -348,11 +373,17 @@ public function testBaseURLService(): void $_SERVER['REQUEST_URI'] = '/ci/v4/x/y'; $this->config->baseURL = 'http://example.com/ci/v4/'; - $request = Services::request($this->config); - Services::injectMock('request', $request); - $this->assertSame('http://example.com/ci/v4/index.php/controller/method', site_url('controller/method', null, $this->config)); - $this->assertSame('http://example.com/ci/v4/controller/method', base_url('controller/method', null)); + $this->createRequest($this->config); + + $this->assertSame( + 'http://example.com/ci/v4/index.php/controller/method', + site_url('controller/method', null, $this->config) + ); + $this->assertSame( + 'http://example.com/ci/v4/controller/method', + base_url('controller/method', null) + ); } public function testBaseURLWithCLIRequest(): void @@ -360,8 +391,8 @@ public function testBaseURLWithCLIRequest(): void unset($_SERVER['HTTP_HOST'], $_SERVER['REQUEST_URI']); $this->config->baseURL = 'http://example.com/'; - $request = Services::clirequest($this->config); - Services::injectMock('request', $request); + + $this->createRequest($this->config); $this->assertSame( 'http://example.com/index.php/controller/method', @@ -381,11 +412,8 @@ public function testSiteURLWithAllowedHostname(): void $this->config->baseURL = 'http://example.com/public/'; $this->config->allowedHostnames = ['www.example.jp']; - Services::injectMock('config', $this->config); - // URI object are updated in IncomingRequest constructor. - $request = Services::incomingrequest($this->config); - Services::injectMock('request', $request); + $this->createRequest($this->config); $this->assertSame( 'http://www.example.jp/public/index.php/controller/method', @@ -402,9 +430,7 @@ public function testSiteURLWithAltConfig(): void $this->config->baseURL = 'http://example.com/public/'; $this->config->allowedHostnames = ['www.example.jp']; - // URI object are updated in IncomingRequest constructor. - $request = Services::incomingrequest($this->config); - Services::injectMock('request', $request); + $this->createRequest($this->config); $altConfig = clone $this->config; $altConfig->baseURL = 'http://alt.example.com/public/'; @@ -424,9 +450,7 @@ public function testBaseURLWithAllowedHostname(): void $this->config->baseURL = 'http://example.com/public/'; $this->config->allowedHostnames = ['www.example.jp']; - // URI object are updated in IncomingRequest constructor. - $request = Services::incomingrequest($this->config); - Services::injectMock('request', $request); + $this->createRequest($this->config); $this->assertSame( 'http://www.example.jp/public/controller/method', From 468b8b5f26f42aaf673d265d2cc5e31be46e60b1 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 21 Feb 2023 15:17:33 +0900 Subject: [PATCH 03/34] test: fix incorrect REQUEST_URI The public is a folder, so should be end with `/`. --- .../Helpers/URLHelper/CurrentUrlTest.php | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/tests/system/Helpers/URLHelper/CurrentUrlTest.php b/tests/system/Helpers/URLHelper/CurrentUrlTest.php index fd5f7dda9ea7..a28fa78b9b98 100644 --- a/tests/system/Helpers/URLHelper/CurrentUrlTest.php +++ b/tests/system/Helpers/URLHelper/CurrentUrlTest.php @@ -61,10 +61,10 @@ protected function tearDown(): void public function testCurrentURLReturnsBasicURL(): void { - $_SERVER['REQUEST_URI'] = '/public'; + $_SERVER['REQUEST_URI'] = '/public/'; $_SERVER['SCRIPT_NAME'] = '/public/index.php'; - $this->config->baseURL = 'http://example.com/public'; + $this->config->baseURL = 'http://example.com/public/'; $this->createRequest($this->config); @@ -74,10 +74,10 @@ public function testCurrentURLReturnsBasicURL(): void public function testCurrentURLReturnsAllowedHostname(): void { $_SERVER['HTTP_HOST'] = 'www.example.jp'; - $_SERVER['REQUEST_URI'] = '/public'; + $_SERVER['REQUEST_URI'] = '/public/'; $_SERVER['SCRIPT_NAME'] = '/public/index.php'; - $this->config->baseURL = 'http://example.com/public'; + $this->config->baseURL = 'http://example.com/public/'; $this->config->allowedHostnames = ['www.example.jp']; $this->createRequest($this->config); @@ -105,10 +105,10 @@ private function createRequest(?App $config = null, $body = null, ?string $path public function testCurrentURLReturnsBaseURLIfNotAllowedHostname(): void { $_SERVER['HTTP_HOST'] = 'invalid.example.org'; - $_SERVER['REQUEST_URI'] = '/public'; + $_SERVER['REQUEST_URI'] = '/public/'; $_SERVER['SCRIPT_NAME'] = '/public/index.php'; - $this->config->baseURL = 'http://example.com/public'; + $this->config->baseURL = 'http://example.com/public/'; $this->config->allowedHostnames = ['www.example.jp']; $this->createRequest($this->config); @@ -118,8 +118,7 @@ public function testCurrentURLReturnsBaseURLIfNotAllowedHostname(): void public function testCurrentURLReturnsObject(): void { - // Since we're on a CLI, we must provide our own URI - $this->config->baseURL = 'http://example.com/public'; + $this->config->baseURL = 'http://example.com/public/'; $this->createRequest($this->config); @@ -132,7 +131,7 @@ public function testCurrentURLReturnsObject(): void public function testCurrentURLEquivalence(): void { $_SERVER['HTTP_HOST'] = 'example.com'; - $_SERVER['REQUEST_URI'] = '/public'; + $_SERVER['REQUEST_URI'] = '/public/'; $_SERVER['SCRIPT_NAME'] = '/index.php'; $this->config->indexPage = ''; @@ -148,8 +147,7 @@ public function testCurrentURLInSubfolder(): void $_SERVER['REQUEST_URI'] = '/foo/public/bar?baz=quip'; $_SERVER['SCRIPT_NAME'] = '/foo/public/index.php'; - // Since we're on a CLI, we must provide our own URI - $this->config->baseURL = 'http://example.com/foo/public'; + $this->config->baseURL = 'http://example.com/foo/public/'; $this->createRequest($this->config); @@ -169,7 +167,7 @@ public function testCurrentURLWithPortInSubfolder(): void $_SERVER['REQUEST_URI'] = '/foo/public/bar?baz=quip'; $_SERVER['SCRIPT_NAME'] = '/foo/public/index.php'; - $this->config->baseURL = 'http://example.com:8080/foo/public'; + $this->config->baseURL = 'http://example.com:8080/foo/public/'; $this->createRequest($this->config); @@ -210,7 +208,7 @@ public function testUriStringNoTrailingSlash(): void $_SERVER['HTTP_HOST'] = 'example.com'; $_SERVER['REQUEST_URI'] = '/assets/image.jpg'; - $this->config->baseURL = 'http://example.com'; + $this->config->baseURL = 'http://example.com/'; $this->config->indexPage = ''; $this->createRequest($this->config); From c29c172385290e006154538b1cb19fae020a0b32 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 21 Feb 2023 15:18:27 +0900 Subject: [PATCH 04/34] test: update FormHelperTest --- tests/system/Helpers/FormHelperTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/system/Helpers/FormHelperTest.php b/tests/system/Helpers/FormHelperTest.php index 1f222aff850f..cbd361875286 100644 --- a/tests/system/Helpers/FormHelperTest.php +++ b/tests/system/Helpers/FormHelperTest.php @@ -11,7 +11,7 @@ namespace CodeIgniter\Helpers; -use CodeIgniter\HTTP\URI; +use CodeIgniter\HTTP\SiteURI; use CodeIgniter\Test\CIUnitTestCase; use Config\App; use Config\Filters; @@ -35,12 +35,12 @@ protected function setUp(): void private function setRequest(): void { - $uri = new URI('http://example.com/'); - Services::injectMock('uri', $uri); - $config = new App(); $config->indexPage = 'index.php'; + $uri = new SiteURI($config); + Services::injectMock('uri', $uri); + $request = Services::request($config); Services::injectMock('request', $request); } From e68379ef52643887fd1e566e0c81a3970900ac0d Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 21 Feb 2023 16:02:03 +0900 Subject: [PATCH 05/34] test: fix failed test In GitHub Actions: Error: Call to undefined method CodeIgniter\HTTP\URI::siteUrl() --- tests/system/Helpers/HTMLHelperTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/system/Helpers/HTMLHelperTest.php b/tests/system/Helpers/HTMLHelperTest.php index 1f8485393b3f..0d9452640730 100755 --- a/tests/system/Helpers/HTMLHelperTest.php +++ b/tests/system/Helpers/HTMLHelperTest.php @@ -39,6 +39,8 @@ protected function setUp(): void { parent::setUp(); + $this->resetServices(); + helper('html'); $this->tracks = [ From 8464087e299082f8b64fc7fab7509ffad69c03d5 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 23 Feb 2023 16:16:40 +0900 Subject: [PATCH 06/34] test: update URLs in assertions --- tests/system/CodeIgniterTest.php | 4 ++-- tests/system/HTTP/ResponseTest.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index 55392e0aebc4..5fae784b51c5 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -450,7 +450,7 @@ public function testRunForceSecure(): void $response = $codeigniter->run(null, true); - $this->assertSame('https://example.com/', $response->header('Location')->getValue()); + $this->assertSame('https://example.com/index.php/', $response->header('Location')->getValue()); } public function testRunRedirectionWithNamed(): void @@ -618,7 +618,7 @@ public function testStoresPreviousURL(): void ob_get_clean(); $this->assertArrayHasKey('_ci_previous_url', $_SESSION); - $this->assertSame('http://example.com/index.php', $_SESSION['_ci_previous_url']); + $this->assertSame('http://example.com/index.php/', $_SESSION['_ci_previous_url']); } public function testNotStoresPreviousURL(): void diff --git a/tests/system/HTTP/ResponseTest.php b/tests/system/HTTP/ResponseTest.php index 50d515c3c144..e08dc1de62a3 100644 --- a/tests/system/HTTP/ResponseTest.php +++ b/tests/system/HTTP/ResponseTest.php @@ -172,7 +172,7 @@ public function testSetLink(): void $response->setLink($pager); $this->assertSame( - '; rel="first",; rel="prev",; rel="next",; rel="last"', + '; rel="first",; rel="prev",; rel="next",; rel="last"', $response->header('Link')->getValue() ); @@ -180,7 +180,7 @@ public function testSetLink(): void $response->setLink($pager); $this->assertSame( - '; rel="next",; rel="last"', + '; rel="next",; rel="last"', $response->header('Link')->getValue() ); @@ -188,7 +188,7 @@ public function testSetLink(): void $response->setLink($pager); $this->assertSame( - '; rel="first",; rel="prev"', + '; rel="first",; rel="prev"', $response->header('Link')->getValue() ); } From 76dfd39eb6ae52157fca2e7b606d651a2d629b53 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 23 Feb 2023 16:28:16 +0900 Subject: [PATCH 07/34] feat: add Services::siteurifactory() --- system/Config/BaseService.php | 3 +++ system/Config/Services.php | 22 +++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/system/Config/BaseService.php b/system/Config/BaseService.php index e4e82c67298b..474e5e06cfa3 100644 --- a/system/Config/BaseService.php +++ b/system/Config/BaseService.php @@ -37,6 +37,7 @@ use CodeIgniter\HTTP\Request; use CodeIgniter\HTTP\RequestInterface; use CodeIgniter\HTTP\ResponseInterface; +use CodeIgniter\HTTP\SiteURIFactory; use CodeIgniter\HTTP\URI; use CodeIgniter\Images\Handlers\BaseHandler; use CodeIgniter\Language\Language; @@ -47,6 +48,7 @@ use CodeIgniter\Router\Router; use CodeIgniter\Security\Security; use CodeIgniter\Session\Session; +use CodeIgniter\Superglobals; use CodeIgniter\Throttle\Throttler; use CodeIgniter\Typography\Typography; use CodeIgniter\Validation\ValidationInterface; @@ -123,6 +125,7 @@ * @method static RouteCollection routes($getShared = true) * @method static Security security(App $config = null, $getShared = true) * @method static Session session(App $config = null, $getShared = true) + * @method static SiteURIFactory siteurifactory(App $config = null, Superglobals $superglobals = null, $getShared = true) * @method static Throttler throttler($getShared = true) * @method static Timer timer($getShared = true) * @method static Toolbar toolbar(ConfigToolbar $config = null, $getShared = true) diff --git a/system/Config/Services.php b/system/Config/Services.php index 2e1e7879c087..6355b92ecc63 100644 --- a/system/Config/Services.php +++ b/system/Config/Services.php @@ -695,6 +695,26 @@ public static function session(?SessionConfig $config = null, bool $getShared = return $session; } + /** + * The Factory for SiteURI. + * + * @return SiteURIFactory + */ + public static function siteurifactory( + ?App $config = null, + ?Superglobals $superglobals = null, + bool $getShared = true + ) { + if ($getShared) { + return static::getSharedInstance('siteurifactory', $config, $superglobals); + } + + $config ??= config('App'); + $superglobals ??= new Superglobals(); + + return new SiteURIFactory($config, $superglobals); + } + /** * The Throttler class provides a simple method for implementing * rate limiting in your applications. @@ -756,7 +776,7 @@ public static function uri(?string $uri = null, bool $getShared = true) if ($uri === null) { $appConfig = config(App::class); - $factory = new SiteURIFactory($appConfig, new Superglobals()); + $factory = AppServices::siteurifactory($appConfig, new Superglobals()); return $factory->createFromGlobals(); } From 0674874352e7ac5c437042b23362833383bc14ac Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 24 Feb 2023 09:51:16 +0900 Subject: [PATCH 08/34] refactor: remove adjustPathTrailingSlash() --- system/HTTP/SiteURI.php | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/system/HTTP/SiteURI.php b/system/HTTP/SiteURI.php index 72196e1bc365..f4baa343ed37 100644 --- a/system/HTTP/SiteURI.php +++ b/system/HTTP/SiteURI.php @@ -384,7 +384,7 @@ public function baseUrl($relativePath = '', ?string $scheme = null): string return URI::createURIString( $uri->getScheme(), $uri->getAuthority(), - $this->adjustPathTrailingSlash($uri, $relativePath), + $uri->getPath(), $uri->getQuery(), $uri->getFragment() ); @@ -420,30 +420,12 @@ public function siteUrl($relativePath = '', ?string $scheme = null, ?App $config $uri = new self($config, $relativePath, $host, $scheme); - // Adjust path - $path = $this->adjustPathTrailingSlash($uri, $relativePath); - if ($config->indexPage !== '' && $relativePath === '') { - $path = rtrim($path, '/'); - } - return URI::createURIString( $uri->getScheme(), $uri->getAuthority(), - $path, + $uri->getPath(), $uri->getQuery(), $uri->getFragment() ); } - - private function adjustPathTrailingSlash(self $uri, string $relativePath): string - { - $parts = parse_url($this->getBaseURL() . $relativePath); - $path = $parts['path'] ?? ''; - - if (substr($path, -1) === '/' && substr($uri->getPath(), -1) !== '/') { - return $uri->getPath() . '/'; - } - - return $uri->getPath(); - } } From de46d3e746b50df17dde0e45b682569789a0c290 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 26 Feb 2023 16:47:55 +0900 Subject: [PATCH 09/34] fix: SiteURI's query is not set in FeatureTestTrait --- system/Test/FeatureTestTrait.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/system/Test/FeatureTestTrait.php b/system/Test/FeatureTestTrait.php index ded49bd35011..5647430609f0 100644 --- a/system/Test/FeatureTestTrait.php +++ b/system/Test/FeatureTestTrait.php @@ -183,6 +183,8 @@ public function call(string $method, string $path, ?array $params = null) /** * Performs a GET request. * + * @param string $path URI path relative to baseURL. May include query. + * * @return TestResponse * * @throws RedirectException @@ -278,6 +280,7 @@ protected function setupRequest(string $method, ?string $path = null): IncomingR $_SERVER['QUERY_STRING'] = $query; $uri->setPath($path); + $uri->setQuery($query); Services::injectMock('uri', $uri); From 9827e0e068d87b917bce664ba27c3d9b3edd2f0f Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 26 Feb 2023 17:01:18 +0900 Subject: [PATCH 10/34] fix: use SiteURI in ControllerTestTrait --- system/Test/ControllerTestTrait.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/system/Test/ControllerTestTrait.php b/system/Test/ControllerTestTrait.php index 828dabb9e946..9ca47e24196c 100644 --- a/system/Test/ControllerTestTrait.php +++ b/system/Test/ControllerTestTrait.php @@ -100,7 +100,8 @@ protected function setUpControllerTestTrait(): void } if (! $this->uri instanceof URI) { - $this->uri = Services::uri($this->appConfig->baseURL ?? 'http://example.com/', false); + $factory = Services::siteurifactory($_SERVER, $this->appConfig, false); + $this->uri = $factory->createFromGlobals(); } if (empty($this->request)) { From 32078427cf6c5c74477e710157bcb60bf0c6f803 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 26 Feb 2023 17:03:50 +0900 Subject: [PATCH 11/34] test: update assertion URL (add `index.php/`) --- tests/system/CommonFunctionsTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/system/CommonFunctionsTest.php b/tests/system/CommonFunctionsTest.php index 1ce49c4cd37e..c678a0dd1fbb 100644 --- a/tests/system/CommonFunctionsTest.php +++ b/tests/system/CommonFunctionsTest.php @@ -609,7 +609,10 @@ public function testForceHttpsNullRequestAndResponse(): void force_https(); } catch (Exception $e) { $this->assertInstanceOf(RedirectException::class, $e); - $this->assertSame('https://example.com/', $e->getResponse()->header('Location')->getValue()); + $this->assertSame( + 'https://example.com/index.php/', + $e->getResponse()->header('Location')->getValue() + ); $this->assertFalse($e->getResponse()->hasCookie('force')); $this->assertSame('header', $e->getResponse()->getHeaderLine('Force')); $this->assertSame('', $e->getResponse()->getBody()); From a4ba66d4f329f4b6965ce49e8a52d408dd7d80fc Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 25 Jul 2023 17:38:45 +0900 Subject: [PATCH 12/34] refactor: remove unused private method --- system/HTTP/IncomingRequest.php | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index 4f0f1cb0d85d..8734e8f97552 100755 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -472,30 +472,6 @@ public function setPath(string $path, ?App $config = null) return $this; } - /** - * @deprecated 4.4.0 Moved to SiteURIFactory. - */ - 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; - } - /** * Returns the URI path relative to baseURL, * running detection as necessary. From 9ec7fbc83ba7fbbfd228664176e88e54ae45c656 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 25 Jul 2023 17:45:07 +0900 Subject: [PATCH 13/34] feat: add Services::superglobals() --- system/Config/BaseService.php | 1 + system/Config/Services.php | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/system/Config/BaseService.php b/system/Config/BaseService.php index 474e5e06cfa3..b66b8c9f535e 100644 --- a/system/Config/BaseService.php +++ b/system/Config/BaseService.php @@ -126,6 +126,7 @@ * @method static Security security(App $config = null, $getShared = true) * @method static Session session(App $config = null, $getShared = true) * @method static SiteURIFactory siteurifactory(App $config = null, Superglobals $superglobals = null, $getShared = true) + * @method static Superglobals superglobals(array $server = null, array $get = null, bool $getShared = true) * @method static Throttler throttler($getShared = true) * @method static Timer timer($getShared = true) * @method static Toolbar toolbar(ConfigToolbar $config = null, $getShared = true) diff --git a/system/Config/Services.php b/system/Config/Services.php index 6355b92ecc63..ca88b10221ca 100644 --- a/system/Config/Services.php +++ b/system/Config/Services.php @@ -710,11 +710,28 @@ public static function siteurifactory( } $config ??= config('App'); - $superglobals ??= new Superglobals(); + $superglobals ??= AppServices::superglobals(); return new SiteURIFactory($config, $superglobals); } + /** + * Superglobals. + * + * @return Superglobals + */ + public static function superglobals( + ?array $server = null, + ?array $get = null, + bool $getShared = true + ) { + if ($getShared) { + return static::getSharedInstance('superglobals', $server, $get); + } + + return new Superglobals($server, $get); + } + /** * The Throttler class provides a simple method for implementing * rate limiting in your applications. @@ -776,7 +793,7 @@ public static function uri(?string $uri = null, bool $getShared = true) if ($uri === null) { $appConfig = config(App::class); - $factory = AppServices::siteurifactory($appConfig, new Superglobals()); + $factory = AppServices::siteurifactory($appConfig, AppServices::superglobals()); return $factory->createFromGlobals(); } From f00cd8cfbd5fb89284a970538f96c09c799274e3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 25 Jul 2023 17:54:46 +0900 Subject: [PATCH 14/34] fix: out-of-dated parameters --- system/Test/ControllerTestTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Test/ControllerTestTrait.php b/system/Test/ControllerTestTrait.php index 9ca47e24196c..2948bc5790c0 100644 --- a/system/Test/ControllerTestTrait.php +++ b/system/Test/ControllerTestTrait.php @@ -100,7 +100,7 @@ protected function setUpControllerTestTrait(): void } if (! $this->uri instanceof URI) { - $factory = Services::siteurifactory($_SERVER, $this->appConfig, false); + $factory = Services::siteurifactory($this->appConfig, Services::superglobals(), false); $this->uri = $factory->createFromGlobals(); } From 70179192966efee0198d645f30c4e1dff716f026 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 25 Jul 2023 17:55:26 +0900 Subject: [PATCH 15/34] test: use Services::superglobals() --- tests/_support/Config/Services.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/_support/Config/Services.php b/tests/_support/Config/Services.php index 9a553d0116a0..e051ea8d6b24 100644 --- a/tests/_support/Config/Services.php +++ b/tests/_support/Config/Services.php @@ -13,7 +13,6 @@ use CodeIgniter\HTTP\SiteURIFactory; use CodeIgniter\HTTP\URI; -use CodeIgniter\Superglobals; use Config\App; use Config\Services as BaseServices; use RuntimeException; @@ -46,7 +45,7 @@ public static function uri(?string $uri = null, bool $getShared = true) if ($uri === null) { $appConfig = config(App::class); - $factory = new SiteURIFactory($appConfig, new Superglobals()); + $factory = new SiteURIFactory($appConfig, Services::superglobals()); return $factory->createFromGlobals(); } From 715bd00da9e053b38f8341aa826ed4bfc6fc35f3 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 25 Jul 2023 18:11:11 +0900 Subject: [PATCH 16/34] test: update out-of-dated parameters --- tests/system/Helpers/URLHelper/MiscUrlTest.php | 3 ++- tests/system/Helpers/URLHelper/SiteUrlTest.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/system/Helpers/URLHelper/MiscUrlTest.php b/tests/system/Helpers/URLHelper/MiscUrlTest.php index c26e9e7fbf9b..ecb59e1ef2b5 100644 --- a/tests/system/Helpers/URLHelper/MiscUrlTest.php +++ b/tests/system/Helpers/URLHelper/MiscUrlTest.php @@ -17,6 +17,7 @@ use CodeIgniter\HTTP\SiteURIFactory; use CodeIgniter\HTTP\UserAgent; use CodeIgniter\Router\Exceptions\RouterException; +use CodeIgniter\Superglobals; use CodeIgniter\Test\CIUnitTestCase; use Config\App; use InvalidArgumentException; @@ -69,7 +70,7 @@ public function testPreviousURLUsesSessionFirst(): void private function createRequest(string $uri): void { - $factory = new SiteURIFactory($_SERVER, $this->config); + $factory = new SiteURIFactory($this->config, new Superglobals()); $uri = $factory->createFromString($uri); diff --git a/tests/system/Helpers/URLHelper/SiteUrlTest.php b/tests/system/Helpers/URLHelper/SiteUrlTest.php index a28a588cbc5f..1a03d80f3521 100644 --- a/tests/system/Helpers/URLHelper/SiteUrlTest.php +++ b/tests/system/Helpers/URLHelper/SiteUrlTest.php @@ -17,6 +17,7 @@ use CodeIgniter\HTTP\SiteURIFactory; use CodeIgniter\HTTP\URI; use CodeIgniter\HTTP\UserAgent; +use CodeIgniter\Superglobals; use CodeIgniter\Test\CIUnitTestCase; use Config\App; @@ -55,7 +56,7 @@ private function createRequest(?App $config = null, $body = null, ?string $path { $config ??= new App(); - $factory = new SiteURIFactory($_SERVER, $config); + $factory = new SiteURIFactory($config, new Superglobals()); $uri = $factory->createFromGlobals(); if ($path !== null) { From f24ca6ac93d02f61dc930c2d18054f80fb41db92 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 25 Jul 2023 18:22:09 +0900 Subject: [PATCH 17/34] docs: add doc comments --- system/HTTP/SiteURIFactory.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/system/HTTP/SiteURIFactory.php b/system/HTTP/SiteURIFactory.php index f0b846a44457..c6636f33898f 100644 --- a/system/HTTP/SiteURIFactory.php +++ b/system/HTTP/SiteURIFactory.php @@ -15,6 +15,11 @@ use CodeIgniter\Superglobals; use Config\App; +/** + * Creates SiteURI using superglobals. + * + * This class also updates superglobal $_SERVER and $_GET. + */ final class SiteURIFactory { private App $appConfig; @@ -42,6 +47,7 @@ public function createFromGlobals(): SiteURI * Create the SiteURI object from URI string. * * @internal Used for testing purposes only. + * @testTag */ public function createFromString(string $uri): SiteURI { @@ -79,6 +85,7 @@ public function createFromString(string $uri): SiteURI * @return string The route path * * @internal Used for testing purposes only. + * @testTag */ public function detectRoutePath(string $protocol = ''): string { From 7514d6d595272de1c186c7a820911947ed5ac632 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jul 2023 13:02:41 +0900 Subject: [PATCH 18/34] fix: support protocol-relative links --- system/HTTP/SiteURI.php | 28 +++++++++++++--------------- system/Helpers/url_helper.php | 1 - tests/system/HTTP/SiteURITest.php | 11 +++++++++++ 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/system/HTTP/SiteURI.php b/system/HTTP/SiteURI.php index f4baa343ed37..287146e64c7c 100644 --- a/system/HTTP/SiteURI.php +++ b/system/HTTP/SiteURI.php @@ -145,7 +145,7 @@ private function determineBaseURL( $uri = new URI($baseURL); // Update scheme - if ($scheme !== null) { + if ($scheme !== null && $scheme !== '') { $uri->setScheme($scheme); } elseif ($configApp->forceGlobalSecureRequests) { $uri->setScheme('https'); @@ -381,13 +381,12 @@ public function baseUrl($relativePath = '', ?string $scheme = null): string $uri = new self($config, $relativePath, $host, $scheme); - return URI::createURIString( - $uri->getScheme(), - $uri->getAuthority(), - $uri->getPath(), - $uri->getQuery(), - $uri->getFragment() - ); + // Support protocol-relative links + if ($scheme === '') { + return substr((string) $uri, strlen($uri->getScheme()) + 1); + } + + return (string) $uri; } /** @@ -420,12 +419,11 @@ public function siteUrl($relativePath = '', ?string $scheme = null, ?App $config $uri = new self($config, $relativePath, $host, $scheme); - return URI::createURIString( - $uri->getScheme(), - $uri->getAuthority(), - $uri->getPath(), - $uri->getQuery(), - $uri->getFragment() - ); + // Support protocol-relative links + if ($scheme === '') { + return substr((string) $uri, strlen($uri->getScheme()) + 1); + } + + return (string) $uri; } } diff --git a/system/Helpers/url_helper.php b/system/Helpers/url_helper.php index b8c1a687e6bb..a74fe944f148 100644 --- a/system/Helpers/url_helper.php +++ b/system/Helpers/url_helper.php @@ -33,7 +33,6 @@ function site_url($relativePath = '', ?string $scheme = null, ?App $config = nul assert($currentURI instanceof SiteURI); - // @TODO supprot protocol-relative links return $currentURI->siteUrl($relativePath, $scheme, $config); } } diff --git a/tests/system/HTTP/SiteURITest.php b/tests/system/HTTP/SiteURITest.php index 392394086d30..f839b51c0771 100644 --- a/tests/system/HTTP/SiteURITest.php +++ b/tests/system/HTTP/SiteURITest.php @@ -293,6 +293,17 @@ public function testConstructorScheme() $this->assertSame('https://example.com/', $uri->getBaseURL()); } + public function testConstructorEmptyScheme() + { + $config = new App(); + + $uri = new SiteURI($config, '', null, ''); + + $this->assertInstanceOf(SiteURI::class, $uri); + $this->assertSame('http://example.com/index.php', (string) $uri); + $this->assertSame('http://example.com/', $uri->getBaseURL()); + } + public function testConstructorForceGlobalSecureRequests() { $config = new App(); From ae4f7c84caeca41e93d1c9529d3215e5c1f63d94 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jul 2023 16:59:28 +0900 Subject: [PATCH 19/34] docs: add @deprecated versions --- system/HTTP/IncomingRequest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/system/HTTP/IncomingRequest.php b/system/HTTP/IncomingRequest.php index 8734e8f97552..947f5cc4c668 100755 --- a/system/HTTP/IncomingRequest.php +++ b/system/HTTP/IncomingRequest.php @@ -234,7 +234,7 @@ public function detectLocale($config) * * @return void * - * @deprecated No longer used. + * @deprecated 4.4.0 No longer used. */ protected function detectURI(string $protocol, string $baseURL) { @@ -463,7 +463,7 @@ public function isSecure(): bool * * @return $this * - * @deprecated This method will be private. The parameter $config is deprecated. No longer used. + * @deprecated 4.4.0 This method will be private. The parameter $config is deprecated. No longer used. */ public function setPath(string $path, ?App $config = null) { @@ -911,7 +911,7 @@ public function getFile(string $fileID) * * Do some final cleaning of the URI and return it, currently only used in static::_parse_request_uri() * - * @deprecated Use URI::removeDotSegments() directly + * @deprecated 4.1.2 Use URI::removeDotSegments() directly */ protected function removeRelativeDirectory(string $uri): string { From 6c83470cace439e0be8021547be560e6cca1aab7 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jul 2023 16:56:09 +0900 Subject: [PATCH 20/34] docs: update user guide --- user_guide_src/source/changelogs/v4.4.0.rst | 42 +++++++++++++++++++ .../source/incoming/incomingrequest.rst | 24 +++++++---- .../source/incoming/incomingrequest/021.php | 3 +- .../source/incoming/incomingrequest/022.php | 15 ------- .../source/installation/upgrade_440.rst | 9 ++++ user_guide_src/source/libraries/uri.rst | 33 +++++++++++---- user_guide_src/source/libraries/uri/001.php | 2 +- user_guide_src/source/libraries/uri/002.php | 2 +- user_guide_src/source/libraries/uri/003.php | 1 - user_guide_src/source/libraries/uri/005.php | 2 +- 10 files changed, 96 insertions(+), 37 deletions(-) delete mode 100644 user_guide_src/source/incoming/incomingrequest/022.php diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index 4b7b065cd6e2..d7ec516fa8da 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -72,6 +72,46 @@ CodeIgniter and exit() The ``CodeIgniter::run()`` method no longer calls ``exit(EXIT_SUCCESS)``. The exit call is moved to **public/index.php**. +.. _v440-site-uri-changes: + +Site URI Changes +================ + +A new ``SiteURI`` class that extends the ``URI`` class and represents the site +URI has been added, and now it is used in many places that need the current URI. + +``$this->request->getUri()`` in controllers returns the ``SiteURI`` instance. +Also, :php:func:`site_url()`, :php:func:`base_url()`, and :php:func:`current_url()` +use the SiteURI internally. + +getPath() +--------- + +The ``getPath()`` method now always returns the full URI path with leading ``/``. +Therefore, when your baseURL has sub-directories and you want to get the relative +path to baseURL, you must use the new ``getRoutePath()`` method instead. + +For example:: + + baseURL: http://localhost:8888/CodeIgniter4/ + The current URI: http://localhost:8888/CodeIgniter4/foo/bar + getPath(): /CodeIgniter4/foo/bar + getRoutePath(): foo/bar + +Site URI Values +--------------- + +The SiteURI class normalizes site URIs more strictly than before, and some bugs +have been fixed. + +As a result, the framework may return site URIs or the URI paths slightly differently +than in previous versions. +For example, ``/`` will be added after ``index.php``:: + + http://example.com/test/index.php?page=1 + ↓ + http://example.com/test/index.php/?page=1 + .. _v440-interface-changes: Interface Changes @@ -192,6 +232,8 @@ Libraries the value of the `full_path` index of the file if it was uploaded via directory upload. - **CURLRequest:** Added a request option ``proxy``. See :ref:`CURLRequest Class `. +- **URI:** A new ``SiteURI`` class that extends ``URI`` and represents the site + URI has been added. Helpers and Functions ===================== diff --git a/user_guide_src/source/incoming/incomingrequest.rst b/user_guide_src/source/incoming/incomingrequest.rst index ed3c40ec7f2b..5a58ea1c1608 100644 --- a/user_guide_src/source/incoming/incomingrequest.rst +++ b/user_guide_src/source/incoming/incomingrequest.rst @@ -228,11 +228,10 @@ The object gives you full abilities to grab any part of the request on it's own: .. literalinclude:: incomingrequest/021.php -You can work with the current URI string (the path relative to your baseURL) using the ``getPath()`` and ``setPath()`` methods. -Note that this relative path on the shared instance of ``IncomingRequest`` is what the :doc:`URL Helper ` -functions use, so this is a helpful way to "spoof" an incoming request for testing: +You can work with the current URI string (the path relative to your baseURL) using the ``getRoutePath()``. -.. literalinclude:: incomingrequest/022.php +.. note:: The ``getRoutePath()`` method can be used since v4.4.0. Prior to v4.4.0, + the ``getPath()`` method returned the path relative to your baseURL. Uploaded Files ************** @@ -476,15 +475,22 @@ The methods provided by the parent classes that are available are: :returns: The current URI path relative to baseURL :rtype: string - This is the safest method to determine the "current URI", since ``IncomingRequest::$uri`` - may not be aware of the complete App configuration for base URLs. + This method returns the current URI path relative to baseURL. + + .. note:: Prior to v4.4.0, this was the safest method to determine the + "current URI", since ``IncomingRequest::$uri`` might not be aware of + the complete App configuration for base URLs. .. php:method:: setPath($path) + .. deprecated:: 4.4.0 + :param string $path: The relative path to use as the current URI :returns: This Incoming Request :rtype: IncomingRequest - Used mostly just for testing purposes, this allows you to set the relative path - value for the current request instead of relying on URI detection. This will also - update the underlying ``URI`` instance with the new path. + .. note:: Prior to v4.4.0, used mostly just for testing purposes, this + allowed you to set the relative path value for the current request + instead of relying on URI detection. This also updated the + underlying ``URI`` instance with the new path. + diff --git a/user_guide_src/source/incoming/incomingrequest/021.php b/user_guide_src/source/incoming/incomingrequest/021.php index 558bb6a98e6f..9a1b8601b07c 100644 --- a/user_guide_src/source/incoming/incomingrequest/021.php +++ b/user_guide_src/source/incoming/incomingrequest/021.php @@ -7,7 +7,8 @@ echo $uri->getUserInfo(); // snoopy:password echo $uri->getHost(); // example.com echo $uri->getPort(); // 88 -echo $uri->getPath(); // path/to/page +echo $uri->getPath(); // /path/to/page +echo $uri->getRoutePath(); // path/to/page echo $uri->getQuery(); // foo=bar&bar=baz print_r($uri->getSegments()); // Array ( [0] => path [1] => to [2] => page ) echo $uri->getSegment(1); // path diff --git a/user_guide_src/source/incoming/incomingrequest/022.php b/user_guide_src/source/incoming/incomingrequest/022.php deleted file mode 100644 index 723d81e3a3a3..000000000000 --- a/user_guide_src/source/incoming/incomingrequest/022.php +++ /dev/null @@ -1,15 +0,0 @@ -setPath('users/list'); - - $menu = new MyMenu(); - - $this->assertTrue('users/list', $menu->getActiveLink()); - } -} diff --git a/user_guide_src/source/installation/upgrade_440.rst b/user_guide_src/source/installation/upgrade_440.rst index ffb8751c5206..52a4d45066d9 100644 --- a/user_guide_src/source/installation/upgrade_440.rst +++ b/user_guide_src/source/installation/upgrade_440.rst @@ -47,6 +47,15 @@ If your code depends on this bug, fix the segment number. .. literalinclude:: upgrade_440/002.php :lines: 2- +Site URI Changes +================ + +When your baseURL has sub-directories and you get the relative path to baseURL of +the current URI by the ``URI::getPath()`` method, you must use the new +``SiteURI::getRoutePath()`` method instead. + +See :ref:`v440-site-uri-changes` for details. + When You Extend Exceptions ========================== diff --git a/user_guide_src/source/libraries/uri.rst b/user_guide_src/source/libraries/uri.rst index bfd189859bf4..3ce031bf64bf 100644 --- a/user_guide_src/source/libraries/uri.rst +++ b/user_guide_src/source/libraries/uri.rst @@ -14,34 +14,47 @@ relative URI to an existing one and have it resolved safely and correctly. Creating URI instances ====================== -Creating a URI instance is as simple as creating a new class instance: +Creating a URI instance is as simple as creating a new class instance. + +When you create the new instance, you can pass a full or partial URL in the constructor and it will be parsed +into its appropriate sections: .. literalinclude:: uri/001.php + :lines: 2- -Alternatively, you can use the ``service()`` function to return an instance for you: +Alternatively, you can use the :php:func:`service()` function to return an instance for you: -.. literalinclude:: uri/002.php +.. literalinclude:: uri/003.php + :lines: 2- -When you create the new instance, you can pass a full or partial URL in the constructor and it will be parsed -into its appropriate sections: +Since v4.4.0, if you don't pass a URL, it returns the current URI: -.. literalinclude:: uri/003.php +.. literalinclude:: uri/002.php + :lines: 2- + +.. note:: The above code returns the ``SiteURI`` instance, that extends the ``URI`` + class. The ``URI`` class is for general URIs, but the ``SiteURI`` class is + for your site URIs. The Current URI --------------- Many times, all you really want is an object representing the current URL of this request. -You can use one of the functions available in the :doc:`../helpers/url_helper`: +You can use the :php:func:`current_url()` function available in the :doc:`../helpers/url_helper`: .. literalinclude:: uri/004.php + :lines: 2- You must pass ``true`` as the first parameter, otherwise, it will return the string representation of the current URL. This URI is based on the path (relative to your ``baseURL``) as determined by the current request object and your settings in ``Config\App`` (``baseURL``, ``indexPage``, and ``forceGlobalSecureRequests``). -Assuming that you're in a controller that extends ``CodeIgniter\Controller`` you can get this relative path: + +Assuming that you're in a controller that extends ``CodeIgniter\Controller``, you +can also get the current SiteURI instance: .. literalinclude:: uri/005.php + :lines: 2- =========== URI Strings @@ -136,6 +149,10 @@ can be used to manipulate it: .. note:: When setting the path this way, or any other way the class allows, it is sanitized to encode any dangerous characters, and remove dot segments for safety. +.. note:: Since v4.4.0, the ``SiteURI::getRoutePath()`` method, + returns the URI path relative to baseURL, and the ``SiteURI::getPath()`` + method always returns the full URI path with leading ``/``. + Query ----- diff --git a/user_guide_src/source/libraries/uri/001.php b/user_guide_src/source/libraries/uri/001.php index 524e67669110..7219d03d248f 100644 --- a/user_guide_src/source/libraries/uri/001.php +++ b/user_guide_src/source/libraries/uri/001.php @@ -1,3 +1,3 @@ request->getPath(); +$uri = $this->request->getUri(); From e4a2523d4c2868fe83592a842388d1444eb59d98 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jul 2023 17:17:33 +0900 Subject: [PATCH 21/34] docs: add @deprecated versions --- system/HTTP/URI.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/system/HTTP/URI.php b/system/HTTP/URI.php index 429aa73a7b7f..fb9d780c1485 100644 --- a/system/HTTP/URI.php +++ b/system/HTTP/URI.php @@ -36,14 +36,14 @@ class URI * * @var string * - * @deprecated Not used. + * @deprecated 4.4.0 Not used. */ protected $uriString; /** * The Current baseURL. * - * @deprecated Use SiteURI instead. + * @deprecated 4.4.0 Use SiteURI instead. */ private ?string $baseURL = null; @@ -292,7 +292,7 @@ public function useRawQueryString(bool $raw = true) * * @throws HTTPException * - * @deprecated This method will be private. + * @deprecated 4.4.0 This method will be private. */ public function setURI(?string $uri = null) { @@ -705,7 +705,7 @@ public function setAuthority(string $str) * * @return $this * - * @deprecated Use `withScheme()` instead. + * @deprecated 4.4.0 Use `withScheme()` instead. */ public function setScheme(string $str) { From c2f88600ee1074aa979690566b8d1031277a5fbe Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 28 Jul 2023 17:17:46 +0900 Subject: [PATCH 22/34] docs: add Deprecations --- user_guide_src/source/changelogs/v4.4.0.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index d7ec516fa8da..9ce187ddae98 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -324,8 +324,20 @@ Deprecations ``$redirect`` in ``Security`` are deprecated, and no longer used. Use ``$config`` instead. - **URI:** + - ``URI::$uriString`` is deprecated. + - ``URI::$baseURL`` is deprecated. Use ``SiteURI`` instead. - ``URI::setSilent()`` is deprecated. - ``URI::setScheme()`` is deprecated. Use ``withScheme()`` instead. + - ``URI::setURI()`` is deprecated. +- **IncomingRequest:** + - ``IncomingRequest::detectURI()`` is deprecated and no longer used. + - ``IncomingRequest::detectPath()`` is deprecated, and no longer used. It + moved to ``SiteURIFactory``. + - ``IncomingRequest::parseRequestURI()`` is deprecated, and no longer used. It + moved to ``SiteURIFactory``. + - ``IncomingRequest::parseQueryString()`` is deprecated, and no longer used. It + moved to ``SiteURIFactory``. + - ``IncomingRequest::setPath()`` is deprecated. Bugs Fixed ********** From e01d6220874d9fa95e56eb517acd0024adfa4fc0 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 31 Jul 2023 17:29:37 +0900 Subject: [PATCH 23/34] test: remove unused private method --- tests/system/Helpers/URLHelper/CurrentUrlTest.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/system/Helpers/URLHelper/CurrentUrlTest.php b/tests/system/Helpers/URLHelper/CurrentUrlTest.php index a28fa78b9b98..c735064fa287 100644 --- a/tests/system/Helpers/URLHelper/CurrentUrlTest.php +++ b/tests/system/Helpers/URLHelper/CurrentUrlTest.php @@ -194,15 +194,6 @@ public function testUriString(): void $this->assertSame('assets/image.jpg', uri_string()); } - private function setService(string $uri): void - { - $uri = new URI($uri); - Services::injectMock('uri', $uri); - - $request = Services::request($this->config); - Services::injectMock('request', $request); - } - public function testUriStringNoTrailingSlash(): void { $_SERVER['HTTP_HOST'] = 'example.com'; From eeca9e9a023d18dc502082097b2c060e5afd8b82 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 4 Aug 2023 13:47:18 +0900 Subject: [PATCH 24/34] docs: fix section level --- user_guide_src/source/changelogs/v4.4.0.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index 9ce187ddae98..c67497ede1a2 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -75,7 +75,7 @@ exit call is moved to **public/index.php**. .. _v440-site-uri-changes: Site URI Changes -================ +---------------- A new ``SiteURI`` class that extends the ``URI`` class and represents the site URI has been added, and now it is used in many places that need the current URI. @@ -85,7 +85,7 @@ Also, :php:func:`site_url()`, :php:func:`base_url()`, and :php:func:`current_url use the SiteURI internally. getPath() ---------- +^^^^^^^^^ The ``getPath()`` method now always returns the full URI path with leading ``/``. Therefore, when your baseURL has sub-directories and you want to get the relative @@ -99,7 +99,7 @@ For example:: getRoutePath(): foo/bar Site URI Values ---------------- +^^^^^^^^^^^^^^^ The SiteURI class normalizes site URIs more strictly than before, and some bugs have been fixed. From 975281545114848324aecd6bdc3028ef8829abb0 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 4 Aug 2023 14:09:45 +0900 Subject: [PATCH 25/34] test: update data provider method name --- tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php index 1e96bf065b92..2890085627f3 100644 --- a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php +++ b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php @@ -278,7 +278,7 @@ public function testPathInfoSubfolder() } /** - * @dataProvider providePathChecks + * @dataProvider provideExtensionPHP * * @param string $path * @param string $detectPath @@ -296,7 +296,7 @@ public function testExtensionPHP($path, $detectPath) $this->assertSame($detectPath, $factory->detectRoutePath()); } - public function providePathChecks(): iterable + public function provideExtensionPHP(): iterable { return [ 'not /index.php' => [ From d80c7b5a35742d69fb70e74c88b2803b8db6c4b1 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 9 Aug 2023 14:31:18 +0900 Subject: [PATCH 26/34] fix: ControllerTestTrait::withUri() updates Request instance --- system/Test/ControllerTestTrait.php | 8 +++++++- user_guide_src/source/changelogs/v4.4.0.rst | 4 ++++ user_guide_src/source/testing/controllers.rst | 5 +++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/system/Test/ControllerTestTrait.php b/system/Test/ControllerTestTrait.php index 2948bc5790c0..7e4091be6175 100644 --- a/system/Test/ControllerTestTrait.php +++ b/system/Test/ControllerTestTrait.php @@ -278,7 +278,13 @@ public function withLogger($logger) */ public function withUri(string $uri) { - $this->uri = new URI($uri); + $factory = Services::siteurifactory(); + $this->uri = $factory->createFromString($uri); + Services::injectMock('uri', $this->uri); + + // Update the Request instance, because Request has the SiteURI instance. + $this->request = Services::incomingrequest(null, false); + Services::injectMock('request', $this->request); return $this; } diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index c67497ede1a2..a6a3bdde7d88 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -347,6 +347,10 @@ Bugs Fixed (e.g., **foo-bar**) and one URI for underscores (e.g., **foo_bar**). This bug has been fixed. Now the URI for underscores (**foo_bar**) is not accessible. - **Output Buffering:** Bug fix with output buffering. +- **ControllerTestTrait:** ``ControllerTestTrait::withUri()`` creates a new Request + instance with the URI. Because the Request instance should have the URI instance. + Also if the hostname in the URI string is invalid with ``Config\App``, the valid + hostname will be set. See the repo's `CHANGELOG.md `_ diff --git a/user_guide_src/source/testing/controllers.rst b/user_guide_src/source/testing/controllers.rst index 130364260cba..8f38f68c4e3d 100644 --- a/user_guide_src/source/testing/controllers.rst +++ b/user_guide_src/source/testing/controllers.rst @@ -101,6 +101,11 @@ representing a valid URI: It is a good practice to always provide the URI during testing to avoid surprises. +.. note:: Since v4.4.0, this method creates a new Request instance with the URI. + Because the Request instance should have the URI instance. Also if the hostname + in the URI string is invalid with ``Config\App``, the valid hostname will be + set. + withBody($body) --------------- From 3a76189864395859a819e29442b116138918bd26 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 9 Aug 2023 14:52:41 +0900 Subject: [PATCH 27/34] test: update data provider method name --- tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php index 2890085627f3..34b242b57ffa 100644 --- a/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php +++ b/tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php @@ -296,7 +296,7 @@ public function testExtensionPHP($path, $detectPath) $this->assertSame($detectPath, $factory->detectRoutePath()); } - public function provideExtensionPHP(): iterable + public static function provideExtensionPHP(): iterable { return [ 'not /index.php' => [ From 0eebc5e60b545ae062fc094f721608dcbf92af85 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 9 Aug 2023 16:16:09 +0900 Subject: [PATCH 28/34] test: add tests for SiteURIFactory --- tests/system/HTTP/SiteURIFactoryTest.php | 89 ++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 7 deletions(-) diff --git a/tests/system/HTTP/SiteURIFactoryTest.php b/tests/system/HTTP/SiteURIFactoryTest.php index 138ccad0b48d..c7522c8cf089 100644 --- a/tests/system/HTTP/SiteURIFactoryTest.php +++ b/tests/system/HTTP/SiteURIFactoryTest.php @@ -85,16 +85,91 @@ public function testCreateFromGlobalsAllowedHost() $this->assertSame('woot', $uri->getRoutePath()); } - public function testCreateFromString() - { + /** + * @dataProvider provideCreateFromStringWithIndexPage + */ + public function testCreateFromStringWithIndexPage( + string $uriString, + string $expectUriString, + string $expectedPath, + string $expectedRoutePath + ) { $factory = $this->createSiteURIFactory(); - $uriString = 'http://invalid.example.jp/foo/bar?page=3'; - $uri = $factory->createFromString($uriString); + $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()); + $this->assertSame($expectUriString, (string) $uri); + $this->assertSame($expectedPath, $uri->getPath()); + $this->assertSame($expectedRoutePath, $uri->getRoutePath()); + } + + public static function provideCreateFromStringWithIndexPage(): iterable + { + return [ + 'indexPage path query' => [ + 'http://invalid.example.jp/foo/bar?page=3', // $uriString + 'http://localhost:8080/index.php/foo/bar?page=3', // $expectUriString + '/index.php/foo/bar', // $expectedPath + 'foo/bar', // $expectedRoutePath + ], + 'indexPage noPath' => [ + 'http://localhost:8080', // $uriString + 'http://localhost:8080/index.php', // $expectUriString + '/index.php', // $expectedPath + '', // $expectedRoutePath + ], + 'indexPage slash' => [ + 'http://localhost:8080/', // $uriString + 'http://localhost:8080/index.php/', // $expectUriString + '/index.php/', // $expectedPath + '', // $expectedRoutePath + ], + ]; + } + + /** + * @dataProvider provideCreateFromStringWithoutIndexPage + */ + public function testCreateFromStringWithoutIndexPage( + string $uriString, + string $expectUriString, + string $expectedPath, + string $expectedRoutePath + ) { + $config = new App(); + $config->indexPage = ''; + $factory = $this->createSiteURIFactory($config); + + $uri = $factory->createFromString($uriString); + + $this->assertInstanceOf(SiteURI::class, $uri); + $this->assertSame($expectUriString, (string) $uri); + $this->assertSame($expectedPath, $uri->getPath()); + $this->assertSame($expectedRoutePath, $uri->getRoutePath()); + } + + public static function provideCreateFromStringWithoutIndexPage(): iterable + { + return [ + 'path query' => [ + 'http://invalid.example.jp/foo/bar?page=3', // $uriString + 'http://localhost:8080/foo/bar?page=3', // $expectUriString + '/foo/bar', // $expectedPath + 'foo/bar', // $expectedRoutePath + ], + 'noPath' => [ + 'http://localhost:8080', // $uriString + 'http://localhost:8080/', // $expectUriString + '/', // $expectedPath + '', // $expectedRoutePath + ], + 'slash' => [ + 'http://localhost:8080/', // $uriString + 'http://localhost:8080/', // $expectUriString + '/', // $expectedPath + '', // $expectedRoutePath + ], + ]; } } From d9a5a6d1feadc9c7b89b9bd979710100535ce92a Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 9 Aug 2023 16:16:42 +0900 Subject: [PATCH 29/34] fix: when URI string does not have path --- 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 c6636f33898f..e250c559058c 100644 --- a/system/HTTP/SiteURIFactory.php +++ b/system/HTTP/SiteURIFactory.php @@ -70,7 +70,7 @@ public function createFromString(string $uri): SiteURI $fragment = '#' . $parts['fragment']; } - $relativePath = $parts['path'] . $query . $fragment; + $relativePath = ($parts['path'] ?? '') . $query . $fragment; $host = $this->getValidHost($parts['host']); return new SiteURI($this->appConfig, $relativePath, $host, $parts['scheme']); From 546d11200296d22f0690109fee5b40148480fc70 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 10 Aug 2023 09:02:58 +0900 Subject: [PATCH 30/34] test: remove redundant test --- tests/system/Test/FeatureTestTraitTest.php | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/tests/system/Test/FeatureTestTraitTest.php b/tests/system/Test/FeatureTestTraitTest.php index 75d0db1a1d57..d4e5195b3347 100644 --- a/tests/system/Test/FeatureTestTraitTest.php +++ b/tests/system/Test/FeatureTestTraitTest.php @@ -52,26 +52,13 @@ public function testCallGet(): void ]); $response = $this->get('home'); - $response->assertSee('Hello World'); - $response->assertDontSee('Again'); - } - - public function testCallSimpleGet(): void - { - $this->withRoutes([ - [ - 'add', - 'home', - static fn () => 'Hello Earth', - ], - ]); - $response = $this->call('get', 'home'); - $this->assertInstanceOf(TestResponse::class, $response); $this->assertInstanceOf(Response::class, $response->response()); $this->assertTrue($response->isOK()); - $this->assertSame('Hello Earth', $response->response()->getBody()); + $this->assertSame('Hello World', $response->response()->getBody()); $this->assertSame(200, $response->response()->getStatusCode()); + $response->assertSee('Hello World'); + $response->assertDontSee('Again'); } public function testClosureWithEcho() From 16621bf69d67fa1ceff1185791b4231ce5ca6abe Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 10 Aug 2023 08:56:06 +0900 Subject: [PATCH 31/34] test: add tests for uri_string() and current_url() --- tests/system/Test/ControllerTestTraitTest.php | 12 ++++++++++++ tests/system/Test/FeatureTestTraitTest.php | 16 ++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/tests/system/Test/ControllerTestTraitTest.php b/tests/system/Test/ControllerTestTraitTest.php index b5d08e72ed8f..ab45c6372d41 100644 --- a/tests/system/Test/ControllerTestTraitTest.php +++ b/tests/system/Test/ControllerTestTraitTest.php @@ -19,6 +19,7 @@ use Config\App; use Config\Services; use Exception; +use Tests\Support\Controllers\Newautorouting; use Tests\Support\Controllers\Popcorn; /** @@ -259,4 +260,15 @@ public function throwsBody(): void $this->withBody('banana')->execute('throwsBody'); } + + public function testWithUriUpdatesUriStringAndCurrentUrlValues() + { + $result = $this->withURI('http://example.com/foo/bar/1/2/3') + ->controller(Newautorouting::class) + ->execute('postSave', '1', '2', '3'); + + $this->assertSame('Saved', $result->response()->getBody()); + $this->assertSame('foo/bar/1/2/3', uri_string()); + $this->assertSame('http://example.com/index.php/foo/bar/1/2/3', current_url()); + } } diff --git a/tests/system/Test/FeatureTestTraitTest.php b/tests/system/Test/FeatureTestTraitTest.php index d4e5195b3347..0f5bb7ce33d5 100644 --- a/tests/system/Test/FeatureTestTraitTest.php +++ b/tests/system/Test/FeatureTestTraitTest.php @@ -61,6 +61,22 @@ public function testCallGet(): void $response->assertDontSee('Again'); } + public function testCallGetAndUriString(): void + { + $this->withRoutes([ + [ + 'get', + 'foo/bar/1/2/3', + static fn () => 'Hello World', + ], + ]); + $response = $this->get('foo/bar/1/2/3'); + + $this->assertSame('Hello World', $response->response()->getBody()); + $this->assertSame('foo/bar/1/2/3', uri_string()); + $this->assertSame('http://example.com/index.php/foo/bar/1/2/3', current_url()); + } + public function testClosureWithEcho() { $this->withRoutes([ From cce185c92f4c34be520c9d4515350db61201acc0 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 10 Aug 2023 10:22:08 +0900 Subject: [PATCH 32/34] docs: add upgrade_440 --- user_guide_src/source/installation/upgrade_440.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/user_guide_src/source/installation/upgrade_440.rst b/user_guide_src/source/installation/upgrade_440.rst index 52a4d45066d9..c4bdfe0c81ff 100644 --- a/user_guide_src/source/installation/upgrade_440.rst +++ b/user_guide_src/source/installation/upgrade_440.rst @@ -50,9 +50,12 @@ If your code depends on this bug, fix the segment number. Site URI Changes ================ -When your baseURL has sub-directories and you get the relative path to baseURL of -the current URI by the ``URI::getPath()`` method, you must use the new -``SiteURI::getRoutePath()`` method instead. +- Because of the rework for the current URI determination, the framework may return + site URIs or the URI paths slightly differently than in previous versions. It may + break your test code. Update assertions if the existing tests fail. +- When your baseURL has sub-directories and you get the relative path to baseURL of + the current URI by the ``URI::getPath()`` method, you must use the new + ``SiteURI::getRoutePath()`` method instead. See :ref:`v440-site-uri-changes` for details. From f35dc9bc7bddffa79f3a44f94c99b604d77c146e Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 14 Aug 2023 18:11:59 +0900 Subject: [PATCH 33/34] docs: fix typo Co-authored-by: MGatner --- system/HTTP/SiteURI.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/HTTP/SiteURI.php b/system/HTTP/SiteURI.php index 287146e64c7c..362c8ee32362 100644 --- a/system/HTTP/SiteURI.php +++ b/system/HTTP/SiteURI.php @@ -365,7 +365,7 @@ protected function applyParts(array $parts): void } /** - * For baser_url() helper. + * For base_url() helper. * * @param array|string $relativePath URI string or array of URI segments * @param string|null $scheme URI scheme. E.g., http, ftp From c6f3d57fdd51b059e810f854d8c649953dfddc4c Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 15 Aug 2023 10:19:19 +0900 Subject: [PATCH 34/34] refactor: use Services::superglobals() --- system/Test/FeatureTestTrait.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/system/Test/FeatureTestTrait.php b/system/Test/FeatureTestTrait.php index 5647430609f0..b117a1d890ae 100644 --- a/system/Test/FeatureTestTrait.php +++ b/system/Test/FeatureTestTrait.php @@ -277,7 +277,8 @@ protected function setupRequest(string $method, ?string $path = null): IncomingR $path = $parts[0]; $query = $parts[1] ?? ''; - $_SERVER['QUERY_STRING'] = $query; + $superglobals = Services::superglobals(); + $superglobals->setServer('QUERY_STRING', $query); $uri->setPath($path); $uri->setQuery($query);