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

Percent encoding #205

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/Data/Swagger/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ import Data.Swagger.Internal.AesonUtils (sopSwaggerGenericToJSON
,AesonDefaultValue(..)
,mkSwaggerAesonOptions
,saoAdditionalPairs
,saoSubObject)
,saoSubObject
,percentEncodeT
,percentDecodeT)
import Data.Swagger.Internal.Utils

#if MIN_VERSION_aeson(0,10,0)
Expand Down Expand Up @@ -1150,10 +1152,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 ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

I will fix the tests (if that will be necessary), once we come to a common understanding.


referencedToJSON :: ToJSON a => Text -> Referenced a -> Value
referencedToJSON prefix (Ref (Reference ref)) = object [ "$ref" .= (prefix <> ref) ]
referencedToJSON prefix (Ref (Reference ref)) =
object [ "$ref" .= (prefix <> percentEncodeT ref) ]
referencedToJSON _ (Inline x) = toJSON x

instance ToJSON (Referenced Schema) where toJSON = referencedToJSON "#/definitions/"
Expand Down Expand Up @@ -1314,7 +1317,7 @@ instance FromJSON SecurityDefinitions where
parseJSON js = SecurityDefinitions <$> parseJSON js

instance FromJSON Reference where
parseJSON (Object o) = Reference <$> o .: "$ref"
parseJSON (Object o) = Reference . percentDecodeT <$> o .: "$ref"
parseJSON _ = empty

referencedParseJSON :: FromJSON a => Text -> Value -> JSON.Parser (Referenced a)
Expand Down
57 changes: 57 additions & 0 deletions src/Data/Swagger/Internal/AesonUtils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ module Data.Swagger.Internal.AesonUtils (
saoPrefix,
saoAdditionalPairs,
saoSubObject,
-- * Percent encoding
percentEncodeS,
percentEncodeT,
percentDecodeS,
percentDecodeT,
) where

import Prelude ()
Expand Down Expand Up @@ -344,3 +349,55 @@ sopSwaggerGenericToEncoding'' (SwaggerAesonOptions prefix _ sub) = go
where (x, y) = span isUpper s

#endif

percentEncodeS :: String -> String
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would use urlEncode.

Copy link
Author

Choose a reason for hiding this comment

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

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.

percentEncodeS "" = ""
percentEncodeS ('!' : s) = "%21" <> percentEncodeS s
percentEncodeS ('#' : s) = "%23" <> percentEncodeS s
percentEncodeS ('$' : s) = "%24" <> percentEncodeS s
percentEncodeS ('%' : s) = "%25" <> percentEncodeS s
percentEncodeS ('&' : s) = "%26" <> percentEncodeS s
percentEncodeS ('\'' : s) = "%27" <> percentEncodeS s
percentEncodeS ('(' : s) = "%28" <> percentEncodeS s
percentEncodeS (')' : s) = "%29" <> percentEncodeS s
percentEncodeS ('*' : s) = "%2A" <> percentEncodeS s
percentEncodeS ('+' : s) = "%2B" <> percentEncodeS s
percentEncodeS (',' : s) = "%2C" <> percentEncodeS s
percentEncodeS ('/' : s) = "%2F" <> percentEncodeS s
percentEncodeS (':' : s) = "%3A" <> percentEncodeS s
percentEncodeS (';' : s) = "%3B" <> percentEncodeS s
percentEncodeS ('=' : s) = "%3D" <> percentEncodeS s
percentEncodeS ('?' : s) = "%3F" <> percentEncodeS s
percentEncodeS ('@' : s) = "%40" <> percentEncodeS s
percentEncodeS ('[' : s) = "%5B" <> percentEncodeS s
percentEncodeS (']' : s) = "%5D" <> percentEncodeS s
percentEncodeS (c : s) = c : percentEncodeS s

percentEncodeT :: Text -> Text
percentEncodeT = T.pack . percentEncodeS . T.unpack

percentDecodeS :: String -> String
percentDecodeS "" = ""
percentDecodeS ('%':'2':'1':s) = '!' : percentDecodeS s
percentDecodeS ('%':'2':'3':s) = '#' : percentDecodeS s
percentDecodeS ('%':'2':'4':s) = '$' : percentDecodeS s
percentDecodeS ('%':'2':'5':s) = '%' : percentDecodeS s
percentDecodeS ('%':'2':'6':s) = '&' : percentDecodeS s
percentDecodeS ('%':'2':'7':s) = '\'' : percentDecodeS s
percentDecodeS ('%':'2':'8':s) = '(' : percentDecodeS s
percentDecodeS ('%':'2':'9':s) = ')' : percentDecodeS s
percentDecodeS ('%':'2':'A':s) = '*' : percentDecodeS s
percentDecodeS ('%':'2':'B':s) = '+' : percentDecodeS s
percentDecodeS ('%':'2':'C':s) = ',' : percentDecodeS s
percentDecodeS ('%':'2':'F':s) = '/' : percentDecodeS s
percentDecodeS ('%':'3':'A':s) = ':' : percentDecodeS s
percentDecodeS ('%':'3':'B':s) = ';' : percentDecodeS s
percentDecodeS ('%':'3':'D':s) = '=' : percentDecodeS s
percentDecodeS ('%':'3':'F':s) = '?' : percentDecodeS s
percentDecodeS ('%':'4':'0':s) = '@' : percentDecodeS s
percentDecodeS ('%':'5':'B':s) = '[' : percentDecodeS s
percentDecodeS ('%':'5':'D':s) = ']' : percentDecodeS s
percentDecodeS (c : s) = c : percentDecodeS s

percentDecodeT :: Text -> Text
percentDecodeT = T.pack . percentDecodeS . T.unpack