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

Percent encode params that will be put into the url #237

Closed

Conversation

bschmaltz
Copy link
Contributor

Before, we did not percent escape params being put into the url, so invalid urls would be created (most commonly with '%' or '?' characters). When these invalid urls hit Apache, a 400 was returned, and bravado raised an exception when trying to parse the 400 (resulting in a 500 for the user).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 97.932% when pulling 56366ec on bschmaltz:schmaltz_percent_encode_url_params into 7146e3f on Yelp:master.

@sjaensch
Copy link
Contributor

Hey @bschmaltz, unfortunately I didn't have time to work on this today. I'd like to create a testcase and make sure that all HTTP clients handle this uniformly. I should have time to do it tomorrow. Thanks for submitting the bug report and the PR!

@sjaensch
Copy link
Contributor

Unfortunately the change to query param handling is not correct, I published #238 and have integration tests for bravado ready to publish once #238 is merged in.

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.

None yet

3 participants