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

[HttpFoundation] Request should not use php://input if content is NULL #24437

Open
andig opened this Issue Oct 5, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@andig

andig commented Oct 5, 2017

Q A
Bug report? not sure
Feature request? yes
BC Break report? yes
RFC? yes/no
Symfony version ^2.6

I have a problem when sending HTTP request with using HttpFoundation, Http Messagebridge together with GuzzleHttp.

What happens is that a simple Symfony GET request:

GET /entity.json HTTP/1.1
Host: 127.0.0.1:8080

becomes when converted to Psr7 and sent via Guzzle:

GET /entity.json HTTP/1.1
Host: 127.0.0.1:8080
Transfer-Encoding: chunked
Expect: 100-Continue
user-agent: Symfony/3.X
accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
accept-language: en-us,en;q=0.5
accept-charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7

The problem:

Getting request as ressource via Request->getContent(true) returns php://input as ressource for GET request body since content is NULL (default). The HttpMessageBridge creates a Psr7 request with php://input as body stream. Due to this stream being present Guzzle is not able to determine body size (NULL instead of 0) and creates a complex request.

Impact:

Request is slow (Expect: 100-continue) and some server libraries like reactphp/http are not able to handle it.

While I could fix that for my own HTTP client by setting content to (string)"" instead of NULL this won't fix it for other clients.

Does it really make sense that Symfony Request returns php://input when content is not set?

@andig

This comment has been minimized.

Show comment
Hide comment
@andig

andig Oct 6, 2017

Note: the original body asRessource functionality has been introduced in symfony/http-foundation@1b2736d by @Seldaek

andig commented Oct 6, 2017

Note: the original body asRessource functionality has been introduced in symfony/http-foundation@1b2736d by @Seldaek

@Simperfit

This comment has been minimized.

Show comment
Hide comment
@Simperfit

Simperfit Nov 8, 2017

Contributor

@andig Do you want to provide a PR ? I think this has to be fixed.

Contributor

Simperfit commented Nov 8, 2017

@andig Do you want to provide a PR ? I think this has to be fixed.

@andig

This comment has been minimized.

Show comment
Hide comment
@andig

andig Nov 8, 2017

Do you want to provide a PR ? I think this has to be fixed.

Would need at least some insight if this makes sense.

andig commented Nov 8, 2017

Do you want to provide a PR ? I think this has to be fixed.

Would need at least some insight if this makes sense.

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Nov 26, 2017

Member

@andig if the patch is not huge, I think you could gather more feedback by providing a PR. I don't see anything that would allow me to tell you to not try it...

Member

nicolas-grekas commented Nov 26, 2017

@andig if the patch is not huge, I think you could gather more feedback by providing a PR. I don't see anything that would allow me to tell you to not try it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment