Skip to content

[HttpFoundation] InputBag feature seems problematic #37029

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

Closed
gmponos opened this issue May 31, 2020 · 2 comments
Closed

[HttpFoundation] InputBag feature seems problematic #37029

gmponos opened this issue May 31, 2020 · 2 comments

Comments

@gmponos
Copy link
Contributor

gmponos commented May 31, 2020

Symfony version(s) affected: 5.1.0

Description

Hi this is a followup of this: #34363

  1. http://localhost/myendpoint?test=what&test=what2 -> this is a valid request. So the query option test can exist one or multiple times. Which makes it a string AND an array... Although this is not handled by sf ATM. I have another app written on another lang that I would like to migrate to sf at some point.. was hoping that in the future sf will add support for the feature above but I see it is moving towards to the opposite direction.
  2. Now you are defining the behavior. I mean before the behavior was in the hands of the programmer. If he didn't check that it is an array it was it's own bug therefore a 500 is fine.. now why to throw an exception?

For instance how is someone supposed to handle now this? (which I do have cases like this, yea I know they suck a bit)

http://localhost/myendpoint?username=what
http://localhost/myendpoint?username[]=what&username[]=what2

Let's say we are on sf6... should the developer add a try catch using get and then if an exception is thrown use the all function. It seems a little bit not worth it..

Furthermore what was the motion behind this PR...? Was this something commonly reported that should've been fixed? IMHO it was working fine as it was and the strictness there is not needed.

@ro0NL
Copy link
Contributor

ro0NL commented May 31, 2020

AFAIK, in PHP, test=what&test=what2 is always string as the latter overwrites the first.

The motion is to have 400 response instead of 500, when users tempers with the query params. As the app may unexpectedly receive an array leading to all sorts of crashes (given a string is expected/assumed).

To get the raw value, thus string|array use all()['key'].

@gmponos
Copy link
Contributor Author

gmponos commented May 31, 2020

AFAIK, in PHP, test=what&test=what2 is always string as the latter overwrites the first.

This is a problem of the $_QUERY superglobal. Symfony could still overcome this by reading and parsing the QUERY_STRING from the $_SERVER.

To get the raw value, thus string|array use all()['key'].

This is indeed better 👍 .. could also work for the above case..

The motion is to ....

still not convinced thought but will close it.

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

No branches or pull requests

4 participants