Percent encoding #205
Percent encoding #205
Conversation
|
There is a test finding |
|
It seems like the test is finding it in a URI, which should be escaped. Am I wrong? |
| @@ -344,3 +349,55 @@ sopSwaggerGenericToEncoding'' (SwaggerAesonOptions prefix _ sub) = go | |||
| where (x, y) = span isUpper s | |||
|
|
|||
| #endif | |||
|
|
|||
| percentEncodeS :: String -> String | |||
There was a problem hiding this comment.
Looking through the source code, it doesn't escape all reserved characters: source.
It doesn't escape the sub-delims set of characters from RFC 3986, which isn't strictly necessary and is scheme-specific.
I think the desired behaviour is for mainstream tools (like https://editor.swagger.io) to accept all output as valid.
| @@ -1150,10 +1153,11 @@ instance ToJSON SecurityDefinitions where | |||
| toJSON (SecurityDefinitions sd) = toJSON sd | |||
|
|
|||
| instance ToJSON Reference where | |||
| toJSON (Reference ref) = object [ "$ref" .= ref ] | |||
| toJSON (Reference ref) = object [ "$ref" .= percentEncodeT ref ] | |||
There was a problem hiding this comment.
This seems to me the wrong place to do this. The text inside the Reference should really be a type that can only be turned into a string by encoding it; the fact that the string is also encoded if it appears inside a json object is coincidental.
There was a problem hiding this comment.
I'm not sure I understand what you are trying to say.
Reference references some other type, as you have alluded to.
IMO the Haskell data type should contain the literate string, which is the name of the referenced type.
So, if the type referenced is User', then the literal string contained in the Reference should be "User'". However, swagger requires all URIs (references) to be a proper URI, which requires special characters (such as ') to be escaped with percent-encoding. So, percent-encoding really is just the way the Reference type is converted to swagger.
And, as far as I understand it, in the scope of this library, ToJSON instances on swagger-related datatypes should produce a valid swagger description.
There was a problem hiding this comment.
I will fix the tests (if that will be necessary), once we come to a common understanding.
|
Thanks, btw! :) |
|
BTW, the way I found out about the need to percent-encode is by importing a generated swagger file, containing a type with a |
closes #204