symfony: [HttpFoundation] Deprecation error null not allowed InputBag::set()

Symfony version(s) affected: 5.1.0

Description
Calling InputBag::set()with the second argument set to null triggers deprecation error:

Since symfony/http-foundation 5.1: Passing "null" as a 2nd Argument to "Symfony\Component\HttpFoundation\InputBag::set()" is deprecated, pass a string or an array instead.

This behaviour causes issues when replacing JSON request body with the JSON decoded value as JSON supports null values.

How to reproduce

/** @var Symfony\Component\HttpFoundation\Request $request */
$request->request->set('key', null);

Possible Solution
Allow null values by adding is_null(), since the currently used is_scalar(null) returns false: https://www.php.net/manual/en/function.is-scalar.php

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 6
Comments: 16 (10 by maintainers)

Commits related to this issue

bug #37265 [HttpFoundation] use InputBag for Request::$request only if data is coming from a form (nicolas-grekas) This PR was merged into the 5.1 branch. Discussion ---------- [HttpFoundation] use... — committed to symfony/symfony by fabpot 4 years ago
minor #40315 [HttpFoundation] Use InputBag for POST requests too (acasademont) This PR was merged into the 5.3-dev branch. Discussion ---------- [HttpFoundation] Use InputBag for POST requests too ... — committed to symfony/symfony by fabpot 3 years ago

Most upvoted comments

Is it a hack? I would rather see it as an IO-Result

Either the body is a form, in this cases you’re reasoning is very valid. but modern infrastructure shifts toward json bodys or other content. which are not really “parameters” since they are content type json.

if the whole body is JSON and JSON has the allowed type of null it does make sense to allow null values.

Personally I would say that’s a monomorphism/transforming or a strategy/adapter pattern -> the input which can be CT1 | CT2 -> IT (content-type1, content-type2 should become input-type (input interface)).

So as a developer I would expect it to work in a defined way. Esp since bundles implemented it in a similar way.

Maybe I am missing the real implications in the Framework that make it “bad” to use null values. Because if there are no directly affected parts one might argue it’s primarily a question of taste.

I don’t mean any disrespect just thinking out loud here.

chilimatic on Jun 4, 2020

I don’t think that addresses the original issue. It’s a pretty common pattern in projects I’ve worked on to replace the request parameters with decoded JSON which might contain null values. I know a request attribute could be used here instead, but that means everything has to know to examine that attribute instead of using Request::request which is much more natural and obvious IMO.

krixon on Jun 8, 2020

i agree there are few unclarities 😃 see also https://github.com/symfony/symfony/pull/34363#discussion_r357991020

I think ultimately it’ll rely on implicit string casting when accessed: https://3v4l.org/IIWBt

But then we should actually cast it today manually for consistent behavior (e.g. the same behavior with or without a real return type).

The array case should probably validated/documented to be string[] (any no. of levels actually, but i dont think there’s a notation for that).

InputBag is meant to make accessing single/multi values explict (?k=v vs. ?k[]=v), as the user is free to mangle those, throwing “400 bad request” as such. That’s the USP of InputBag at least.

I tend to agree the same case may apply to JSON decoded payloads.

So yes, given scalar is already allowed (not strictly string) it probably makes sense to allow null as well, esp. since it’s already anticipated in the return type.

ro0NL on Jun 6, 2020

Assuming the request method causes $_POST to be populated

PHP also checks the content-type before populating $_POST. This means your case is safe with my approach.

I am forced to pull out all of the parameters in this situation and extract the value from that:

yes, that’s on purpose: most ppl don’t use query parameters in the way you describe - the defaults are to make them safer. Now, it also means some use cases will need to adapt. You just described one 😃

nicolas-grekas on Jun 11, 2020

I see the reasoning behind $_POST/$_GET.

I still can be hacky as long as I cast it to string? or any other scalar value which would be in this union type variance? and arrays?

The rest of possible assumptions aside. what’s the purpose and how do we benefit from it not being null?

chilimatic on Jun 4, 2020