From a988128db1dbb4f1da1c403f4e7720373ba4ca68 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 22 Oct 2022 18:11:04 +0900 Subject: [PATCH 1/7] test: replace Services::request() with Services::incomingrequest() These tests are for web tests. --- tests/system/CodeIgniterTest.php | 42 ++++++++++++++++---------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index 4535096ccc61..07c64af7a83c 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -78,7 +78,7 @@ public function testRunClosureRoute() $routes->add('pages/(:segment)', static function ($segment) { echo 'You want to see "' . esc($segment) . '" page.'; }); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -97,7 +97,7 @@ public function testRun404Override() $routes = Services::routes(); $routes->setAutoRoute(false); $routes->set404Override('Tests\Support\Controllers\Hello::index'); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -116,7 +116,7 @@ public function testRun404OverrideControllerReturnsResponse() $routes = Services::routes(); $routes->setAutoRoute(false); $routes->set404Override('Tests\Support\Controllers\Popcorn::pop'); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -137,7 +137,7 @@ public function testRun404OverrideByClosure() $routes->set404Override(static function () { echo '404 Override by Closure.'; }); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -157,7 +157,7 @@ public function testControllersCanReturnString() // Inject mock router. $routes = Services::routes(); $routes->add('pages/(:segment)', static fn ($segment) => 'You want to see "' . esc($segment) . '" page.'); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -182,7 +182,7 @@ public function testControllersCanReturnResponseObject() return $response->setBody($string); }); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -209,7 +209,7 @@ public function testControllersCanReturnDownloadResponseObject() return $response->download('some.txt', 'some text', true); }); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -228,9 +228,9 @@ public function testControllersRunFilterByClassName() // Inject mock router. $routes = Services::routes(); - $routes->add('pages/about', static fn () => Services::request()->getBody(), ['filter' => Customfilter::class]); + $routes->add('pages/about', static fn () => Services::incomingrequest()->getBody(), ['filter' => Customfilter::class]); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -258,7 +258,7 @@ public function testRoutesIsEmpty() $_SERVER['argc'] = 2; // Inject mock router. - $router = Services::router(null, Services::request(), false); + $router = Services::router(null, Services::incomingrequest(), false); Services::injectMock('router', $router); ob_start(); @@ -335,7 +335,7 @@ public function testRunRedirectionWithNamed() }, ['as' => 'name']); $routes->addRedirect('example', 'name'); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -358,7 +358,7 @@ public function testRunRedirectionWithURI() }); $routes->addRedirect('example', 'pages/uri'); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -382,7 +382,7 @@ public function testRunRedirectionWithURINotSet() $routes = Services::routes(); $routes->addRedirect('example', 'pages/notset'); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -405,7 +405,7 @@ public function testRunRedirectionWithHTTPCode303() $routes = Services::routes(); $routes->addRedirect('example', 'pages/notset', 301); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -422,7 +422,7 @@ public function testStoresPreviousURL() $_SERVER['argc'] = 2; // Inject mock router. - $router = Services::router(null, Services::request(), false); + $router = Services::router(null, Services::incomingrequest(), false); Services::injectMock('router', $router); ob_start(); @@ -446,7 +446,7 @@ public function testNotStoresPreviousURL() $routes = Services::routes(); $routes->addRedirect('example', 'pages/notset', 301); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -470,7 +470,7 @@ public function testNotStoresPreviousURLByCheckingContentType() return $response->setContentType('image/jpeg', ''); }); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); ob_start(); @@ -537,7 +537,7 @@ public function testSpoofRequestMethodCanUsePUT() $this->codeigniter->useSafeOutput(true)->run(); ob_get_clean(); - $this->assertSame('put', Services::request()->getMethod()); + $this->assertSame('put', Services::incomingrequest()->getMethod()); } public function testSpoofRequestMethodCannotUseGET() @@ -561,7 +561,7 @@ public function testSpoofRequestMethodCannotUseGET() $this->codeigniter->useSafeOutput(true)->run(); ob_get_clean(); - $this->assertSame('post', Services::request()->getMethod()); + $this->assertSame('post', Services::incomingrequest()->getMethod()); } /** @@ -588,7 +588,7 @@ public function testPageCacheSendSecureHeaders() return $response->setBody($string); }); - $router = Services::router($routes, Services::request()); + $router = Services::router($routes, Services::incomingrequest()); Services::injectMock('router', $router); /** @var Filters $filterConfig */ @@ -662,7 +662,7 @@ public function testPageCacheWithCacheQueryString($cacheQueryStringValue, int $e }); // Inject router - $router = Services::router($routes, Services::request(null, false)); + $router = Services::router($routes, Services::incomingrequest(null, false)); Services::injectMock('router', $router); // Cache the page output using default caching function and $cacheConfig with value from the data provider From 6d404299691d0c9d6f89769efec27ed0efd7402e Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 22 Oct 2022 18:12:01 +0900 Subject: [PATCH 2/7] docs: fix typos --- tests/system/CodeIgniterTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index 07c64af7a83c..c191b8fadaea 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -619,7 +619,7 @@ public function testPageCacheSendSecureHeaders() // Clear Page cache command('cache:clear'); - // Remove stream fliters + // Remove stream filters stream_filter_remove($outputStreamFilter); stream_filter_remove($errorStreamFilter); } @@ -654,7 +654,7 @@ public function testPageCacheWithCacheQueryString($cacheQueryStringValue, int $e $_SERVER['REQUEST_URI'] = '/' . $testingUrl; $routes = Services::routes(true); $routes->add($testingUrl, static function () { - CodeIgniter::cache(0); // Dont cache the page in the run() function because CodeIgniter class will create default $cacheConfig and overwrite settings from the dataProvider + CodeIgniter::cache(0); // Don't cache the page in the run() function because CodeIgniter class will create default $cacheConfig and overwrite settings from the dataProvider $response = Services::response(); $string = 'This is a test page, to check cache configuration'; From ec9733215a0a88f44b72ed4a41363f932d01cabf Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 22 Oct 2022 19:20:02 +0900 Subject: [PATCH 3/7] fix: CodeIgniter::run() doesn't respect $returnResponse parameter --- system/CodeIgniter.php | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 0cc4861be854..7b88acf05986 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -151,6 +151,11 @@ class CodeIgniter */ protected ?string $context = null; + /** + * Whether to return Response object or send response. + */ + protected bool $returnResponse = false; + /** * Constructor. */ @@ -291,6 +296,8 @@ protected function initializeKint() */ public function run(?RouteCollectionInterface $routes = null, bool $returnResponse = false) { + $this->returnResponse = $returnResponse; + if ($this->context === null) { throw new LogicException('Context must be set before run() is called. If you are upgrading from 4.1.x, you need to merge `public/index.php` and `spark` file from `vendor/codeigniter4/framework`.'); } @@ -309,7 +316,11 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon if ($this->request instanceof IncomingRequest && strtolower($this->request->getMethod()) === 'cli') { $this->response->setStatusCode(405)->setBody('Method Not Allowed'); - $this->sendResponse(); + if (! $this->returnResponse) { + $this->sendResponse(); + } else { + return $this->response; + } return; } @@ -345,13 +356,22 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon // If the route is a 'redirect' route, it throws // the exception with the $to as the message $this->response->redirect(base_url($e->getMessage()), 'auto', $e->getCode()); - $this->sendResponse(); + + if (! $this->returnResponse) { + $this->sendResponse(); + } else { + return $this->response; + } $this->callExit(EXIT_SUCCESS); return; } catch (PageNotFoundException $e) { - $this->display404errors($e); + $return = $this->display404errors($e); + + if ($return instanceof ResponseInterface) { + return $return; + } } } @@ -400,6 +420,8 @@ private function isWeb(): bool * * @throws PageNotFoundException * @throws RedirectException + * + * @deprecated $returnResponse is deprecated, and no longer used. */ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cacheConfig, bool $returnResponse = false) { @@ -433,7 +455,8 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache // If a ResponseInterface instance is returned then send it back to the client and stop if ($possibleResponse instanceof ResponseInterface) { - return $returnResponse ? $possibleResponse : $possibleResponse->pretend($this->useSafeOutput)->send(); + return $this->returnResponse ? $possibleResponse + : $possibleResponse->pretend($this->useSafeOutput)->send(); } if ($possibleResponse instanceof Request) { @@ -512,7 +535,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache unset($uri); - if (! $returnResponse) { + if (! $this->returnResponse) { $this->sendResponse(); } @@ -910,6 +933,8 @@ protected function runController($class) /** * Displays a 404 Page Not Found error. If set, will try to * call the 404Override controller/method that was set in routing config. + * + * @return ResponseInterface|void */ protected function display404errors(PageNotFoundException $e) { @@ -934,7 +959,11 @@ protected function display404errors(PageNotFoundException $e) $cacheConfig = new Cache(); $this->gatherOutput($cacheConfig, $returned); - $this->sendResponse(); + if (! $this->returnResponse) { + $this->sendResponse(); + } else { + return $this->response; + } return; } From ac667628da609e34038bb7febfb7e9f9a0ac9a0b Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 22 Oct 2022 19:28:24 +0900 Subject: [PATCH 4/7] test: add tests --- tests/system/CodeIgniterTest.php | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/system/CodeIgniterTest.php b/tests/system/CodeIgniterTest.php index c191b8fadaea..fee7759bb264 100644 --- a/tests/system/CodeIgniterTest.php +++ b/tests/system/CodeIgniterTest.php @@ -66,6 +66,16 @@ public function testRunEmptyDefaultRoute() $this->assertStringContainsString('Welcome to CodeIgniter', $output); } + public function testRunEmptyDefaultRouteReturnResponse() + { + $_SERVER['argv'] = ['index.php']; + $_SERVER['argc'] = 1; + + $response = $this->codeigniter->useSafeOutput(true)->run(null, true); + + $this->assertStringContainsString('Welcome to CodeIgniter', $response->getBody()); + } + public function testRunClosureRoute() { $_SERVER['argv'] = ['index.php', 'pages/about']; @@ -126,6 +136,23 @@ public function testRun404OverrideControllerReturnsResponse() $this->assertStringContainsString('Oops', $output); } + public function testRun404OverrideReturnResponse() + { + $_SERVER['argv'] = ['index.php', '/']; + $_SERVER['argc'] = 2; + + // Inject mock router. + $routes = Services::routes(); + $routes->setAutoRoute(false); + $routes->set404Override('Tests\Support\Controllers\Popcorn::pop'); + $router = Services::router($routes, Services::incomingrequest()); + Services::injectMock('router', $router); + + $response = $this->codeigniter->useSafeOutput(true)->run($routes, true); + + $this->assertStringContainsString('Oops', $response->getBody()); + } + public function testRun404OverrideByClosure() { $_SERVER['argv'] = ['index.php', '/']; From 585dbd0dd9cab838f19b2ed2f70f592352a14219 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sat, 22 Oct 2022 19:35:16 +0900 Subject: [PATCH 5/7] docs: add changelog --- user_guide_src/source/changelogs/v4.2.8.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/user_guide_src/source/changelogs/v4.2.8.rst b/user_guide_src/source/changelogs/v4.2.8.rst index 2d69f6c709ef..2c977be5a0f7 100644 --- a/user_guide_src/source/changelogs/v4.2.8.rst +++ b/user_guide_src/source/changelogs/v4.2.8.rst @@ -31,7 +31,7 @@ none. Deprecations ************ -none. +- The third parameter ``$returnResponse`` of ``CodeIgniter::handleRequest()`` is deprecated, and no longer used. Bugs Fixed ********** From 59c076f03af863338b3d3397b80e4caa3bf99b6c Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 23 Oct 2022 10:48:59 +0900 Subject: [PATCH 6/7] refactor: reverse if condition Co-authored-by: Abdul Malik Ikhsan --- system/CodeIgniter.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index 7b88acf05986..c15feec9e4a1 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -316,12 +316,12 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon if ($this->request instanceof IncomingRequest && strtolower($this->request->getMethod()) === 'cli') { $this->response->setStatusCode(405)->setBody('Method Not Allowed'); - if (! $this->returnResponse) { - $this->sendResponse(); - } else { + if ($this->returnResponse) { return $this->response; } + $this->sendResponse(); + return; } @@ -357,12 +357,12 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon // the exception with the $to as the message $this->response->redirect(base_url($e->getMessage()), 'auto', $e->getCode()); - if (! $this->returnResponse) { - $this->sendResponse(); - } else { + if ($this->returnResponse) { return $this->response; } + $this->sendResponse(); + $this->callExit(EXIT_SUCCESS); return; @@ -959,12 +959,12 @@ protected function display404errors(PageNotFoundException $e) $cacheConfig = new Cache(); $this->gatherOutput($cacheConfig, $returned); - if (! $this->returnResponse) { - $this->sendResponse(); - } else { + if ($this->returnResponse) { return $this->response; } + $this->sendResponse(); + return; } From b795d0fd32195604c5ec004dc8d63110f5544abf Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 24 Oct 2022 11:19:06 +0900 Subject: [PATCH 7/7] fix: behavior so that it does not change --- system/CodeIgniter.php | 4 +++- user_guide_src/source/changelogs/v4.2.8.rst | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/system/CodeIgniter.php b/system/CodeIgniter.php index c15feec9e4a1..a8d1c8fa7b91 100644 --- a/system/CodeIgniter.php +++ b/system/CodeIgniter.php @@ -421,10 +421,12 @@ private function isWeb(): bool * @throws PageNotFoundException * @throws RedirectException * - * @deprecated $returnResponse is deprecated, and no longer used. + * @deprecated $returnResponse is deprecated. */ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cacheConfig, bool $returnResponse = false) { + $this->returnResponse = $returnResponse; + $routeFilter = $this->tryToRouteIt($routes); $uri = $this->determinePath(); diff --git a/user_guide_src/source/changelogs/v4.2.8.rst b/user_guide_src/source/changelogs/v4.2.8.rst index 2c977be5a0f7..f8451958d2a3 100644 --- a/user_guide_src/source/changelogs/v4.2.8.rst +++ b/user_guide_src/source/changelogs/v4.2.8.rst @@ -31,7 +31,7 @@ none. Deprecations ************ -- The third parameter ``$returnResponse`` of ``CodeIgniter::handleRequest()`` is deprecated, and no longer used. +- The third parameter ``$returnResponse`` of ``CodeIgniter::handleRequest()`` is deprecated. Bugs Fixed **********