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

Revert explicit escaping #27

Merged
merged 2 commits into from
Feb 14, 2024
Merged
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
21 changes: 7 additions & 14 deletions src/Scarf/Gateway/Rule.hs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import Data.Text (Text)
import Data.Text qualified as Text
import Data.Text.Encoding qualified as Text
import Network.HTTP.Types.URI (Query, parseQuery, parseQueryText, renderQuery)
import Network.URI (URI (..), escapeURIString, isUnreserved, parseRelativeReference, parseURI)
import Network.URI (URI (..), parseRelativeReference, parseURI)
import Network.Wai
( pathInfo,
rawQueryString,
Expand Down Expand Up @@ -643,11 +643,15 @@ rewriteQueryString ::
Wai.Request ->
ByteString
rewriteQueryString redirectTarget request
-- Fast path: In case there are no query parameters on the request
-- there is no need for rewriting.
| [] <- Wai.queryString request =
Text.encodeUtf8 redirectTarget
-- Parse the redirect target as a URI to extract, combine and replace
-- the query part of it.
-- TODO: there are an aweful lot of string conversions going on,
-- maybe there's a more direct way.
| Just uri@URI {uriQuery, uriPath} <- parseURI (Text.unpack redirectTarget) =
| Just uri@URI {uriQuery} <- parseURI (Text.unpack redirectTarget) =
let redirectTargetQuery =
parseQuery (Text.encodeUtf8 (Text.pack uriQuery))

Expand All @@ -672,18 +676,7 @@ rewriteQueryString redirectTarget request
)
in Text.encodeUtf8 $
Text.pack $
show
( uri
{ uriQuery =
Text.unpack renderedQuery,
uriPath =
-- We go over the URI path to escape anything that is worth escaping.
-- Unfortunately rendering the URI doesn't do any escaping automatically.
escapeURIString
(\c -> c == '/' || isUnreserved c)
uriPath
}
)
show (uri {uriQuery = Text.unpack renderedQuery})
-- In case the redirectTarget didn't parse as a URI we are not doing anything
-- and ideally shouldn't happen.
| otherwise =
Expand Down
Empty file.
12 changes: 12 additions & 0 deletions test/golden/file-package-escaping-1.output.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
capture: |-
Just
FlatfileCapture
{ fileAbsoluteUrl =
Just "https://output.com/some-nice-path/very-nice/really+nice"
, fileVariables =
fromList [ ( "path" , "some-nice-path/very-nice/really+nice" ) ]
, filePackage = "21c24cd1-73fa-4970-8a6a-bc570e55b91e"
}
headers:
Location: https://output.com/some-nice-path/very-nice/really+nice
status: 302
14 changes: 14 additions & 0 deletions test/golden/file-package-escaping-1.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# This tests a catch-all pattern with a file variable. This
# Using {+path} to render out the captured value will result
# in invalid encoding in the output.
# The encoding issue makes this a pattern that you don't want to use
path: "/some-nice-path/very-nice/really+nice"
headers:
Host: nicedomain.com
manifest:
rules:
- type: file-v1
incoming-path: "/{+path}"
domain: nicedomain.com
package-id: 21c24cd1-73fa-4970-8a6a-bc570e55b91e
outgoing-url: https://output.com/{+path}
8 changes: 3 additions & 5 deletions test/golden/file-package-escaping.output.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
capture: |-
Just
FlatfileCapture
{ fileAbsoluteUrl =
Just "https://output.com/some-nice-path/very-nice/really%2Bnice"
, fileVariables =
fromList [ ( "path" , "some-nice-path/very-nice/really+nice" ) ]
{ fileAbsoluteUrl = Just "https://output.com/really%2Bnice"
, fileVariables = fromList [ ( "variable" , "really+nice" ) ]
, filePackage = "21c24cd1-73fa-4970-8a6a-bc570e55b91e"
}
headers:
Location: https://output.com/some-nice-path/very-nice/really%2Bnice
Location: https://output.com/really%2Bnice
status: 302
4 changes: 2 additions & 2 deletions test/golden/file-package-escaping.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ headers:
manifest:
rules:
- type: file-v1
incoming-path: "/{+path}"
incoming-path: "/some-nice-path/very-nice/{+variable}"
domain: nicedomain.com
package-id: 21c24cd1-73fa-4970-8a6a-bc570e55b91e
outgoing-url: https://output.com/{+path}
outgoing-url: https://output.com/{variable}
Loading