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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bug with array into params #183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix bug with array into params #183

wants to merge 1 commit into from

Conversation

achempion
Copy link

array colors['red', 'green'] must convert to
colors[]=red&colors[]=green but not colors=red&colors=green

array `colors['red', 'green']` must convert to
`colors[]=red&colors[]=green` but not `colors=red&colors=green`
@buildhive
Copy link

Hiroshi Nakamura » httpclient #86 FAILURE
Looks like there's a problem with this pull request
(what's this?)

@achempion
Copy link
Author

@nahi What do you think about?

@nahi
Copy link
Owner

nahi commented Dec 2, 2013

Is this a Rails convention, right? I'm open to add this as a new feature but I don't want to break existing application.

@achempion
Copy link
Author

right, here is to params convert sample https://gist.github.com/achempion/7750548 into rails console

I have a trouble with fix specs for changes

@chulkilee
Copy link

As far as I know, there is no "right answer" for it.

@kikonen-fiksu
Copy link

if Rails gets query params encoded as "foo=x&foo=y" then in the end randomly (well randomness in this case may mean always last value, but haven't checked) results:

{
  foo: 'y'
}

which is clearly inapproriate.

So currently params must be manually encoded (aka. "params.to_query") if using Rails as backend server.

This naturally means that default behavior of HTTPClient is broken with Rails. Note that this is also inconsistent with default behavior of "rest-client" gem (common simple REST client), which may cause surprise when switching to HTTPClient.

When fixing this, please also consider addressing "null value" encoding, so that null value doesn't convert into empty string. In Rails, if query param is passed in as "foo" then it is interpreted as nil, while "foo=" comes out as empty string.

Of course using "application/json" encoding in body avoids problem, but for GET calls that option is naturally not available.

Surely could just document well, that one must use "params.to_query" if wanting this possibly Rails specific encoding convention (especially since "to_query" will also manage Rails specific logic for encoding nested map structures).

@achempion
Copy link
Author

I think that we can resolve this just added a configuration file with setting called like "query_params_pattern"

@nahi nahi added the Feedback label Nov 3, 2014
@nahi
Copy link
Owner

nahi commented Nov 3, 2014

Adding option sounds nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants