Skip to content

Commit 315f8c6

Browse files
kenjiskenjis
authored
Merge pull request #7199 from kenjis/fix-uriProtocol-QUERY_STRING
fix: routing behavior when $uriProtocol is QUERY_STRING
2 parents 4274fbf + 2063868 commit 315f8c6

6 files changed

Lines changed: 75 additions & 42 deletions

File tree

‎phpstan-baseline.neon.dist‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ parameters:
217217

218218
-
219219
message: "#^Call to an undefined method CodeIgniter\\\\Router\\\\RouteCollectionInterface\\:\\:getDefaultNamespace\\(\\)\\.$#"
220-
count: 3
220+
count: 2
221221
path: system/Router/Router.php
222222

223223
-

‎system/HTTP/IncomingRequest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ protected function parseQueryString(): string
329329
$uri = $_SERVER['QUERY_STRING'] ?? @getenv('QUERY_STRING');
330330

331331
if (trim($uri, '/') === '') {
332-
return '';
332+
return '/';
333333
}
334334

335335
if (strncmp($uri, '/', 1) === 0) {

‎system/Router/Router.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,19 +155,20 @@ public function __construct(RouteCollectionInterface $routes, ?Request $request
155155
}
156156

157157
/**
158+
* Finds the controller method corresponding to the URI.
159+
*
160+
* @param string|null $uri URI path relative to baseURL
161+
*
158162
* @return Closure|string Controller classname or Closure
159163
*
160164
* @throws PageNotFoundException
161165
* @throws RedirectException
162166
*/
163167
public function handle(?string $uri = null)
164168
{
165-
// If we cannot find a URI to match against, then
166-
// everything runs off of its default settings.
169+
// If we cannot find a URI to match against, then set it to root (`/`).
167170
if ($uri === null || $uri === '') {
168-
return strpos($this->controller, '\\') === false
169-
? $this->collection->getDefaultNamespace() . $this->controller
170-
: $this->controller;
171+
$uri = '/';
171172
}
172173

173174
// Decode URL-encoded string

‎system/Router/RouterInterface.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public function __construct(RouteCollectionInterface $routes, ?Request $request
2727
/**
2828
* Finds the controller method corresponding to the URI.
2929
*
30-
* @param string $uri
30+
* @param string|null $uri URI path relative to baseURL
3131
*
3232
* @return Closure|string Controller classname or Closure
3333
*/

‎tests/system/HTTP/IncomingRequestDetectingTest.php‎

Lines changed: 61 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -31,127 +31,158 @@ protected function setUp(): void
3131

3232
$_POST = $_GET = $_SERVER = $_REQUEST = $_ENV = $_COOKIE = $_SESSION = [];
3333

34-
$origin = 'http://www.example.com/index.php/woot?code=good#pos';
35-
34+
// The URI object is not used in detectPath().
35+
$origin = 'http://www.example.com/index.php/woot?code=good#pos';
3636
$this->request = new IncomingRequest(new App(), new URI($origin), null, new UserAgent());
3737
}
3838

3939
public function testPathDefault()
4040
{
41-
$this->request->uri = '/index.php/woot?code=good#pos';
41+
// /index.php/woot?code=good#pos
4242
$_SERVER['REQUEST_URI'] = '/index.php/woot';
4343
$_SERVER['SCRIPT_NAME'] = '/index.php';
44-
$expected = 'woot';
44+
45+
$expected = 'woot';
4546
$this->assertSame($expected, $this->request->detectPath());
4647
}
4748

48-
public function testPathEmpty()
49+
public function testPathDefaultEmpty()
4950
{
50-
$this->request->uri = '/';
51+
// /
5152
$_SERVER['REQUEST_URI'] = '/';
5253
$_SERVER['SCRIPT_NAME'] = '/index.php';
53-
$expected = '/';
54+
55+
$expected = '/';
5456
$this->assertSame($expected, $this->request->detectPath());
5557
}
5658

5759
public function testPathRequestURI()
5860
{
59-
$this->request->uri = '/index.php/woot?code=good#pos';
61+
// /index.php/woot?code=good#pos
6062
$_SERVER['REQUEST_URI'] = '/index.php/woot';
6163
$_SERVER['SCRIPT_NAME'] = '/index.php';
62-
$expected = 'woot';
64+
65+
$expected = 'woot';
6366
$this->assertSame($expected, $this->request->detectPath('REQUEST_URI'));
6467
}
6568

6669
public function testPathRequestURINested()
6770
{
68-
$this->request->uri = '/ci/index.php/woot?code=good#pos';
71+
// I'm not sure but this is a case of Apache config making such SERVER
72+
// values?
73+
// The current implementation doesn't use the value of the URI object.
74+
// So I removed the code to set URI. Therefore, it's exactly the same as
75+
// the method above as a test.
76+
// But it may be changed in the future to use the value of the URI object.
77+
// So I don't remove this test case.
78+
79+
// /ci/index.php/woot?code=good#pos
6980
$_SERVER['REQUEST_URI'] = '/index.php/woot';
7081
$_SERVER['SCRIPT_NAME'] = '/index.php';
71-
$expected = 'woot';
82+
83+
$expected = 'woot';
7284
$this->assertSame($expected, $this->request->detectPath('REQUEST_URI'));
7385
}
7486

7587
public function testPathRequestURISubfolder()
7688
{
77-
$this->request->uri = '/ci/index.php/popcorn/woot?code=good#pos';
89+
// /ci/index.php/popcorn/woot?code=good#pos
7890
$_SERVER['REQUEST_URI'] = '/ci/index.php/popcorn/woot';
7991
$_SERVER['SCRIPT_NAME'] = '/ci/index.php';
80-
$expected = 'popcorn/woot';
92+
93+
$expected = 'popcorn/woot';
8194
$this->assertSame($expected, $this->request->detectPath('REQUEST_URI'));
8295
}
8396

8497
public function testPathRequestURINoIndex()
8598
{
86-
$this->request->uri = '/sub/example';
99+
// /sub/example
87100
$_SERVER['REQUEST_URI'] = '/sub/example';
88101
$_SERVER['SCRIPT_NAME'] = '/sub/index.php';
89-
$expected = 'example';
102+
103+
$expected = 'example';
90104
$this->assertSame($expected, $this->request->detectPath('REQUEST_URI'));
91105
}
92106

93107
public function testPathRequestURINginx()
94108
{
95-
$this->request->uri = '/ci/index.php/woot?code=good#pos';
109+
// /ci/index.php/woot?code=good#pos
96110
$_SERVER['REQUEST_URI'] = '/index.php/woot?code=good';
97111
$_SERVER['SCRIPT_NAME'] = '/index.php';
98-
$expected = 'woot';
112+
113+
$expected = 'woot';
99114
$this->assertSame($expected, $this->request->detectPath('REQUEST_URI'));
100115
}
101116

102117
public function testPathRequestURINginxRedirecting()
103118
{
104-
$this->request->uri = '/?/ci/index.php/woot';
119+
// /?/ci/index.php/woot
105120
$_SERVER['REQUEST_URI'] = '/?/ci/woot';
106121
$_SERVER['SCRIPT_NAME'] = '/index.php';
107-
$expected = 'ci/woot';
122+
123+
$expected = 'ci/woot';
108124
$this->assertSame($expected, $this->request->detectPath('REQUEST_URI'));
109125
}
110126

111127
public function testPathRequestURISuppressed()
112128
{
113-
$this->request->uri = '/woot?code=good#pos';
129+
// /woot?code=good#pos
114130
$_SERVER['REQUEST_URI'] = '/woot';
115131
$_SERVER['SCRIPT_NAME'] = '/';
116-
$expected = 'woot';
132+
133+
$expected = 'woot';
117134
$this->assertSame($expected, $this->request->detectPath('REQUEST_URI'));
118135
}
119136

120137
public function testPathQueryString()
121138
{
122-
$this->request->uri = '/?/ci/index.php/woot';
123-
$_SERVER['REQUEST_URI'] = '/?/ci/woot';
139+
// /index.php?/ci/woot
140+
$_SERVER['REQUEST_URI'] = '/index.php?/ci/woot';
124141
$_SERVER['QUERY_STRING'] = '/ci/woot';
125142
$_SERVER['SCRIPT_NAME'] = '/index.php';
126-
$expected = 'ci/woot';
143+
144+
$expected = 'ci/woot';
145+
$this->assertSame($expected, $this->request->detectPath('QUERY_STRING'));
146+
}
147+
148+
public function testPathQueryStringWithQueryString()
149+
{
150+
// /index.php?/ci/woot?code=good#pos
151+
$_SERVER['REQUEST_URI'] = '/index.php?/ci/woot?code=good';
152+
$_SERVER['QUERY_STRING'] = '/ci/woot?code=good';
153+
$_SERVER['SCRIPT_NAME'] = '/index.php';
154+
155+
$expected = 'ci/woot';
127156
$this->assertSame($expected, $this->request->detectPath('QUERY_STRING'));
128157
}
129158

130159
public function testPathQueryStringEmpty()
131160
{
132-
$this->request->uri = '/?/ci/index.php/woot';
133-
$_SERVER['REQUEST_URI'] = '/?/ci/woot';
161+
// /index.php?
162+
$_SERVER['REQUEST_URI'] = '/index.php?';
134163
$_SERVER['QUERY_STRING'] = '';
135164
$_SERVER['SCRIPT_NAME'] = '/index.php';
136-
$expected = '';
165+
166+
$expected = '/';
137167
$this->assertSame($expected, $this->request->detectPath('QUERY_STRING'));
138168
}
139169

140170
public function testPathPathInfo()
141171
{
142-
$this->request->uri = '/index.php/woot?code=good#pos';
172+
// /index.php/woot?code=good#pos
143173
$this->request->setGlobal('server', [
144174
'PATH_INFO' => null,
145175
]);
146176
$_SERVER['REQUEST_URI'] = '/index.php/woot';
147177
$_SERVER['SCRIPT_NAME'] = '/index.php';
148-
$expected = 'woot';
178+
179+
$expected = 'woot';
149180
$this->assertSame($expected, $this->request->detectPath('PATH_INFO'));
150181
}
151182

152183
public function testPathPathInfoGlobal()
153184
{
154-
$this->request->uri = '/index.php/woot?code=good#pos';
185+
// /index.php/woot?code=good#pos
155186
$this->request->setGlobal('server', [
156187
'PATH_INFO' => 'silliness',
157188
]);

‎tests/system/Router/RouterTest.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ protected function setUp(): void
3939
$this->collection = new RouteCollection(Services::locator(), $moduleConfig);
4040

4141
$routes = [
42+
'/' => 'Home::index',
4243
'users' => 'Users::index',
4344
'user-setting/show-list' => 'User_setting::show_list',
4445
'user-setting/(:segment)' => 'User_setting::detail/$1',
@@ -56,20 +57,20 @@ protected function setUp(): void
5657
'objects/(:segment)/sort/(:segment)/([A-Z]{3,7})' => 'AdminList::objectsSortCreate/$1/$2/$3',
5758
'(:segment)/(:segment)/(:segment)' => '$2::$3/$1',
5859
];
59-
6060
$this->collection->map($routes);
61+
6162
$this->request = Services::request();
6263
$this->request->setMethod('get');
6364
}
6465

65-
public function testEmptyURIMatchesDefaults()
66+
public function testEmptyURIMatchesRoot()
6667
{
6768
$router = new Router($this->collection, $this->request);
6869

6970
$router->handle('');
7071

71-
$this->assertSame($this->collection->getDefaultController(), $router->controllerName());
72-
$this->assertSame($this->collection->getDefaultMethod(), $router->methodName());
72+
$this->assertSame('\Home', $router->controllerName());
73+
$this->assertSame('index', $router->methodName());
7374
}
7475

7576
public function testZeroAsURIPath()

0 commit comments

Comments
 (0)