Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has acontribution guide which I suggest you to read.

In short:

• Always add tests
• Keep backward compatibility (seehttps://symfony.com/bc).
• Bug fixes must be submitted against the lowest maintained branch where they apply (seehttps://symfony.com/releases)
• Features and deprecations must be submitted against the 7.3 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

You need to add a check for the php version then if it is only available in 8.4

The problem is that the tests fail because they were designed for the old solution with$request->getContent(). Additionally, the existing tests only cover requests tests forapplication/x-www-form-urlencoded, notmultipart/form-data.
The exception thrown originates fromrequest_parse_body():RequestParseBodyException: Request does not provide a content type because (as far as I understand) it is a test environment without a real request.
Setting$_SERVER['CONTENT_TYPE'] has no effect.

At the moment, I’m unsure how to properly adapt or rewrite the tests to handle this scenario. I would appreciate any suggestions or ideas on how to proceed with testing for the new implementation (and withmultipart/form-data) in this context.

I did some "manual tests" with a simple controller to see what the output is.

The controller:

class TestController{    #[Route('/test')]publicfunctiontest(Request$request):JsonResponse    {returnnewJsonResponse(['method' =>$request->getMethod(),'content_type' =>$request->headers->get('content-type'),'request_all' =>$request->request->all(),'request_get_content' =>$request->getContent(),'payload_all' =>$request->getPayload()->all(),'files_all' =>$request->files->all(),        ]);    }}

The requests (sent with cURL):

curl --location --request PUT 'http://127.0.0.1:8000/test' \--header 'Content-Type: application/x-www-form-urlencoded' \--data-urlencode 'foo=foovalue' \--data-urlencode 'bar=barvalue'

curl --location --request PUT 'http://127.0.0.1:8000/test' \--form 'foo="foovalue"' \--form 'bar="barvalue"' \--form 'baz=@"/Users/rott/textfile.txt"'

with textfile.txt:
Symfony is cool

Setting$_SERVER['CONTENT_TYPE'] has no effect.

Indeed you’d need$_SERVER['HTTP_CONTENT_TYPE']:https://www.php.net/manual/en/reserved.variables.server.php#refsect1-reserved.variables.server-description

Setting$_SERVER['CONTENT_TYPE'] has no effect.

Indeed you’d need$_SERVER['HTTP_CONTENT_TYPE']:https://www.php.net/manual/en/reserved.variables.server.php#refsect1-reserved.variables.server-description

Thanks for the notice, but unfortunately this also doesn't change it.
Even if I set$_SERVER['HTTP_CONTENT_TYPE'] right before callingrequest_parse_body()RequestParseBodyException: Request does not provide a content type is thrown.

I am a little bit confused because$_SERVER['CONTENT_TYPE'] is used in the test:

Woops indeed my bad I missedf16e206 😅

Oh, and we need to define the behavior when an exception is throw by the function!

@nicolas-grekas Thank you for the input.
What would you suggest to do when an Exception is thrown? Just leave request and files empty?

I guess so. We should behave as close as how PHP behaves with other methods.

Just a thought I had: wouldn’t the Runtime component be a better place forrequest_parse_body? As its goal is allegedly to populate$_POST and$_FILES it feels like it should be called beforeRequest::createFromGlobals?

Just a thought I had: wouldn’t the Runtime component be a better place forrequest_parse_body? As its goal is allegedly to populate$_POST and$_FILES it feels like it should be called beforeRequest::createFromGlobals?

Good point, but another thought from me: I originally came across this via Laravel, which is using Symfony as foundation for requests. For instance, Laravel relies on Symfony’s Request class for parsing requests. If this functionality were implemented in the Request class, Laravel and other frameworks leveraging Symfony’s components could benefit from it as well, given that they don’t rely on the Runtime component, as far as I understand

Up to move this PR forward@rottifant ?

Up to move this PR forward@rottifant ?

Yes, I will continue in the next days.

What's the current problem? I would love to have this fix/feature, maybe I can help.

@florian2peaches
I'm sorry. I am currently working on many projects and I don't have the time.

I would be very happy if you could take it over :-) The implementation itself seems to work. Test tests that cover this new functionrequest_parse_body still need to be added. I can give you access to my fork if you want.

The exception thrown originates from request_parse_body(): RequestParseBodyException: Request does not provide a content type because (as far as I understand) it is a test environment without a real request.
Setting $_SERVER['CONTENT_TYPE'] has no effect.

That's still turning CI red, any more info on that?

The exception thrown originates from request_parse_body(): RequestParseBodyException: Request does not provide a content type because (as far as I understand) it is a test environment without a real request.
Setting $_SERVER['CONTENT_TYPE'] has no effect.

That's still turning CI red, any more info on that?

Yes. As far as I found out, there is currently no way to test and simulaterequest_parse_body without a HTTP real request.
I thought about starting a server (php -S) when testing and sending real HTTP Requests to it to test the implementation. But tbh I do not have much experience with PHPUnit, don't exactly know how this should be done and if it is even possible to start the server in the test – especially with different environments/OS.

I'm on the same lane, I know php works that way sorequest_parse_body internally accesses the request, but isn't there a way to inject a request internally or some kind of polyfill forrequest_parse_body?

Unfortunately that was also the point where I got stuck and had no other idea than the "real" http server. Maybe the Symfony team has an idea how to implement this in the tests?
As the function itself is relatively new, I could not find any information on how to test it properly.

In php-src there are helper functions which start and send requests to built-in cli server.
You can look at example herephp_cli_server_005.phpt
And these helper functions live herephp_cli_server.inc

So in theory you can start simple server which callsRequest::createFromGlobals() and dumps result in response which you can then assert