Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: Add array to return of InputBag::get#41766

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
shyim wants to merge1 commit intosymfony:5.3fromshyim:add-missing-array-to-input-bag
Closed

fix: Add array to return of InputBag::get#41766

shyim wants to merge1 commit intosymfony:5.3fromshyim:add-missing-array-to-input-bag

Conversation

@shyim
Copy link
Contributor

QA
Branch?5.3
Bug fix?yes
New feature?no
Deprecations?no
Tickets
LicenseMIT
Doc PR

The method can return array. Theset method has already array typehint

@nicolas-grekas
Copy link
Member

This is on purpose. If you want to get an array, you should useall('foo').
See the deprecation in the body of the method.

akcoder, David-Crty, adrian-sm-tremend, steveclifton, lordisp, artyuum, GwendalGrelier, mel-any, joesenova, and musahaidari reacted with thumbs up emojidvdknaap, carakas, EitanElbaz, andimg93, Silinator, harli91, MacDada, and Nerogee reacted with thumbs down emojimel-any and musahaidari reacted with hooray emoji

@TjorvenB
Copy link

TjorvenB commentedJun 22, 2021
edited
Loading

This is quite the BC break. Is there a place where we can find the reasoning behind this? There is no explanation in the upgrade notes or the commits.
It would be a nice read while I look for all controllers where I handle array query parameters and add a custom implementation for default values, as such are no longer supported.

harli91 and Unifex reacted with thumbs up emoji

@MaximePinot
Copy link
Contributor

This is quite the BC break. Is there a place where we can find the reasoning behind this? There is no explanation in the upgrade notes or the commits.
It would be a nice read while I look for all controllers where I handle array query parameters and add a custom implementation for default values, as such are no longer supported.

It was introduced in#34363. You will find some explanations here! ;)

TjorvenB and MacDada reacted with thumbs up emoji

@dvdknaap
Copy link

Only adding the array type will not fix it. the if statement check in the get should be the same as the set function so you should add!\is_array($value)

@stof
Copy link
Member

stof commentedJul 8, 2021

@nicolas-grekas even if it is deprecated, it can still return an array in 5.3

@nicolas-grekas
Copy link
Member

Sure, but we don't phpdocument deprecated types.

@stof
Copy link
Member

stof commentedJul 8, 2021

@nicolas-grekas we will need to change that policy anyway. With native return types, we cannot remove the deprecated types from the signature, otherwise we get a TypeError

@dvdknaap
Copy link

Only adding the array type in the docblock doesn't fix anything.

This is still a bug and throws a error because it doesn't accept array while it accepts it in the set function.
vendor/symfony/http-foundation/InputBag.php:36

if (null !==$value &&$this !==$value && !is_scalar($value)) {thrownewBadRequestException(sprintf('Input value "%s" contains a non-scalar value.',$key));        }

It should also accept array again because a lot of websites and app are broke now like mine.

It should accept the same as in the set function

vendor/symfony/http-foundation/InputBag.php:77

if (null !==$value && !is_scalar($value) && !\is_array($value) && !$valueinstanceof \Stringable) {thrownew \InvalidArgumentException(sprintf('Excepted a scalar, or an array as a 2nd argument to "%s()", "%s" given.',__METHOD__,get_debug_type($value)));        }

@shyim
Copy link
ContributorAuthor

@dvdknaap you need to call nowall instead of get when you want a array

xabbuh, mateuszkrol-bandithouse, win-d, and monica-marc reacted with thumbs up emoji

@szepczynski
Copy link
Contributor

szepczynski commentedMar 31, 2022
edited
Loading

But how to test is given key is array or scalar value.
Let's go with this example:

GET https://localhost/?q=aGET https://localhost/?q[]=a&q[]=b

First case you can get only through$request->query->get('q');
Second case only through$request->query->all('q');

I can't see a way how to test isq parameter is scalar or array (except catching exception of course)

VDev-S and Nerogee reacted with thumbs up emoji

@alexander-schranz
Copy link
Contributor

alexander-schranz commentedApr 29, 2022
edited
Loading

I also think this leads into some strange cases now specially when working with the FOSRestBundle which is setting the JSON Content to$request->request via theBodyListener. I would had understand if it go strict to string now as from the http specification but instead now float, int are allowed but no array feels strange.

andimg93 reacted with thumbs up emoji

@ddr5292
Copy link

This breaks my code guys. Bad form.

miqrogroove reacted with thumbs up emoji

@ro0NL
Copy link
Contributor

a deprecation breaks your code?

@ro0NL
Copy link
Contributor

ro0NL commentedMar 23, 2023
edited
Loading

But how to test is given key is array or scalar value.

you mostly know what your application expects

getting the raw value is done usingall()['key']. Please open a doc PR if it's unclear.

experteam-mx pushed a commit to experteam-mx/api-crud-bundle that referenced this pull requestApr 18, 2023
@christiaangoossens
Copy link

@ddr5292 And another one, also broke my code on a minor dependency update. For anyone searching for this on the internet, if you getSymfony\Component\HttpFoundation\Exception\BadRequestException: Input value $key contains a non-scalar value. in /vendor/symfony/http-foundation/InputBag.php:37 you have a controller doing$request->json()-get($key) somewhere in your code on an array. Replace it with$request->input($key) (for Laravel).

Filoz added a commit to Filoz/form-filter-bundle that referenced this pull requestAug 17, 2023
Change from$request->query->getto$request->query->allbecause of Symfony 6 (symfony/symfony#41766 (comment) ).This change prevent this error: "Input value "horse_filter" contains a non-scalar value."
@Nerogee
Copy link

But how to test is given key is array or scalar value. Let's go with this example:

GET https://localhost/?q=aGET https://localhost/?q[]=a&q[]=b

First case you can get only through$request->query->get('q'); Second case only through$request->query->all('q');

I can't see a way how to test isq parameter is scalar or array (except catching exception of course)

I cannot agree more with you. I useget() for all type of data. but now it all mess up. The worst thing is that I cannot see the reason why it worth even introducing a breaking change. User submitted value could be very unpredictable. Sometimes it's on purpose and allowed to input either a scalar value or an array. What we need is the flexibility in the back-end to handle it.

@wouterj
Copy link
Member

This ship sailed long ago. As you can see, this has been discussed numerous times 2 years ago, so it's unlikely that the outcome changes now. Furthermore, both deprecation and new behavior has been part of Symfony for nearly 4 years. Changing or reverting now is impossible and will impact more applications than changing this in existing applications that are upgrading to 5.x or 6.x now.

Please also do not comment more or less the same things across multiple issues/PRs within the same topic. This divides the same discussion across multiple places, making it harder to track.

@derrabus
Copy link
Member

I'm locking all of those issues now. If there's something NEW to discuss, please open a new issue.

@symfonysymfony locked and limited conversation to collaboratorsDec 3, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

5.3

Development

Successfully merging this pull request may close these issues.

15 participants

@shyim@nicolas-grekas@TjorvenB@MaximePinot@dvdknaap@stof@szepczynski@alexander-schranz@ddr5292@ro0NL@christiaangoossens@Nerogee@wouterj@derrabus@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp