Properly encode slashes in path parameters#280
Properly encode slashes in path parameters#280gwicke wants to merge 1 commit intoswagger-api:masterfrom
Conversation
Swagger spec version 2.0 only supports level 1 URI templating as described in http://tools.ietf.org/html/rfc6570#section-3.2.2. This means that the entire path component (including slashes) needs to be encoded with encodeURIComponent.
There was a problem hiding this comment.
It does the same thing as the old code, but not the same thing as
encodeURIComponent(pathParam);There was a problem hiding this comment.
That's true.
encodeURIComponent('a/b') === "a%2Fb"
'a/b'.split('/').map(encodeURIComponent).join('/') === 'a/b'
if we decided to have an option to allow slash(/), we would need to skip encoding it. Your code skip encoding slash. My suggestion is to make that code shorter and less complex.
Based on @fehguy's comment we need to decide if we want to skip slash in encoding and if the answer to that is yes, we should make this behavior configurable.
There was a problem hiding this comment.
@mohsen1, according to the URI template spec:
- slashes need to be encoded for
{foo}(the only variant supported in Swagger 2.0), and - slashes don't need to be encoded for
{+foo}.
So both can be supported easily and within the same URL once the spec gains support for level 2 URI templating.
This is a local version of swagger-api/swagger-js#280.
|
I believe the spec does not support nor prohibit encoding slashes in path params. Let me get input from @webron on this and if it is supported, I'll clean and merge this. |
|
We don't support non-encoded slashes in path parameter values. However, I don't see a problem if they are indeed encoded. Please note that this PR as against |
|
The UI still seems to have this not encoding, this would seem like a pretty silly thing to not encode values the user enters in the boxes for path parameters. Is it planned to change this? |
|
Which branch have you tested, @Gaillard? |
|
Hey @webron, I tried the develop_2.0 branch for swagger-ui with the same effect. Totally unrelated but also on the develop_2.0 branch when url into the SwaggerUi options is null it explodes, for those that are passing in a spec directly. Putting something silly in url fixes it it, I think it has to do with line 20949 of swagger-ui.js. |
|
@Gaillard - please open two separate issues on the above then, we won't be able to keep track of those as comments on a closed PR. |
|
will do thanks |
5 participants
Swagger spec version 2.0 only supports level 1 URI templating as described in
http://tools.ietf.org/html/rfc6570#section-3.2.2. This means that the entire
path component (including slashes) needs to be encoded with
encodeURIComponent.