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

[c-libcurl]: add support for url encoding #5158

Closed
wants to merge 0 commits into from

Conversation

zhemant
Copy link
Contributor

@zhemant zhemant commented Jan 29, 2020

Added support for url encoding (present only in the query params)

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@wing328 @ityuhui

@sebastien-rosset
Copy link
Contributor

Related to this, I have opened OAI/OpenAPI-Specification#2119 and #5117. I think it would be great to address URL path and query encoding in a consistent way across languages.

@zhemant
Copy link
Contributor Author

zhemant commented Jan 30, 2020

For URL path encoding, I believe it should have support right from the library instead of reinventing the whole encoding wheel again. I agree with the example you have given. But with libcurl I tried and it encodes "/" also this creates the problem in the server where the handlers do not recognize the URLs as they are encoded. Let me know if you get any update on encodings.

@sebastien-rosset
Copy link
Contributor

For URL path encoding, I believe it should have support right from the library instead of reinventing the whole encoding wheel again.

Yes, I completely agree we should not reinvent the wheel. By library, do you mean codegen? Per-language encoding under modules/openapi-generator/src/main/java/org/openapitools/codegen/languages? A mustache template invoking a language specific library? I think we may need a combination, but ideally as much as possible would be pushed to codegen.

I think codegen should be able to do some sanitization even before the

I agree with the example you have given. But with libcurl I tried and it encodes "/" also this creates the problem in the server where the handlers do not recognize the URLs as they are encoded. Let me know if you get any update on encodings.

Yes, I had a similar problem with golang. The standard golang library performs incorrect path encoding, and I had to use a different library that carefully encodes the URL path.

@zhemant
Copy link
Contributor Author

zhemant commented Jan 30, 2020

By library I mean for C client, we use the curl library to send the request. Curl library provides URL encoding functionality. I tried using it for the whole path but it creates more problem. and CURL is a standard library so my guessing is it is updated as per the required standards(what to encode and what not to encode).

replaceSpaceWithPlus(pair->value);
strcat(targetUrl, pair->value);
if(curl) {
char *output = curl_easy_escape(curl, pair->value, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

PATH : Http://SomeUrl.com:8080//store/curlybraces-%7B%7D/AZaz-09._~!$&'(abc)*+, ;=:@Œë🍇ℬ/order/template-{orderId}/.././c///g?c=3&a=1&b=9&c=0#target

This is how the above URL is supposed to be converted. Note characters like :, /, $ ! are not percent encoded.

ESCAPED: http://someurl.com:8080//store/curlybraces-%7B%7D/AZaz-09._~!$&'(abc)*+,%20;=:@%C5%92%C3%AB%F0%9F%8D%87%E2%84%AC/order/template-{orderId}/.././c///g?c=3&a=1&b=9&c=0#target

Using curl_easy_escape, I get a lot more character escapes:
Http%3A%2F%2FSomeUrl.com%3A8080%2F%2Fstore%2Fcurlybraces-%257B%257D%2FAZaz-09._~%21%24%26%27%28abc%29%2A%2B%2C%20%3B%3D%3A%40%C5%92%C3%AB%F0%9F%8D%87%E2%84%AC%2Forder%2Ftemplate-%7BorderId%7D%2F..%2F.%2Fc%2F%2F%2Fg%3Fc%3D3%26a%3D1%26b%3D9%26c%3D0%23target

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.

2 participants