8000 bug #13039 [HttpFoundation] [Request] fix baseUrl parsing to fix wron… · symfony/symfony@b8e4b4a · GitHub
[go: up one dir, main page]
Skip to content

Commit b8e4b4a

Browse files
fabpotfabpot
committed
bug #13039 [HttpFoundation] [Request] fix baseUrl parsing to fix wrong path_info (rk3rn3r)
This PR was squashed before being merged into the 2.3 branch (closes #13039). Discussion ---------- [HttpFoundation] [Request] fix baseUrl parsing to fix wrong path_info Hi everyone! We at trivago had an issue with the Request object. It seems that all versions of symfony 2.x and 3.x are affected from this (possible) bug (don't checked 1.x). Here is the problem: some old legacy pages are deployed in the Document Root, let's say /var/www/www.test.com/ . one or more new applications based on symfony are deployed to /var/release/new_app1/ , /var/release/new_app2/ , ... . in /var/www/www.test.com/ there is a symlink "app" to /var/release/new_app1/web, like: /var/www/www.test.com/app --> /var/release/new_app1/web/ there is a "SEO"/human-readable rewrite rule for Document Root (if called path/file not exist): (.*) --> app/app.php the problem comes, when the user calls a uri starting with "app" or whatever the rewrite rule / symlink points to: the user calls "http://www.test.com/apparthotel-1234" results in $_SERVER parameters like this ``` 'DOCUMENT_ROOT' =>'/var/www/www.test.com', 'SCRIPT_FILENAME' => '/var/www/www.test.com/app/app.php', 'SCRIPT_NAME' => '/app/app.php', 'PHP_SELF' => '/app/app.php/apparthotel-1234' ``` in Request::prepareBaseUrl() there are checks to find the baseUrl: ``` if ($baseUrl && false !== $prefix = $this->getUrlencodedPrefix($requestUri, $baseUrl)) { // full $baseUrl matches return $prefix; } if ($baseUrl && false !== $prefix = $this->getUrlencodedPrefix($requestUri, dirname($baseUrl))) { // directory portion of $baseUrl matches return rtrim($prefix, '/'); } ``` first it is checked if (in our case) "/app/app.php" is in the request uri (/apparthotel-1234). it's not. then it takes the dirname (of /app/app.php) which is /app and checks if it is in the request uri (/apparthotel-1234), and YES, it is! and "/app" is returned, but this is wrong, it should be empty (because it comes from a rewrite rule from root: /)! later in preparePathInfo(), if there is a baseUrl, then the baseUrl is removed from the request uri: /apparthotel-1234 ---> /arthotel-1234 The cause is, the second baseUrl check, checks if the path of the application is already in the uri, like when the request was "http://www.test.com/app/apparthotel-1234" and hit a rewrite rule like (.*) --> app.php in there, but because it matches a directory it must match "dirname($baseUrl) . '/'". I also needed to fix one unit test of the getBaseUrl test: the request uri recently was "/foo%20bar". but from the $_SERVER infos "foo bar" is a directory, see: ``` 'SCRIPT_FILENAME' => '/home/John Doe/public_html/foo bar/app.php', 'SCRIPT_NAME' => '/foo bar/app.php', 'PHP_SELF' => '/foo bar/app.php', ``` webservers will redirect a request "http://www.test.com/foo%20bar" to "http://www.test.com/foo%20bar/" when "foo bar" is a directory. checked this for apache 2.x and nginx 1.4.x. this fix is for symfony master (3.0.x, see #13039). I also prepared a merge request for actual 2.7 branch, it will also follow in some minutes. (see #13040) | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | this, #13040, #13038, #7329 | License | MIT [HttpFoundation] [Request] * added missing slash to baseUrl-path part check to remove the path, only when it's also a path in the uri [HttpFoundation] [Tests] [RequestTest] * fixed and added unittests This is the symfony 2.3 branch fix for the issue related to #13038 and #13040 Happy christmas! Commits ------- 3a3ecd3 [HttpFoundation] [Request] fix baseUrl parsing to fix wrong path_info
2 parents 94e8e03 + 3a3ecd3 commit b8e4b4a

File tree

2 files changed

+17
-2
lines changed

2 files changed

+17
-2
lines changed

‎src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1712,7 +1712,7 @@ protected function prepareBaseUrl()
17121712
return $prefix;
17131713
}
17141714

1715-
if ($baseUrl && false !== $prefix = $this->getUrlencodedPrefix($requestUri, dirname($baseUrl))) {
1715+
if ($baseUrl && false !== $prefix = $this->getUrlencodedPrefix($requestUri, dirname($baseUrl).'/')) {
17161716
// directory portion of $baseUrl matches
17171717
return rtrim($prefix, '/');
17181718
}

‎src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,21 @@ public function testCreate()
223223
$request = Request::create('http://test.com/?foo');
224224
$this->assertEquals('/?foo', $request->getRequestUri());
225225
$this->assertEquals(array('foo' => ''), $request->query->all());
226+
227+
## assume rewrite rule: (.*) --> app/app.php ; app/ is a symlink to a symfony web/ directory
228+
$request = Request::create('http://test.com/apparthotel-1234', 'GET', array(), array(), array(),
229+
array(
230+
'DOCUMENT_ROOT' => '/var/www/www.test.com',
231+
'SCRIPT_FILENAME' => '/var/www/www.test.com/app/app.php',
232+
'SCRIPT_NAME' => '/app/app.php',
233+
'PHP_SELF' => '/app/app.php/apparthotel-1234',
234+
));
235+
$this->assertEquals('http://test.com/apparthotel-1234', $request->getUri());
236+
$this->assertEquals('/apparthotel-1234', $request->getPathInfo());
237+
$this->assertEquals('', $request->getQueryString());
238+
$this->assertEquals(80, $request->getPort());
239+
$this->assertEquals('test.com', $request->getHttpHost());
240+
$this->assertFalse($request->isSecure());
226241
}
227242

228243
/**
@@ -1302,7 +1317,7 @@ public function getBaseUrlData()
13021317
{
13031318
return array(
13041319
array(
1305-
'/foo%20bar',
1320+
'/foo%20bar/',
13061321
array(
13071322
'SCRIPT_FILENAME' => '/home/John Doe/public_html/foo bar/app.php',
13081323
'SCRIPT_NAME' => '/foo bar/app.php',

0 commit comments

Comments
 (0)
0