-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpFoundation] Problem with Request::getBaseUrl and url-rewrites #7329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
There are inconsistencies also when the uri basename is included in the SCRIPT_NAME. This data makes testGetBaseUrl fails too: //Test Fails
array(
'/foobar/index.php',
array(
'SCRIPT_FILENAME' => '/foo/bar/web/index.php',
'SCRIPT_NAME' => '/web/index.php',
'PHP_SELF' => '/web/index.php',
),
'', //getBaseUrl Fails, '/web/index.php' returned
'/foobar/index.php', //getPathInfo Fails, 'php' returned
) |
this test is written wrong: array(
'/webmaster',
array(
'SCRIPT_FILENAME' => '/foo/bar/web/index.php',
'SCRIPT_NAME' => '/web/index.php',
'PHP_SELF' => '/web/index.php',
),
'', //getBaseUrl Fails, '/web' returned
'/webmaster', //getPathInfo Fails, 'master' returned
) it should be array(
'/webmaster',
array(
'SCRIPT_FILENAME' => '/foo/bar/web/index.php',
'SCRIPT_NAME' => '/web/index.php',
'PHP_SELF' => '/web/index.php',
),
'/web',
'master',
) |
This is what the actual implementation returns... :) But is the result the wished one? It seems to me a an inconsistency with the other case. |
See also #7053 that maybe fixes the problems here becausee it includes fixes made upstream in ZF to |
I'm not implying that the current implementation is not buggy (I don't have the specs), I was just making an observation based on what is the implementation and how the test was written. (sorry @nicmart I didn't notice from your code comments that you already know that) |
@nicmart Could you please explain me when the problem you describe above can actually happen? I don't see how the SCRIPT_NAME can have a folder prefix when the request URI does not specifiy one. Do you actually have a real case for this? The only theoretical case I can maybe think of is when the mod_rewrite rules are moved one directory up and rewrites |
@Tobion It's a real case. I've crushed against this problem trying to gettin a legacy application working with the http and routing symfony components. Take for example
SCRIPT_NAME has a folder prefix because the front controller is not in the web root folder, but in a descendant folder. |
Yes that's what I imagined. But it's not best practise because it makes the full project accessible. Usually the web root should be in |
I know, but it was not possible to move the web root due to legacy constraints. |
Hm seeing your second report I figured this makes all symfony projects even more vulnurable to symfony/symfony-standard#469. |
I don't see how your second example can be fixed reliably. Because With mod_rewrite enabled it probably is not the base url but just a requested path that was rewritten to the front controller. Handling it wrong as now, will result in the problems I described above. But this path might also be an apache alias path where |
…ath info I synchronized the code with the changes done upstream in ZF: https://github.com/zendframework/zf2/blob/master/library/Zend/Http/PhpEnvironment/Request.php I also added tests from ZF and symfony#7329. I also rewrote the preparePathInfo slightly because there were some strange things.
As example for the bad base url problem: http://jmsyst.com/www/jmstutorial/current/web/app.php |
When using mod_rewrite-like functionality, heuristic search for baseUrl can only do so much. It would be better to let developers override baseUrl on a per-request basis, as only they can know the proper value. Allowing to set it in environment configuration would solve this problem. Below are tests for my specific case, where I rewrote /foo as /foo/public and /foo/bar as /foo/bar/public, equivalent to putting Symfony project in a subdirectory and rewriting url to point to public folder (both tests are equivalent and the latter only represents nesting Symfony project further, such as domain.com/sandbox/project/): array(
'/foo/home',
array(
'SCRIPT_FILENAME' => '/home/John Doe/public_html/foo/public/app.php',
'SCRIPT_NAME' => '/foo/public/app.php',
'PHP_SELF' => '/foo/public/app.php',
),
'/foo', //getBaseUrl Fails, '' returned
'/home', //getPathInfo Fails, '/foo/home' returned
),
array(
'/foo/bar/home',
array(
'SCRIPT_FILENAME' => '/home/John Doe/public_html/foo/bar/public/app.php',
'SCRIPT_NAME' => '/foo/bar/public/app.php',
'PHP_SELF' => '/foo/bar/public/app.php',
),
'/foo/bar', //getBaseUrl Fails, '' returned
'/home', //getPathInfo Fails, '/foo/bar/home' returned
), |
closed #13038 according to @nicolas-grekas in #13039 (in favor to #13039 that is still open) |
…g 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 $_SERVE 737D R 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
I've encountered an inconsistent behavior with Request::getBaseUrl method when the Request URI does not include the SCRIPT_NAME (due for example to an apache url rewrite), but it includes a segment of the SCRIPT_NAME path.
In RequestTest.php, if I add this data to the getBaseUrlData data provider, testGetBaseUrl fails:
because in this case getBaseUrl returns '/web' and not ''. This make getPathInfo fails too (it returns 'master' instead of '/webmaster'),
If the URI does not start with 'web' all works as expected:
The text was updated successfully, but these errors were encountered: