Skip to content

Commit 76e2dac

Browse files
kenjiskenjis
authored
Merge pull request #6737 from kenjis/fix-CodeIgniter-run-returnResponse
fix: CodeIgniter::run() doesn't respect $returnResponse
2 parents e0779d7 + b795d0f commit 76e2dac

File tree

3 files changed

+85
-27
lines changed

3 files changed

+85
-27
lines changed

‎system/CodeIgniter.php‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,11 @@ class CodeIgniter
151151
*/
152152
protected ?string $context = null;
153153

154+
/**
155+
* Whether to return Response object or send response.
156+
*/
157+
protected bool $returnResponse = false;
158+
154159
/**
155160
* Constructor.
156161
*/
@@ -291,6 +296,8 @@ protected function initializeKint()
291296
*/
292297
public function run(?RouteCollectionInterface $routes = null, bool $returnResponse = false)
293298
{
299+
$this->returnResponse = $returnResponse;
300+
294301
if ($this->context === null) {
295302
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`.');
296303
}
@@ -309,6 +316,10 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon
309316
if ($this->request instanceof IncomingRequest && strtolower($this->request->getMethod()) === 'cli') {
310317
$this->response->setStatusCode(405)->setBody('Method Not Allowed');
311318

319+
if ($this->returnResponse) {
320+
return $this->response;
321+
}
322+
312323
$this->sendResponse();
313324

314325
return;
@@ -345,13 +356,22 @@ public function run(?RouteCollectionInterface $routes = null, bool $returnRespon
345356
// If the route is a 'redirect' route, it throws
346357
// the exception with the $to as the message
347358
$this->response->redirect(base_url($e->getMessage()), 'auto', $e->getCode());
359+
360+
if ($this->returnResponse) {
361+
return $this->response;
362+
}
363+
348364
$this->sendResponse();
349365

350366
$this->callExit(EXIT_SUCCESS);
351367

352368
return;
353369
} catch (PageNotFoundException $e) {
354-
$this->display404errors($e);
370+
$return = $this->display404errors($e);
371+
372+
if ($return instanceof ResponseInterface) {
373+
return $return;
374+
}
355375
}
356376
}
357377

@@ -400,9 +420,13 @@ private function isWeb(): bool
400420
*
401421
* @throws PageNotFoundException
402422
* @throws RedirectException
423+
*
424+
* @deprecated $returnResponse is deprecated.
403425
*/
404426
protected function handleRequest(?RouteCollectionInterface $routes, Cache $cacheConfig, bool $returnResponse = false)
405427
{
428+
$this->returnResponse = $returnResponse;
429+
406430
$routeFilter = $this->tryToRouteIt($routes);
407431

408432
$uri = $this->determinePath();
@@ -433,7 +457,8 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
433457

434458
// If a ResponseInterface instance is returned then send it back to the client and stop
435459
if ($possibleResponse instanceof ResponseInterface) {
436-
return $returnResponse ? $possibleResponse : $possibleResponse->pretend($this->useSafeOutput)->send();
460+
return $this->returnResponse ? $possibleResponse
461+
: $possibleResponse->pretend($this->useSafeOutput)->send();
437462
}
438463

439464
if ($possibleResponse instanceof Request) {
@@ -512,7 +537,7 @@ protected function handleRequest(?RouteCollectionInterface $routes, Cache $cache
512537

513538
unset($uri);
514539

515-
if (! $returnResponse) {
540+
if (! $this->returnResponse) {
516541
$this->sendResponse();
517542
}
518543

@@ -910,6 +935,8 @@ protected function runController($class)
910935
/**
911936
* Displays a 404 Page Not Found error. If set, will try to
912937
* call the 404Override controller/method that was set in routing config.
938+
*
939+
* @return ResponseInterface|void
913940
*/
914941
protected function display404errors(PageNotFoundException $e)
915942
{
@@ -934,6 +961,10 @@ protected function display404errors(PageNotFoundException $e)
934961

935962
$cacheConfig = new Cache();
936963
$this->gatherOutput($cacheConfig, $returned);
964+
if ($this->returnResponse) {
965+
return $this->response;
966+
}
967+
937968
$this->sendResponse();
938969

939970
return;

‎tests/system/CodeIgniterTest.php‎

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,16 @@ public function testRunEmptyDefaultRoute()
6666
$this->assertStringContainsString('Welcome to CodeIgniter', $output);
6767
}
6868

69+
public function testRunEmptyDefaultRouteReturnResponse()
70+
{
71+
$_SERVER['argv'] = ['index.php'];
72+
$_SERVER['argc'] = 1;
73+
74+
$response = $this->codeigniter->useSafeOutput(true)->run(null, true);
75+
76+
$this->assertStringContainsString('Welcome to CodeIgniter', $response->getBody());
77+
}
78+
6979
public function testRunClosureRoute()
7080
{
7181
$_SERVER['argv'] = ['index.php', 'pages/about'];
@@ -78,7 +88,7 @@ public function testRunClosureRoute()
7888
$routes->add('pages/(:segment)', static function ($segment) {
7989
echo 'You want to see "' . esc($segment) . '" page.';
8090
});
81-
$router = Services::router($routes, Services::request());
91+
$router = Services::router($routes, Services::incomingrequest());
8292
Services::injectMock('router', $router);
8393

8494
ob_start();
@@ -97,7 +107,7 @@ public function testRun404Override()
97107
$routes = Services::routes();
98108
$routes->setAutoRoute(false);
99109
$routes->set404Override('Tests\Support\Controllers\Hello::index');
100-
$router = Services::router($routes, Services::request());
110+
$router = Services::router($routes, Services::incomingrequest());
101111
Services::injectMock('router', $router);
102112

103113
ob_start();
@@ -116,7 +126,7 @@ public function testRun404OverrideControllerReturnsResponse()
116126
$routes = Services::routes();
117127
$routes->setAutoRoute(false);
118128
$routes->set404Override('Tests\Support\Controllers\Popcorn::pop');
119-
$router = Services::router($routes, Services::request());
129+
$router = Services::router($routes, Services::incomingrequest());
120130
Services::injectMock('router', $router);
121131

122132
ob_start();
@@ -126,6 +136,23 @@ public function testRun404OverrideControllerReturnsResponse()
126136
$this->assertStringContainsString('Oops', $output);
127137
}
128138

139+
public function testRun404OverrideReturnResponse()
140+
{
141+
$_SERVER['argv'] = ['index.php', '/'];
142+
$_SERVER['argc'] = 2;
143+
144+
// Inject mock router.
145+
$routes = Services::routes();
146+
$routes->setAutoRoute(false);
147+
$routes->set404Override('Tests\Support\Controllers\Popcorn::pop');
148+
$router = Services::router($routes, Services::incomingrequest());
149+
Services::injectMock('router', $router);
150+
151+
$response = $this->codeigniter->useSafeOutput(true)->run($routes, true);
152+
153+
$this->assertStringContainsString('Oops', $response->getBody());
154+
}
155+
129156
public function testRun404OverrideByClosure()
130157
{
131158
$_SERVER['argv'] = ['index.php', '/'];
@@ -137,7 +164,7 @@ public function testRun404OverrideByClosure()
137164
$routes->set404Override(static function () {
138165
echo '404 Override by Closure.';
139166
});
140-
$router = Services::router($routes, Services::request());
167+
$router = Services::router($routes, Services::incomingrequest());
141168
Services::injectMock('router', $router);
142169

143170
ob_start();
@@ -157,7 +184,7 @@ public function testControllersCanReturnString()
157184
// Inject mock router.
158185
$routes = Services::routes();
159186
$routes->add('pages/(:segment)', static fn ($segment) => 'You want to see "' . esc($segment) . '" page.');
160-
$router = Services::router($routes, Services::request());
187+
$router = Services::router($routes, Services::incomingrequest());
161188
Services::injectMock('router', $router);
162189

163190
ob_start();
@@ -182,7 +209,7 @@ public function testControllersCanReturnResponseObject()
182209

183210
return $response->setBody($string);
184211
});
185-
$router = Services::router($routes, Services::request());
212+
$router = Services::router($routes, Services::incomingrequest());
186213
Services::injectMock('router', $router);
187214

188215
ob_start();
@@ -209,7 +236,7 @@ public function testControllersCanReturnDownloadResponseObject()
209236

210237
return $response->download('some.txt', 'some text', true);
211238
});
212-
$router = Services::router($routes, Services::request());
239+
$router = Services::router($routes, Services::incomingrequest());
213240
Services::injectMock('router', $router);
214241

215242
ob_start();
@@ -228,9 +255,9 @@ public function testControllersRunFilterByClassName()
228255

229256
// Inject mock router.
230257
$routes = Services::routes();
231-
$routes->add('pages/about', static fn () => Services::request()->getBody(), ['filter' => Customfilter::class]);
258+
$routes->add('pages/about', static fn () => Services::incomingrequest()->getBody(), ['filter' => Customfilter::class]);
232259

233-
$router = Services::router($routes, Services::request());
260+
$router = Services::router($routes, Services::incomingrequest());
234261
Services::injectMock('router', $router);
235262

236263
ob_start();
@@ -258,7 +285,7 @@ public function testRoutesIsEmpty()
258285
$_SERVER['argc'] = 2;
259286

260287
// Inject mock router.
261-
$router = Services::router(null, Services::request(), false);
288+
$router = Services::router(null, Services::incomingrequest(), false);
262289
Services::injectMock('router', $router);
263290

264291
ob_start();
@@ -335,7 +362,7 @@ public function testRunRedirectionWithNamed()
335362
}, ['as' => 'name']);
336363
$routes->addRedirect('example', 'name');
337364

338-
$router = Services::router($routes, Services::request());
365+
$router = Services::router($routes, Services::incomingrequest());
339366
Services::injectMock('router', $router);
340367

341368
ob_start();
@@ -358,7 +385,7 @@ public function testRunRedirectionWithURI()
358385
});
359386
$routes->addRedirect('example', 'pages/uri');
360387

361-
$router = Services::router($routes, Services::request());
388+
$router = Services::router($routes, Services::incomingrequest());
362389
Services::injectMock('router', $router);
363390

364391
ob_start();
@@ -382,7 +409,7 @@ public function testRunRedirectionWithURINotSet()
382409
$routes = Services::routes();
383410
$routes->addRedirect('example', 'pages/notset');
384411

385-
$router = Services::router($routes, Services::request());
412+
$router = Services::router($routes, Services::incomingrequest());
386413
Services::injectMock('router', $router);
387414

388415
ob_start();
@@ -405,7 +432,7 @@ public function testRunRedirectionWithHTTPCode303()
405432
$routes = Services::routes();
406433
$routes->addRedirect('example', 'pages/notset', 301);
407434

408-
$router = Services::router($routes, Services::request());
435+
$router = Services::router($routes, Services::incomingrequest());
409436
Services::injectMock('router', $router);
410437

411438
ob_start();
@@ -422,7 +449,7 @@ public function testStoresPreviousURL()
422449
$_SERVER['argc'] = 2;
423450

424451
// Inject mock router.
425-
$router = Services::router(null, Services::request(), false);
452+
$router = Services::router(null, Services::incomingrequest(), false);
426453
Services::injectMock('router', $router);
427454

428455
ob_start();
@@ -446,7 +473,7 @@ public function testNotStoresPreviousURL()
446473
$routes = Services::routes();
447474
$routes->addRedirect('example', 'pages/notset', 301);
448475

449-
$router = Services::router($routes, Services::request());
476+
$router = Services::router($routes, Services::incomingrequest());
450477
Services::injectMock('router', $router);
451478

452479
ob_start();
@@ -470,7 +497,7 @@ public function testNotStoresPreviousURLByCheckingContentType()
470497

471498
return $response->setContentType('image/jpeg', '');
472499
});
473-
$router = Services::router($routes, Services::request());
500+
$router = Services::router($routes, Services::incomingrequest());
474501
Services::injectMock('router', $router);
475502

476503
ob_start();
@@ -537,7 +564,7 @@ public function testSpoofRequestMethodCanUsePUT()
537564
$this->codeigniter->useSafeOutput(true)->run();
538565
ob_get_clean();
539566

540-
$this->assertSame('put', Services::request()->getMethod());
567+
$this->assertSame('put', Services::incomingrequest()->getMethod());
541568
}
542569

543570
public function testSpoofRequestMethodCannotUseGET()
@@ -561,7 +588,7 @@ public function testSpoofRequestMethodCannotUseGET()
561588
$this->codeigniter->useSafeOutput(true)->run();
562589
ob_get_clean();
563590

564-
$this->assertSame('post', Services::request()->getMethod());
591+
$this->assertSame('post', Services::incomingrequest()->getMethod());
565592
}
566593

567594
/**
@@ -588,7 +615,7 @@ public function testPageCacheSendSecureHeaders()
588615

589616
return $response->setBody($string);
590617
});
591-
$router = Services::router($routes, Services::request());
618+
$router = Services::router($routes, Services::incomingrequest());
592619
Services::injectMock('router', $router);
593620

594621
/** @var Filters $filterConfig */
@@ -619,7 +646,7 @@ public function testPageCacheSendSecureHeaders()
619646
// Clear Page cache
620647
command('cache:clear');
621648

622-
// Remove stream fliters
649+
// Remove stream filters
623650
stream_filter_remove($outputStreamFilter);
624651
stream_filter_remove($errorStreamFilter);
625652
}
@@ -654,15 +681,15 @@ public function testPageCacheWithCacheQueryString($cacheQueryStringValue, int $e
654681
$_SERVER['REQUEST_URI'] = '/' . $testingUrl;
655682
$routes = Services::routes(true);
656683
$routes->add($testingUrl, static function () {
657-
CodeIgniter::cache(0); // Dont cache the page in the run() function because CodeIgniter class will create default $cacheConfig and overwrite settings from the dataProvider
684+
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
658685
$response = Services::response();
659686
$string = 'This is a test page, to check cache configuration';
660687

661688
return $response->setBody($string);
662689
});
663690

664691
// Inject router
665-
$router = Services::router($routes, Services::request(null, false));
692+
$router = Services::router($routes, Services::incomingrequest(null, false));
666693
Services::injectMock('router', $router);
667694

668695
// Cache the page output using default caching function and $cacheConfig with value from the data provider

‎user_guide_src/source/changelogs/v4.2.8.rst‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ none.
3131
Deprecations
3232
************
3333

34-
none.
34+
- The third parameter ``$returnResponse`` of ``CodeIgniter::handleRequest()`` is deprecated.
3535

3636
Bugs Fixed
3737
**********

0 commit comments

Comments
 (0)