[HttpKernel] Fixed RequestDataCollector handling of null header values. #20747
Conversation
|
But it doesnt expect |
|
@ro0NL You are correct, I should have noticed this. However, this brings up an interesting point, should the HeaderBag set() method protect against this being possible? |
|
No, this is design by contract. We could however clarify the PHPdoc, ie. Then there are also a few |
|
Personnaly I don't use I think that if we set |
|
Imo setting
That doesnt mean you can just violate |
|
@nicolas-grekas would be interested in your opinion here. |
| @@ -125,7 +125,7 @@ public function collect(Request $request, Response $response, \Exception $except | ||
| continue; | ||
| } | ||
| if ('request_headers' === $key || 'response_headers' === $key) { | ||
| $value = array_map(function ($v) { return isset($v[1]) ? $v : $v[0]; }, $value); | ||
| $value = array_map(function ($v) { return isset($v[1]) ? $v : (isset($v[0]) ? $v[0] : null); }, $value); |
There was a problem hiding this comment.
To be even more defensive, I'd suggest: isset($v[0]) && !isset($v[1]) ? $v[0] : $v
There was a problem hiding this comment.
Sure. With regards to the unit test however, would you omit this change to the unit test in order to not violate the current interface contract of the HeaderBag::set() method.
There was a problem hiding this comment.
If it's green, let's do with for now.
|
👍 |
|
Is it really something for master only? |
|
3.2! |
|
Thank you @gmoreira. |
…eader values. (Gabriel Moreira) This PR was submitted for the master branch but it was merged into the 3.2 branch instead (closes #20747). Discussion ---------- [HttpKernel] Fixed RequestDataCollector handling of null header values. | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This PR references this [discussion](symfony/http-kernel@1fee784#commitcomment-20028839). Commits ------- adc4a26 [HttpKernel] Fixed RequestDataCollector handling of null header values.
This PR references this discussion.