Skip to content

Commit b279e4d

Browse files
lonnieezelllonnieezell
committed
Refactor the way the router and route collection determine the current HTTP Verb. Closes #2022 Fixes #2021 
1 parent d478c94 commit b279e4d

12 files changed

Lines changed: 152 additions & 137 deletions

File tree

‎system/CodeIgniter.php‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ public function getPerformanceStats(): array
642642
*/
643643
protected function generateCacheName($config): string
644644
{
645-
if (is_cli() && ! (ENVIRONMENT === 'testing'))
645+
if (get_class($this->request) === CLIRequest::class)
646646
{
647647
return md5($this->request->getPath());
648648
}
@@ -704,7 +704,7 @@ protected function tryToRouteIt(RouteCollectionInterface $routes = null)
704704
}
705705

706706
// $routes is defined in Config/Routes.php
707-
$this->router = Services::router($routes);
707+
$this->router = Services::router($routes, $this->request);
708708

709709
$path = $this->determinePath();
710710

@@ -985,17 +985,9 @@ public function storePreviousURL($uri)
985985
/**
986986
* Modifies the Request Object to use a different method if a POST
987987
* variable called _method is found.
988-
*
989988
*/
990989
public function spoofRequestMethod()
991990
{
992-
// CLI commands always use 'cli' method
993-
if (is_cli())
994-
{
995-
$this->request->setMethod('cli');
996-
return;
997-
}
998-
999991
// Only works with POSTED forms
1000992
if ($this->request->getMethod() !== 'post')
1001993
{

‎system/Config/Services.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
use CodeIgniter\HTTP\IncomingRequest;
5151
use CodeIgniter\HTTP\Negotiate;
5252
use CodeIgniter\HTTP\RedirectResponse;
53+
use CodeIgniter\HTTP\Request;
5354
use CodeIgniter\HTTP\RequestInterface;
5455
use CodeIgniter\HTTP\Response;
5556
use CodeIgniter\HTTP\ResponseInterface;
@@ -640,23 +641,24 @@ public static function routes(bool $getShared = true)
640641
* the correct Controller and Method to execute.
641642
*
642643
* @param \CodeIgniter\Router\RouteCollectionInterface $routes
644+
* @param \CodeIgniter\HTTP\Request $request
643645
* @param boolean $getShared
644646
*
645647
* @return \CodeIgniter\Router\Router
646648
*/
647-
public static function router(RouteCollectionInterface $routes = null, bool $getShared = true)
649+
public static function router(RouteCollectionInterface $routes = null, Request $request = null, bool $getShared = true)
648650
{
649651
if ($getShared)
650652
{
651-
return static::getSharedInstance('router', $routes);
653+
return static::getSharedInstance('router', $routes, $request);
652654
}
653655

654656
if (empty($routes))
655657
{
656658
$routes = static::routes(true);
657659
}
658660

659-
return new Router($routes);
661+
return new Router($routes, $request);
660662
}
661663

662664
//--------------------------------------------------------------------

‎system/HTTP/CLIRequest.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,13 @@ class CLIRequest extends Request
7171
*/
7272
protected $options = [];
7373

74+
/**
75+
* Set the expected HTTP verb
76+
*
77+
* @var string
78+
*/
79+
protected $method = 'cli';
80+
7481
//--------------------------------------------------------------------
7582

7683
/**

‎system/HTTP/Request.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ public function __construct($config)
8484
{
8585
$this->proxyIPs = $config->proxyIPs;
8686

87-
$this->method = $this->getServer('REQUEST_METHOD') ?? 'GET';
87+
if (empty($this->method))
88+
{
89+
$this->method = $this->getServer('REQUEST_METHOD') ?? 'GET';
90+
}
8891
}
8992

9093
//--------------------------------------------------------------------

‎system/Router/RouteCollection.php‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939

4040
namespace CodeIgniter\Router;
4141

42+
use CodeIgniter\HTTP\Request;
4243
use Config\Services;
4344
use CodeIgniter\Autoloader\FileLocator;
4445
use CodeIgniter\Router\Exceptions\RouterException;
@@ -234,11 +235,7 @@ class RouteCollection implements RouteCollectionInterface
234235
*/
235236
public function __construct(FileLocator $locator, $moduleConfig)
236237
{
237-
// Get HTTP verb from current request (accounts for spoofing)
238-
$this->HTTPVerb = Services::request()->getMethod();
239-
240-
$this->fileLocator = $locator;
241-
238+
$this->fileLocator = $locator;
242239
$this->moduleConfig = $moduleConfig;
243240
}
244241

@@ -1116,7 +1113,7 @@ public function reverseRoute(string $search, ...$params)
11161113
{
11171114
$from = key($route['route']);
11181115
$to = $route['route'][$from];
1119-
1116+
11201117
// ignore closures
11211118
if (! is_string($to))
11221119
{

‎system/Router/Router.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838

3939
namespace CodeIgniter\Router;
4040

41+
use CodeIgniter\HTTP\Request;
4142
use CodeIgniter\Exceptions\PageNotFoundException;
4243
use CodeIgniter\Router\Exceptions\RedirectException;
4344
use CodeIgniter\Router\Exceptions\RouterException;
@@ -135,13 +136,16 @@ class Router implements RouterInterface
135136
* Stores a reference to the RouteCollection object.
136137
*
137138
* @param RouteCollectionInterface $routes
139+
* @param Request $request
138140
*/
139-
public function __construct(RouteCollectionInterface $routes)
141+
public function __construct(RouteCollectionInterface $routes, Request $request = null)
140142
{
141143
$this->collection = $routes;
142144

143145
$this->controller = $this->collection->getDefaultController();
144146
$this->method = $this->collection->getDefaultMethod();
147+
148+
$this->collection->setHTTPVerb($request->getMethod() ?? 'get');
145149
}
146150

147151
//--------------------------------------------------------------------

‎system/Router/RouterInterface.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838

3939
namespace CodeIgniter\Router;
4040

41+
use CodeIgniter\HTTP\Request;
42+
4143
/**
4244
* Expected behavior of a Router.
4345
*/
@@ -47,9 +49,10 @@ interface RouterInterface
4749
/**
4850
* Stores a reference to the RouteCollection object.
4951
*
50-
* @param RouteCollectionInterface $routes
52+
* @param RouteCollectionInterface $routes
53+
* @param \CodeIgniter\HTTP\Request $request
5154
*/
52-
public function __construct(RouteCollectionInterface $routes);
55+
public function __construct(RouteCollectionInterface $routes, Request $request = null);
5356

5457
//--------------------------------------------------------------------
5558

‎tests/system/CLI/ConsoleTest.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<?php namespace CodeIgniter\CLI;
22

3+
use CodeIgniter\HTTP\CLIRequest;
34
use Tests\Support\MockCodeIgniter;
45
use Tests\Support\Config\MockCLIConfig;
56
use CodeIgniter\Test\Filters\CITestStreamFilter;
@@ -56,6 +57,9 @@ public function testHeader()
5657

5758
public function testRun()
5859
{
60+
$request = new CLIRequest(config('App'));
61+
$this->app->setRequest($request);
62+
5963
$console = new \CodeIgniter\CLI\Console($this->app);
6064
$console->run(true);
6165
$result = CITestStreamFilter::$buffer;

‎tests/system/CodeIgniterTest.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public function testRunClosureRoute()
9090
$routes->add('pages/(:segment)', function ($segment) {
9191
echo 'You want to see "' . esc($segment) . '" page.';
9292
});
93-
$router = Services::router($routes);
93+
$router = Services::router($routes, Services::request());
9494
Services::injectMock('router', $router);
9595

9696
ob_start();
@@ -114,7 +114,7 @@ public function testRun404Override()
114114
$routes = Services::routes();
115115
$routes->setAutoRoute(false);
116116
$routes->set404Override('Home::index');
117-
$router = Services::router($routes);
117+
$router = Services::router($routes, Services::request());
118118
Services::injectMock('router', $router);
119119

120120
ob_start();
@@ -140,7 +140,7 @@ public function testRun404OverrideByClosure()
140140
$routes->set404Override(function () {
141141
echo '404 Override by Closure.';
142142
});
143-
$router = Services::router($routes);
143+
$router = Services::router($routes, Services::request());
144144
Services::injectMock('router', $router);
145145

146146
ob_start();
@@ -166,7 +166,7 @@ public function testControllersCanReturnString()
166166
$routes->add('pages/(:segment)', function ($segment) {
167167
return 'You want to see "' . esc($segment) . '" page.';
168168
});
169-
$router = Services::router($routes);
169+
$router = Services::router($routes, Services::request());
170170
Services::injectMock('router', $router);
171171

172172
ob_start();
@@ -194,7 +194,7 @@ public function testControllersCanReturnResponseObject()
194194
$string = "You want to see 'about' page.";
195195
return $response->setBody($string);
196196
});
197-
$router = Services::router($routes);
197+
$router = Services::router($routes, Services::request());
198198
Services::injectMock('router', $router);
199199

200200
ob_start();
@@ -230,7 +230,7 @@ public function testRoutesIsEmpty()
230230
$_SERVER['argc'] = 2;
231231

232232
// Inject mock router.
233-
$router = Services::router(null, false);
233+
$router = Services::router(null, Services::request(), false);
234234
Services::injectMock('router', $router);
235235

236236
ob_start();

‎tests/system/HTTP/CLIRequestTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -626,9 +626,9 @@ public function testGetIPAddressThruProxyOutofSubnet()
626626

627627
public function testMethodReturnsRightStuff()
628628
{
629-
// Defaults method to GET now.
630-
$this->assertEquals('get', $this->request->getMethod());
631-
$this->assertEquals('GET', $this->request->getMethod(true));
629+
// Defaults method to CLI now.
630+
$this->assertEquals('cli', $this->request->getMethod());
631+
$this->assertEquals('CLI', $this->request->getMethod(true));
632632
}
633633

634634
}

0 commit comments

Comments
 (0)