swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
16.88k stars 6.03k forks source link

[SWIFT3] URL parameters are not escaped #6400

Open hce opened 7 years ago

hce commented 7 years ago
Description

swagger-codegen does not properly escape parametrized parts of a URL.

For example:

    var path = "/recipient/{recipient}"
    path = path.replacingOccurrences(of: "{recipient}", with: "\(recipient)", options: .literal, range: nil)

The "recipient" variable should be URLEscaped, but it isn't. Thus, if recipient contains spaces or illegal characters, the request fails. Depending on the source of the parameters, this could also lead to vulnerable code.

Swagger-codegen version

git commit id 18c57a65ed5def618be6495e86fd2d0c73f89882

Swagger declaration file content or url

n/a

Command line used for generation

not relevnt

Steps to reproduce

Use a swagger API that has parameters in the URL, for example: /article/{articleName}, then use "Hello World ../" as articleName

Related issues/PRs

n/a

Suggest a fix/enhancement

Properly url escape the passed parameters:

let param1 = param.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed)

wing328 commented 7 years ago

@hce I would suggest you to try the latest master if it's still an issue.

SNAPSHOT version: https://github.com/swagger-api/swagger-codegen#compatibility

hce commented 6 years ago

Sorry for the late reply; tested and it works fine! Thanks!

hce commented 6 years ago

My mistake, the issue still does exist.

Just tested again with the 30a176126aa3600b6252f80b84040a1ab351c125 commit (latest)

wing328 commented 6 years ago

@hce when you've time, can you also test the swift4 generator to see if it has similar issues?

cc @ehyche @jaz-ah @jgavris @Edubits

hce commented 6 years ago

Confirmed for the swift4 generator!

ehyche commented 6 years ago

@hce : thanks for checking on this. I should have a PR fixing this for swift4 next week.

ehyche commented 6 years ago

This PR was opened to fix this issue in swift3: https://github.com/swagger-api/swagger-codegen/pull/6705

xiaochow commented 6 years ago

@ehyche thanks for your work! The same issue still exists for swift4 generator, I have been manually escaping them in the client side code. I wonder if you could make the changes to that when you have time? Thank you!

wing328 commented 6 years ago

@xiaochow do you mind filing a PR to fix it in Swift4 generator?

xiaochow commented 6 years ago

@wing328 The solution I had was make changes to my client side code. I haven't figured out the how to fix it in codegen. But I will look at @ehyche 's PR and try to work on it.

wing328 commented 6 years ago

@xiaochow thanks. Let us know if you need help with the PR.