#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 onInputBag::set) and parts of#37265 are not needed anymore. By using onlyInputBag as the$request bag we can tighten the typehint again and make static analysis a bit more useful.

carsonbot added Status: Needs Review HttpFoundation labels

Feb 26, 2021

Copy link

ContributorAuthor

acasademont commentedFeb 26, 2021

PS: Also not sure if this should be considered a bugfix and merged into 5.2.x instead of 5.x

Copy link

Member

nicolas-grekas commentedFeb 26, 2021•
edited
Loading

Thinking a bit more about this, there might still be an issue when using JSON payloads: theInputBag::get() returns onlystring|null. That means booleans, integers, floats found in JSONs will be lost.

nicolas-grekas added this to the5.x milestone

Feb 26, 2021

Copy link

ContributorAuthor

acasademont commentedFeb 26, 2021•
edited
Loading

Copy link

Member

nicolas-grekas commentedFeb 26, 2021

Can you update the type hints so that we can discuss the full change that would be needed?

Copy link

Member

nicolas-grekas commentedFeb 26, 2021

(and add related tests btw)

acasademont force-pushed theuse_input_bag branch from06bb7e3 to74bdd51Compare

February 26, 2021 22:09

use InputBag for POST requests too, added missing scalar type hints

381a0a1

acasademont force-pushed theuse_input_bag branch from74bdd51 to381a0a1Compare

February 26, 2021 22:11

Copy link

ContributorAuthor

acasademont commentedFeb 26, 2021

Added the missing scalar type hints and some testing

ro0NL reviewed

Feb 27, 2021

View reviewed changes

src/Symfony/Component/HttpFoundation/Tests/InputBagTest.php

		$this->assertNull($bag->get('null','default'),'->get() returns null if null is set');
		$this->assertSame(1,$bag->get('int'),'->get() gets the value of an int parameter');
		$this->assertSame(1.0,$bag->get('float'),'->get() gets the value of a float parameter');
		$this->assertFalse($bag->get('bool'),'->get() gets the value of a bool parameter');

Copy link

Contributor

ro0NLFeb 27, 2021

Choose a reason for hiding this comment

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

perhaps assert eg. $default=true is never casted to string

Copy link

ContributorAuthor

acasademontMar 10, 2021

Choose a reason for hiding this comment

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

Sure, sorry I didn't see your comment!

nicolas-grekas approved these changes

Mar 10, 2021

View reviewed changes

Copy link

Member

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.

LGTM thanks.
@ro0NL's suggestion is a good one, can you update the PR if you agree?

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Mar 10, 2021

fabpot approved these changes

Mar 12, 2021

View reviewed changes

Copy link

Member

fabpot commentedMar 12, 2021

Thank you@acasademont.

fabpot merged commit7754bed intosymfony:5.x

Mar 12, 2021

acasademont deleted the use_input_bag branch

March 12, 2021 09:17

Copy link

ContributorAuthor

acasademont commentedMar 12, 2021

Ah! Thx@fabpot ! I was planning to update the tests this weekend, I'll submit another PR for them.

Copy link

dvdknaap commentedJun 11, 2021•
edited
Loading

Why are arrays not allowed anymore ?

This means if you got an form with checkboxes it's not allowed anymore.

Edit:@acasademont @fabpot Why is the set accepting arrays but the get not could this be fixed asap please?

Copy link

Contributor

olsavmic commentedJul 8, 2021•
edited
Loading

Why are arrays not allowed anymore ?
This means if you got an form with checkboxes it's not allowed anymore.
Edit:@acasademont @fabpot Why is the set accepting arrays but the get not could this be fixed asap please?

~~I guess you're supposed to access those nested arrays by calling$inputBag->all('key'). It's slightly misleading though now, I also stumbled upon this.~~
See:#41766