Skip to content
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] Allow null in InputBag@set #37327

Merged
merged 1 commit into from Jun 17, 2020
Merged

Conversation

taylorotwell
Copy link
Contributor

@taylorotwell taylorotwell commented Jun 17, 2020

This allows null to be passed to InputBag's set method.

Q A
Branch? 5.1
Bug fix? yes
New feature? no
Deprecations? no
License MIT

Previously, null was an allowed value to the InputBag's set method. At some point this was deprecated. It would be very nice for us to be able to set this to null. For example, this ability drives a very popular feature of Laravel where we ship a middleware that converts all empty strings to null on incoming requests.

Note that the get method already specifically allows null and null is a valid decoded JSON value.

@taylorotwell
Copy link
Contributor Author

@yceruto open against 5.1 here.

@chalasr
Copy link
Member

chalasr commented Jun 17, 2020

The travis failure looks related, can you please have a look?

@taylorotwell
Copy link
Contributor Author

@chalasr fixed that test. On Travis re-run the 7.3 branch is failing with a bunch of unrelated PDO failures.

@chalasr chalasr added Bug and removed Feature labels Jun 17, 2020
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Thanks for the feedback and the PR

Copy link
Member

@yceruto yceruto left a comment

Choose a reason for hiding this comment

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

Thanks!

@chalasr
Copy link
Member

chalasr commented Jun 17, 2020

Thank you @taylorotwell. And congratz for your first contribution to Symfony! :)

@chalasr chalasr merged commit 4773c5e into symfony:5.1 Jun 17, 2020
@ro0NL
Copy link
Contributor

ro0NL commented Jun 18, 2020

Could we partially revert #37265 then?

@fabpot fabpot mentioned this pull request Jul 24, 2020
fabpot added a commit that referenced this pull request Mar 12, 2021
…sademont)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[HttpFoundation] Use InputBag for POST requests too

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Partially revert #37265
| License       | MIT
| Doc PR        | -

#37265 was created as a fix for #37100. However, when #37327 was merged, the original bug was also fixed with a different solution (allowing null values on `InputBag::set`) and parts of #37265 are not needed anymore. By using only `InputBag` as the `$request` bag we can tighten the typehint again and make static analysis a bit more useful.

Commits
-------

381a0a1 use InputBag for POST requests too, added missing scalar type hints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants