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 an issue with URL encoding on cURL imports #5006

Merged
merged 5 commits into from
Aug 12, 2022

Conversation

brxxn
Copy link
Contributor

@brxxn brxxn commented Jul 26, 2022

Fixes an issue where application/x-www-form-urlencoded cURL body parameters are left URL encoded when being imported, causing them to be double-encoded (for example, body parameters that contain an encoded = were imported as %3D, which would be sent as %253D rather than just %3D).

This fix was implemented by using decodeURIComponent on the name/value pair since the insomnia form body expects it to be decoded and encodes the values when they are sent.

changelog(Fixes): Fixed an issue with cURL imports that caused URL-encoded parameters to be encoded twice

@filfreire filfreire requested a review from a team July 26, 2022 16:43
@brxxn
Copy link
Contributor Author

brxxn commented Aug 8, 2022

Should be resolved by latest commit on my branch

Copy link
Contributor

@johnwchadwick johnwchadwick left a comment

Choose a reason for hiding this comment

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

LGTM.

dimitropoulos and others added 5 commits August 12, 2022 10:58
fixes an issue where application/x-www-form-urlencoded curl body parameters are left URL encoded when being imported, causing them to be double-encoded (for example body parameters that contain an encoded "=" were imported as "%3D" which would be sent as "%253D" rather than just "%3D")
they are now double-encoded so when they get decoded they will return to what insomnia expects.
Copy link
Contributor

@dimitropoulos dimitropoulos left a comment

Choose a reason for hiding this comment

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

it's a sticky wicket, this one is. recently we had something similar in httpsnippet to deal with Kong/httpsnippet#198

cURL treats -data as being raw data but insomnia's curl importer parses it as form data if the content type is set to application/x-www-form-urlencoded - and insomnia will internally url encode the data that is input here, so if it is already url encoded it will encode it twice. the solution of encoding -data-urlencode arguments may seem a bit weird, but you can interleave -data and -data-urlencode so it needs to be normalized in some form ahead of time and this seems to be the way that makes the most sense.

this should make our handling of -data and -data-urlencode correct so that the cURL importer is able to correctly construct more request bodies.

@johnwchadwick and I wrote some tests for the preexisting behavior in 43556c1 and then updated the tests in 714a6b2 to reflect the new changes this PR brought about

@johnwchadwick
Copy link
Contributor

As a future step, I think it'd be nice to fall back to raw text body for the request in the event that the cURL body cannot be correctly parsed as form data. (Example scenarios: a= couldn't be losslessly parsed since it has no value, so it will be missing the equals sign in the parsed form; a=1=2 couldn't be parsed since one of the equals signs would wind up being url encoded no matter where you split the key/value pair; % couldn't be parsed because it is not url encoded, so no possible form data could result in a single raw % being sent.)

@dimitropoulos dimitropoulos merged commit 6f227ff into Kong:develop Aug 12, 2022
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