Skip to content

Percent encoding #205

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.

Sign up for GitHub

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

Open
ilyakooo0 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Percent encoding #205

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a880be6
Added percent encoding
ilyakooo0 Mar 19, 2020
fa13c4b
encode Reference
ilyakooo0 Mar 19, 2020
cdf6de3
Removed swagger percent encoding
ilyakooo0 Mar 19, 2020
4145bdc
Removed unneeded import
ilyakooo0 Mar 24, 2020
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
12 changes: 8 additions & 4 deletions src/Data/Swagger/Internal.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import Control.Lens ((&), (.~), (?~))
import Control.Applicative
import Data.Aeson
import qualified Data.Aeson.Types as JSON
import Data.Coerce
import Data.Data (Data(..), Typeable, mkConstr, mkDataType, Fixity(..), Constr, DataType, constrIndex)
import Data.Hashable (Hashable)
import qualified Data.HashMap.Strict as HashMap
Expand Down Expand Up @@ -56,7 +57,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 +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 ]
Copy link
Collaborator

@fisx fisx Mar 20, 2020

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

@ilyakooo0 ilyakooo0 Mar 24, 2020

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

@ilyakooo0 ilyakooo0 Mar 24, 2020

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 +1318,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
View file
Open in desktop
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

@fisx fisx Mar 20, 2020

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

@ilyakooo0 ilyakooo0 Mar 24, 2020

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