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

Don't clobber request query string when passing params #77

Merged
merged 2 commits into from
Feb 26, 2015
Merged

Don't clobber request query string when passing params #77

merged 2 commits into from
Feb 26, 2015

Conversation

admtnnr
Copy link
Contributor

@admtnnr admtnnr commented Mar 17, 2013

I'm dealing with an API that requires the same query string key be passed multiple times. Normally Rack will parse the params and the last duplicate pair will win, but you can still parse the query string yourself if necessary. In certain cases in rack-test the query string is rebuilt from the parsed params which loses the duplicate keys altogether. This patch should fix that. Let me know if you have any questions.

Thanks!

@admtnnr
Copy link
Contributor Author

admtnnr commented Apr 6, 2013

Any update on this? Would love some feedback. 😄

@admtnnr
Copy link
Contributor Author

admtnnr commented Nov 16, 2013

Is there anything I can help with to get this merged?

uri.query = build_nested_query(params)
end

params = parse_nested_query(uri.query)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this, params is a local variable and not used afterwards.

@iGEL
Copy link
Contributor

iGEL commented Aug 30, 2014

Thanks for your contribution, it's definitely an improvement. But as mentioned your code is unnecessary complicated. Once you fix that, it should be merged.

@admtnnr
Copy link
Contributor Author

admtnnr commented Aug 30, 2014

Had to push up a commit to fix the Rakefile to get the specs passing again. If you bundle install --path vendor/bundle which seems to be pretty common it tries to run specs for all your dependencies as well leading to a failed build.

@iGEL
Copy link
Contributor

iGEL commented Aug 30, 2014

Wow, that was fast. 👍 (Travis will be fixed as well here, but doesn't hurt to have it here as well)

brynary added a commit that referenced this pull request Feb 26, 2015
Don't clobber request query string when passing params
@brynary brynary merged commit 56ddeda into rack:master Feb 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants