8000 [HttpFoundation] [Request] fix baseUrl parsing to fix wrong path_info by rk3rn3r · Pull Request #13038 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] [Request] fix baseUrl parsing to fix wrong path_info #13038

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[HttpFoundation] [Request] fix baseUrl parsing to fix wrong path_info #13038

wants to merge 2 commits into from

Conversation

rk3rn3r
Copy link
Contributor
@rk3rn3r rk3rn3r commented Dec 19, 2014

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).
We will need it also for symfony 2.3, I will provide a merge request for 2.3 too in some minutes. (done, see #13039)
I also prepared a merge request for actual 2.7 branch, it will also follow in some minutes. (done, see #13040)

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets this
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

Happy christmas!

* fixed baseUrl parsing
a4674d5 
** added missing slash to baseUrl-path part check to remove the path, only when it's also a path in the uri
* fixed and added unittests
This was referenced Dec 19, 2014
Closed
Closed
@rk3rn3r
Copy link
Contributor Author
rk3rn3r commented Dec 19, 2014

@markuspoerschke @andygrunwald FYI
@timglabisch what do you think?

@Tobion
Copy link
Contributor
Tobion commented Dec 19, 2014

Highly related to #7329

@rk3rn3r
Copy link
Contributor Author
rk3rn3r commented Dec 20, 2014

closed according to @nicolas-grekas in #13039

@rk3rn3r rk3rn3r closed this Dec 20, 2014
@rk3rn3r rk3rn3r mentioned this pull request Dec 20, 2014
Closed
fabpot added a commit that referenced this pull request Jan 18, 2015
@fabpot
bug #13039 [HttpFoundation] [Request] fix baseUrl parsing to fix wron…
b8e4b4a 
…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 $_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 baseUr
6621
l-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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rk3rn3r @Tobion
0