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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty arrays (as passed by her/faraday) are parsed as [nil] #2079

Closed
PragTob opened this issue Jul 6, 2020 · 7 comments
Closed

Empty arrays (as passed by her/faraday) are parsed as [nil] #2079

PragTob opened this issue Jul 6, 2020 · 7 comments
Labels

Comments

@PragTob
Copy link

PragTob commented Jul 6, 2020

馃憢

And first thanks for all the work on the project! 馃挌

We have a grape API and applications talking to each other, for which we use her which uses faraday.

At some point the faraday behavior changed for sending empty arrays lostisland/faraday#801

The problem occurs when we use her to query resoruces Resources::Something.where(param_name: []). It used to be that they were omitted (at least as URL params, unsure) but now they are sent as param_name[] in the URL, which is a bugfix. Grape parses this as [nil] which is not what I'd expect (I'd expect []) and also then fails the parameter validation (we have a type of Array[String]). allow_blank: true doesn't help because an array with nil isn't blank (and we'd still have the wrong value to deal with).

We're running the latest grape release (1.3.3).

I'll work around the issue with coerce_with or defining my own type or something. Still thought it was worth opening an issue about it.

@dblock
Copy link
Member

dblock commented Jul 6, 2020

There was a bunch of work related to this in 1.4.0 (HEAD). I suggest writing a spec and seeing if master has fixed this, first?

@dblock dblock added the bug? label Jul 6, 2020
@PragTob
Copy link
Author

PragTob commented Jul 6, 2020

@dblock thanks for letting me know :) I have a bunch of specs for this. Let me see if it's fixed on main.

@PragTob
Copy link
Author

PragTob commented Jul 6, 2020

@dblock just tried it out with 197d5b4 - still exactly the same behavior for us.

@dnesteryuk
Copy link
Member

@PragTob Could you add a spec for this bug, please?

I've tried this in tests

get '/required?cats[]='

declared(params) gave:

{"cats"=>[""]}

@PragTob
Copy link
Author

PragTob commented Jul 7, 2020

Morning @dnesteryuk !

I believe the difference is = vs. no = aka in our case we don't have the =/the way faraday/grape queries.

I'll add a PR with a spec (at least) just have to check whether I can do it on company time or if it's gotta be my own :)

PragTob added a commit to PragTob/grape that referenced this issue Jul 7, 2020
Was unsure where to put the specs, there's probably a better
place but here seemed fine for now.

Added some other array related specs for good measure.
@PragTob
Copy link
Author

PragTob commented Jul 7, 2020

Failing spec etc. sits at #2080 now

@PragTob
Copy link
Author

PragTob commented Jul 20, 2020

Moved to rack/rack#1696 as it seems to me it's rather in rack.

@PragTob PragTob closed this as completed Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants