diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f837b3d6..304b59a0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #1871, Fix OpenAPI missing default values for String types and identify Array types as "array" instead of "string" - @laurenceisla - #1930, Fix RPC return type handling for `RETURNS TABLE` with a single column. Regression of #1615. - @wolfgangwalther + - #1938, Fix using single double quotes(`"`) and backslashes(`/`) as values on the "in" operator - @steve-chavez ### Changed diff --git a/src/PostgREST/Query/SqlFragment.hs b/src/PostgREST/Query/SqlFragment.hs index 7ecb283ba..d1ec13550 100644 --- a/src/PostgREST/Query/SqlFragment.hs +++ b/src/PostgREST/Query/SqlFragment.hs @@ -123,7 +123,8 @@ normalizedBody body = singleParameter :: Maybe BL.ByteString -> ByteString -> H.Snippet singleParameter body typ = if typ == "bytea" - then H.encoderAndParam (HE.nullable HE.bytea) (toS <$> body) -- needed because bytea fails with HE.unknown(pg tries to utf8 encode) + -- TODO: Hasql fails when using HE.unknown with bytea(pg tries to utf8 encode). + then H.encoderAndParam (HE.nullable HE.bytea) (toS <$> body) else H.encoderAndParam (HE.nullable HE.unknown) (toS <$> body) <> "::" <> H.sql typ selectBody :: SqlFragment @@ -138,6 +139,15 @@ pgFmtLit x = then "E" <> slashed else slashed +-- Here we build the pg array literal, e.g '{"Hebdon, John","Other","Another"}', manually. +-- This is necessary to pass an "unknown" array and let pg infer the type. +pgFmtArrayLit :: [Text] -> Text +pgFmtArrayLit vals = + let trimmed = trimNullChars + slashed = T.replace "\\" "\\\\" . trimmed + escaped x = "\"" <> T.replace "\"" "\\\"" (slashed x) <> "\"" in + "{" <> T.intercalate "," (escaped <$> vals) <> "}" + -- TODO: refactor by following https://github.com/PostgREST/postgrest/pull/1631#issuecomment-711070833 pgFmtIdent :: Text -> SqlFragment pgFmtIdent x = encodeUtf8 $ "\"" <> T.replace "\"" "\"\"" (trimNullChars x) <> "\"" @@ -220,12 +230,9 @@ pgFmtFilter table (Filter fld (OpExpr hasNot oper)) = notOp <> " " <> case oper -- We don't use "IN", we use "= ANY". IN has the following disadvantages: -- + No way to use an empty value on IN: "col IN ()" is invalid syntax. With ANY we can do "= ANY('{}')" -- + Can invalidate prepared statements: multiple parameters on an IN($1, $2, $3) will lead to using different prepared statements and not take advantage of caching. - In vals -> pgFmtField table fld <> " " <> - case vals of + In vals -> pgFmtField table fld <> " " <> case vals of [""] -> "= ANY('{}') " - -- Here we build the pg array, e.g '{"Hebdon, John","Other","Another"}', manually. We quote the values to prevent the "," being treated as an element separator. - -- TODO: Ideally this would be done on Hasql with an encoder, but the "array unknown" is not working(Hasql doesn't pass any value). - _ -> "= ANY (" <> unknownLiteral ("{" <> T.intercalate "," ((\x -> "\"" <> x <> "\"") <$> vals) <> "}") <> ")" + _ -> "= ANY (" <> unknownLiteral (pgFmtArrayLit vals) <> ") " Fts op lang val -> pgFmtFieldOp op <> "(" <> ftsLang lang <> unknownLiteral val <> ") " diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index 57f989d1d..350216dc4 100644 --- a/test/Feature/QuerySpec.hs +++ b/test/Feature/QuerySpec.hs @@ -938,25 +938,37 @@ spec actualPgVersion = do get "/w_or_wo_comma_names?name=in.(\"Hebdon, John\",\"Williams, Mary\",\"Smith, Joseph\")" `shouldRespondWith` [json| [{"name":"Hebdon, John"},{"name":"Williams, Mary"},{"name":"Smith, Joseph"}] |] { matchHeaders = [matchContentTypeJson] } - get "/w_or_wo_comma_names?name=not.in.(\"Hebdon, John\",\"Williams, Mary\",\"Smith, Joseph\")" `shouldRespondWith` - [json| [{"name":"David White"},{"name":"Larry Thompson"},{"name":"Double O Seven(007)"}] |] + get "/w_or_wo_comma_names?name=not.in.(\"Hebdon, John\",\"Williams, Mary\",\"Smith, Joseph\")&limit=3" `shouldRespondWith` + [json| [ { "name": "David White" }, { "name": "Larry Thompson" }, { "name": "Double O Seven(007)" }] |] { matchHeaders = [matchContentTypeJson] } it "succeeds w/ and w/o quoted values" $ do get "/w_or_wo_comma_names?name=in.(David White,\"Hebdon, John\")" `shouldRespondWith` [json| [{"name":"Hebdon, John"},{"name":"David White"}] |] { matchHeaders = [matchContentTypeJson] } - get "/w_or_wo_comma_names?name=not.in.(\"Hebdon, John\",Larry Thompson,\"Smith, Joseph\")" `shouldRespondWith` - [json| [{"name":"Williams, Mary"},{"name":"David White"},{"name":"Double O Seven(007)"}] |] + get "/w_or_wo_comma_names?name=not.in.(\"Hebdon, John\",Larry Thompson,\"Smith, Joseph\")&limit=3" `shouldRespondWith` + [json| [ { "name": "Williams, Mary" }, { "name": "David White" }, { "name": "Double O Seven(007)" }] |] { matchHeaders = [matchContentTypeJson] } get "/w_or_wo_comma_names?name=in.(\"Double O Seven(007)\")" `shouldRespondWith` [json| [{"name":"Double O Seven(007)"}] |] { matchHeaders = [matchContentTypeJson] } - it "fails on malformed quoted values" $ do - get "/w_or_wo_comma_names?name=in.(\"\"Hebdon, John\")" `shouldRespondWith` 400 - get "/w_or_wo_comma_names?name=in.(\"\"Hebdon, John\"\"Mary)" `shouldRespondWith` 400 - get "/w_or_wo_comma_names?name=in.(Williams\"Hebdon, John\")" `shouldRespondWith` 400 + describe "IN values without quotes" $ do + it "accepts single double quotes as values" $ do + get "/w_or_wo_comma_names?name=in.(\")" `shouldRespondWith` + [json| [{ "name": "\"" }] |] + { matchHeaders = [matchContentTypeJson] } + get "/w_or_wo_comma_names?name=in.(Double\"Quote\"McGraw\")" `shouldRespondWith` + [json| [ { "name": "Double\"Quote\"McGraw\"" } ] |] + { matchHeaders = [matchContentTypeJson] } + + it "accepts backslashes as values" $ do + get "/w_or_wo_comma_names?name=in.(\\)" `shouldRespondWith` + [json| [{ "name": "\\" }] |] + { matchHeaders = [matchContentTypeJson] } + get "/w_or_wo_comma_names?name=in.(/\\Slash/\\Beast/\\)" `shouldRespondWith` + [json| [ { "name": "/\\Slash/\\Beast/\\" } ] |] + { matchHeaders = [matchContentTypeJson] } describe "IN and NOT IN empty set" $ do context "returns an empty result for IN when no value is present" $ do diff --git a/test/fixtures/data.sql b/test/fixtures/data.sql index 8fb81404a..5104c1777 100644 --- a/test/fixtures/data.sql +++ b/test/fixtures/data.sql @@ -345,6 +345,10 @@ INSERT INTO w_or_wo_comma_names VALUES ('Smith, Joseph'); INSERT INTO w_or_wo_comma_names VALUES ('David White'); INSERT INTO w_or_wo_comma_names VALUES ('Larry Thompson'); INSERT INTO w_or_wo_comma_names VALUES ('Double O Seven(007)'); +INSERT INTO w_or_wo_comma_names VALUES ('"'); +INSERT INTO w_or_wo_comma_names VALUES ('Double"Quote"McGraw"'); +INSERT INTO w_or_wo_comma_names VALUES ('\'); +INSERT INTO w_or_wo_comma_names VALUES ('/\Slash/\Beast/\'); TRUNCATE TABLE items_with_different_col_types CASCADE; INSERT INTO items_with_different_col_types VALUES (1, null, null, null, null, null, null, null);