From ae9e27a0c71c6793d998c79141c4bdaccd6edeeb Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Mon, 19 Aug 2019 10:23:47 -0500 Subject: [PATCH] Fix #1348, go back to converting plus "+" to space Not doing this conflicts with some http clients and proxies. Use the alternative url-safe character '!' instead for disambiguating resource embedding. --- CHANGELOG.md | 5 +++++ src/PostgREST/ApiRequest.hs | 8 ++++---- src/PostgREST/Parsers.hs | 2 +- test/Feature/QuerySpec.hs | 19 ++++++++++--------- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3bf720fe..c119c8323 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - #1369, Change `raw-media-types` to accept a string of comma separated MIME types - @Dansvidania +- #1348, Go back to converting plus "+" to space " " in querystrings by default - @steve-chavez + +### Deprecated + +- #1348, Deprecate `.` symbol for disambiguating resource embedding(added in #918). The url-safe '!' should be used instead. We refrained from using `+` as part of our syntax because it conflicts with some http clients and proxies. ## [6.0.1] - 2019-07-30 diff --git a/src/PostgREST/ApiRequest.hs b/src/PostgREST/ApiRequest.hs index dfada17c2..599d0ea0f 100644 --- a/src/PostgREST/ApiRequest.hs +++ b/src/PostgREST/ApiRequest.hs @@ -138,14 +138,14 @@ userApiRequest schema rootSpec req reqBody , iCanonicalQS = toS $ urlEncodeVars . L.sortOn fst . map (join (***) toS . second (fromMaybe BS.empty)) - $ queryStringWPlus + $ qString , iJWT = tokenStr , iHeaders = [ (toS $ CI.foldedCase k, toS v) | (k,v) <- hdrs, k /= hAuthorization, k /= hCookie] , iCookies = maybe [] parseCookiesText $ lookupHeader "Cookie" } where - -- queryString with '+' not converted to ' ' - queryStringWPlus = parseQueryReplacePlus False $ rawQueryString req + -- queryString with '+' converted to ' '(space) + qString = parseQueryReplacePlus True $ rawQueryString req -- rpcQParams = Rpc query params e.g. /rpc/name?param1=val1, similar to filter but with no operator(eq, lt..) (filters, rpcQParams) = case action of @@ -215,7 +215,7 @@ userApiRequest schema rootSpec req reqBody path = pathInfo req method = requestMethod req hdrs = requestHeaders req - qParams = [(toS k, v)|(k,v) <- queryStringWPlus] + qParams = [(toS k, v)|(k,v) <- qString] lookupHeader = flip lookup hdrs hasPrefer :: Text -> Bool hasPrefer val = any (\(h,v) -> h == "Prefer" && val `elem` split v) hdrs diff --git a/src/PostgREST/Parsers.hs b/src/PostgREST/Parsers.hs index 342a3fe95..6f0cb65cf 100644 --- a/src/PostgREST/Parsers.hs +++ b/src/PostgREST/Parsers.hs @@ -131,7 +131,7 @@ pRelationSelect = lexeme $ try ( do alias <- optionMaybe ( try(pFieldName <* aliasSeparator) ) fld <- pField relationDetail <- optionMaybe ( - try ( char '+' *> pFieldName ) <|> + try ( char '!' *> pFieldName ) <|> try ( char '.' *> pFieldName ) -- TODO deprecated, remove in next major version ) diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index 56451d213..7e9cbddf5 100644 --- a/test/Feature/QuerySpec.hs +++ b/test/Feature/QuerySpec.hs @@ -480,12 +480,12 @@ spec actualPgVersion = do describe "path fixed" $ do it "works when requesting children 2 levels" $ - get "/clients?id=eq.1&select=id,projects:projects%2Bclient_id(id,tasks(id))" `shouldRespondWith` + get "/clients?id=eq.1&select=id,projects:projects!client_id(id,tasks(id))" `shouldRespondWith` [json|[{"id":1,"projects":[{"id":1,"tasks":[{"id":1},{"id":2}]},{"id":2,"tasks":[{"id":3},{"id":4}]}]}]|] { matchHeaders = [matchContentTypeJson] } it "works with parent relation" $ - get "/message?select=id,body,sender:person%2Bsender(name),recipient:person%2Brecipient(name)&id=lt.4" `shouldRespondWith` + get "/message?select=id,body,sender:person!sender(name),recipient:person!recipient(name)&id=lt.4" `shouldRespondWith` [json| [{"id":1,"body":"Hello Jane","sender":{"name":"John"},"recipient":{"name":"Jane"}}, {"id":2,"body":"Hi John","sender":{"name":"Jane"},"recipient":{"name":"John"}}, @@ -499,7 +499,7 @@ spec actualPgVersion = do , matchHeaders = [matchContentTypeJson] } it "works with a parent view relation" $ - get "/message?select=id,body,sender:person_detail%2Bsender(name,sent),recipient:person_detail%2Brecipient(name,received)&id=lt.4" `shouldRespondWith` + get "/message?select=id,body,sender:person_detail!sender(name,sent),recipient:person_detail!recipient(name,received)&id=lt.4" `shouldRespondWith` [json| [{"id":1,"body":"Hello Jane","sender":{"name":"John","sent":2},"recipient":{"name":"Jane","received":2}}, {"id":2,"body":"Hi John","sender":{"name":"Jane","sent":1},"recipient":{"name":"John","received":1}}, @@ -507,10 +507,11 @@ spec actualPgVersion = do { matchHeaders = [matchContentTypeJson] } it "works with many<->many relation" $ - get "/tasks?select=id,users:users%2Busers_tasks(id)" `shouldRespondWith` + get "/tasks?select=id,users:users!users_tasks(id)" `shouldRespondWith` [json|[{"id":1,"users":[{"id":1},{"id":3}]},{"id":2,"users":[{"id":1}]},{"id":3,"users":[{"id":1}]},{"id":4,"users":[{"id":1}]},{"id":5,"users":[{"id":2},{"id":3}]},{"id":6,"users":[{"id":2}]},{"id":7,"users":[{"id":2}]},{"id":8,"users":[]}]|] { matchHeaders = [matchContentTypeJson] } + -- TODO Remove in next major version(7.0) describe "old dot '.' symbol, deprecated" $ it "still works" $ do get "/clients?id=eq.1&select=id,projects:projects.client_id(id,tasks(id))" `shouldRespondWith` @@ -588,7 +589,7 @@ spec actualPgVersion = do { matchHeaders = [matchContentTypeJson] } it "embeds childs recursively" $ - get "/family_tree?id=eq.1&select=id,name, childs:family_tree%2Bparent(id,name,childs:family_tree%2Bparent(id,name))" `shouldRespondWith` + get "/family_tree?id=eq.1&select=id,name, childs:family_tree!parent(id,name,childs:family_tree!parent(id,name))" `shouldRespondWith` [json|[{ "id": "1", "name": "Parental Unit", "childs": [ { "id": "2", "name": "Kid One", "childs": [ { "id": "4", "name": "Grandkid One" } ] }, @@ -597,7 +598,7 @@ spec actualPgVersion = do }]|] { matchHeaders = [matchContentTypeJson] } it "embeds parent and then embeds childs" $ - get "/family_tree?id=eq.2&select=id,name,parent(id,name,childs:family_tree%2Bparent(id,name))" `shouldRespondWith` + get "/family_tree?id=eq.2&select=id,name,parent(id,name,childs:family_tree!parent(id,name))" `shouldRespondWith` [json|[{ "id": "2", "name": "Kid One", "parent": { "id": "1", "name": "Parental Unit", "childs": [ { "id": "2", "name": "Kid One" }, { "id": "3", "name": "Kid Two"} ] @@ -620,7 +621,7 @@ spec actualPgVersion = do }]|] { matchHeaders = [matchContentTypeJson] } it "embeds childs" $ do - get "/organizations?select=id,name,refereeds:organizations%2Breferee(id,name)&id=eq.1" `shouldRespondWith` + get "/organizations?select=id,name,refereeds:organizations!referee(id,name)&id=eq.1" `shouldRespondWith` [json|[{ "id": 1, "name": "Referee Org", "refereeds": [ @@ -634,7 +635,7 @@ spec actualPgVersion = do } ] }]|] { matchHeaders = [matchContentTypeJson] } - get "/organizations?select=id,name,auditees:organizations%2Bauditor(id,name)&id=eq.2" `shouldRespondWith` + get "/organizations?select=id,name,auditees:organizations!auditor(id,name)&id=eq.2" `shouldRespondWith` [json|[{ "id": 2, "name": "Auditor Org", "auditees": [ @@ -668,7 +669,7 @@ spec actualPgVersion = do "manager":{"name":"Referee Manager"}}} }]|] { matchHeaders = [matchContentTypeJson] } - get "/organizations?select=name,manager(name),auditees:organizations%2Bauditor(name,manager(name),refereeds:organizations%2Breferee(name,manager(name)))&id=eq.2" `shouldRespondWith` + get "/organizations?select=name,manager(name),auditees:organizations!auditor(name,manager(name),refereeds:organizations!referee(name,manager(name)))&id=eq.2" `shouldRespondWith` [json|[{ "name":"Auditor Org", "manager":{"name":"Auditor Manager"},