Skip to content

[HttpFoundation] Ensure path info has a leading slash #40750

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

Open
wants to merge 4 commits into
base: 7.4
Choose a base branch
from
Open

[HttpFoundation] Ensure path info has a leading slash #40750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e52294
Ensure path info has a leading slash
alexpott Apr 9, 2021
5464a70
Move test to its own method
alexpott Apr 9, 2021
3a5f319
Improve documentation of assumptions
alexpott Apr 9, 2021
8897fca
Merge branch '6.2' into ensure-path-info-has-leading-slash
alexpott Oct 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Symfony/Component/HttpFoundation/Request.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1872,6 +1872,9 @@ protected function preparePathInfo(): string
// If substr() returns false then PATH_INFO is set to an empty string
return '/';
}
if ('/' !== $pathInfo[0]) {
$pathInfo = '/'.$pathInfo;
}

return $pathInfo;
}
Expand Down
21 changes: 21 additions & 0 deletions src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,27 @@ public function testCreate()
$this->assertEquals('http://test.com/foo', $request->getUri());
}

public function testIndexDotPhpPathInfo()
{
// assume no rewrite rule: /index.php --> /
// assume index.php is a real file.
// assume rewrite rule passes all requests not referring directly to
// files in the filesystem to index.php.
$request = Request::create('http://test.com/index.php.php', 'GET', [], [], [],
[
'DOCUMENT_ROOT' => '/var/www/www.test.com',
'SCRIPT_FILENAME' => '/var/www/www.test.com/index.php',
'SCRIPT_NAME' => '/index.php',
'PHP_SELF' => '/index.php',
]);
$this->assertEquals('http://test.com/index.php/.php', $request->getUri());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that case looks wrong to me though, as that would be a different Uri that the input

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should the uri be though? If you make a request to index.php/something or /something in this situation you'd want the path info to be /something. And it is. I think this is a special edge case because of the way htaccess rules work for Drupal (and probably a number of other projects). The relevant parts of our rule is:

  RewriteCond %{REQUEST_FILENAME} !-f
  RewriteCond %{REQUEST_FILENAME} !-d
  RewriteCond %{REQUEST_URI} !=/favicon.ico
  RewriteRule ^ index.php [L]

I think we should treat a request to index.phpSOMETHING as a request to SOMETHING - regardless of whether SOMETHING starts with a /... which means we need to duplicate the logic of when there is no base path...

This is just above the code I've added.

        if ('' !== $requestUri && '/' !== $requestUri[0]) {
            $requestUri = '/'.$requestUri;
        }

        if (null === ($baseUrl = $this->getBaseUrl())) {
            return $requestUri;
        }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For index.php/something, It totally agree. But this is not what this test is covering.

$this->assertEquals('/.php', $request->getPathInfo());
$this->assertEquals('', $request->getQueryString());
$this->assertEquals(80, $request->getPort());
$this->assertEquals('test.com', $request->getHttpHost());
$this->assertFalse($request->isSecure());
}

public function testCreateWithRequestUri()
{
$request = Request::create('http://test.com:80/foo');
Expand Down