-
Notifications
You must be signed in to change notification settings - Fork 290
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
base: master
Are you sure you want to change the base?
Conversation
array `colors['red', 'green']` must convert to `colors[]=red&colors[]=green` but not `colors=red&colors=green`
Hiroshi Nakamura » httpclient #86 FAILURE |
@nahi What do you think about? |
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. |
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 |
As far as I know, there is no "right answer" for it. |
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). |
I think that we can resolve this just added a configuration file with setting called like "query_params_pattern" |
Adding option sounds nice. |
array
colors['red', 'green']
must convert tocolors[]=red&colors[]=green
but notcolors=red&colors=green