Symfony (http foundation) request->getRequestUri() doesn't contain query string

[Rastusik]

Describe the bug
The creation of Http Request in the RequestFactory doesn't set the $_SERVER[REQUEST_URI] parameter to the $server variable, which results in wrong value returned by calling $request->getRequestUri().

Steps To Reproduce
When a Symfony app with swoole bundle with gets requested with an url like this: http://localhost:9501/twig?test=abc&wtf=123, calling $request->getRequestUri() somewhere in the code, will return '/twig'.

Expected behavior
The method call should return this value: /twig?test=abc&wtf=123

Reference in symfony's Http/Request code: https://github.com/symfony/http-foundation/blob/1b66b861f3ff55fb39c13822a0fc915018b5ac84/Request.php#L1005

Environment (please complete the following information):

• OS: Linux
• PHP Version: 7.3.15
• Swoole Version: 4.4.8
• Symfony Version: 5.1.1
• Running in docker: Yes

Proposed fix
The bug happens because this line: https://github.com/symfony/http-foundation/blob/v5.1.1/Request.php#L413 isn't called by creating the Request instance in the RequestsFactory

One solution would be to add these two lines into the factory:

$queryString = $server['QUERY_STRING'] ?? '';
$server['REQUEST_URI'] = $server['REQUEST_URI'] . ($queryString !== '' ? '?' . $queryString : '');

Another solution would be to call Request::create(), instead of just calling the Request class constructor directly.

[k911]

Yeah, definitely we should use a proper factory method to create a request object when they're available.

Do you want to provide a fix for that issue? @Rastusik

[Rastusik]

I looked into it, and I think the first solution is sufficient because there would need to be very similar two lines of code even with the proper factory method usage because the proper factory method needs the manually concatenated path anyway and it also needs a http method as a string parameter which I don't know how to obtain from swoole request, probably from $server[REQUEST_METHOD] ... Do you think this is worth investigating?

[k911]

Yeah, I think it's worth investigating and probably we should also write a test that proves correct behavior.

[Rastusik]

Well, I did the investigation and the second solution would be to use this method: Request::create(string $uri, string $method = 'GET', array $parameters = [], array $cookies = [], array $files = [], array $server = [], $content = null), but the uri parameter would need to be created in the request factory the same way as the simpler solution.