From d49e3d7132cb7fb0e634d78955514ed5499353cd Mon Sep 17 00:00:00 2001 From: Laurence Isla Date: Wed, 6 Dec 2023 17:28:51 -0500 Subject: [PATCH] fix: allow using special characters in json keys (#3081) * increase memory size test --- CHANGELOG.md | 1 + src/PostgREST/ApiRequest/QueryParams.hs | 32 +++++++++++++++++++-- test/memory/memory-tests.sh | 2 +- test/spec/Feature/Query/JsonOperatorSpec.hs | 20 +++++++++++++ test/spec/fixtures/data.sql | 1 + 5 files changed, 53 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55fe6cfbb..9e2db89c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). + Does not apply to role settings and `app.settings.*` - #2420, Fix bogus message when listening on port 0 - @develop7 - #3067, Fix Acquision Timeout errors logging to stderr when `log-level=crit` - @laurenceisla + - #3054, Fix not allowing special characters in JSON keys - @laurenceisla ### Changed diff --git a/src/PostgREST/ApiRequest/QueryParams.hs b/src/PostgREST/ApiRequest/QueryParams.hs index ff7de07d2..7b2738357 100644 --- a/src/PostgREST/ApiRequest/QueryParams.hs +++ b/src/PostgREST/ApiRequest/QueryParams.hs @@ -350,8 +350,12 @@ pFieldForest = pFieldTree `sepBy` lexeme (char ',') pFieldName :: Parser Text pFieldName = pQuotedValue <|> - T.intercalate "-" . map toS <$> (pIdentifier `sepBy1` dash) + sepByDash pIdentifier "field name (* or [a..z0..9_$])" + +sepByDash :: Parser Text -> Parser Text +sepByDash fieldIdent = + T.intercalate "-" . map toS <$> (fieldIdent `sepBy1` dash) where isDash :: GenParser Char st () isDash = try ( char '-' >> notFollowedBy (char '>') ) @@ -364,12 +368,18 @@ pFieldName = -- >>> P.parse pJsonPath "" "->text" -- Right [JArrow {jOp = JKey {jVal = "text"}}] -- +-- >>> P.parse pJsonPath "" "->!@#$%^&*_a" +-- Right [JArrow {jOp = JKey {jVal = "!@#$%^&*_a"}}] +-- -- >>> P.parse pJsonPath "" "->1" -- Right [JArrow {jOp = JIdx {jVal = "+1"}}] -- -- >>> P.parse pJsonPath "" "->>text" -- Right [J2Arrow {jOp = JKey {jVal = "text"}}] -- +-- >>> P.parse pJsonPath "" "->>!@#$%^&*_a" +-- Right [J2Arrow {jOp = JKey {jVal = "!@#$%^&*_a"}}] +-- -- >>> P.parse pJsonPath "" "->>1" -- Right [J2Arrow {jOp = JIdx {jVal = "+1"}}] -- @@ -406,7 +416,7 @@ pJsonPath = many pJsonOperation try (string "->" $> JArrow) pJsonOperand = - let pJKey = JKey . toS <$> pFieldName + let pJKey = JKey . toS <$> pJsonKeyName pJIdx = JIdx . toS <$> ((:) <$> P.option '+' (char '-') <*> many1 digit) <* pEnd pEnd = try (void $ lookAhead (string "->")) <|> try (void $ lookAhead (string "::")) <|> @@ -415,6 +425,15 @@ pJsonPath = many pJsonOperation try eof in try pJIdx <|> try pJKey +pJsonKeyName :: Parser Text +pJsonKeyName = + pQuotedValue <|> + sepByDash pJsonKeyIdentifier + "any non reserved character different from: .,>()" + +pJsonKeyIdentifier :: Parser Text +pJsonKeyIdentifier = T.strip . toS <$> many1 (noneOf "(-:.,>)") + pField :: Parser Field pField = lexeme $ (,) <$> pFieldName <*> P.option [] pJsonPath @@ -477,6 +496,9 @@ pRelationSelect = lexeme $ do -- >>> P.parse pFieldSelect "" "alias:name->jsonpath::cast" -- Right (SelectField {selField = ("name",[JArrow {jOp = JKey {jVal = "jsonpath"}}]), selAggregateFunction = Nothing, selAggregateCast = Nothing, selCast = Just "cast", selAlias = Just "alias"}) -- +-- >>> P.parse pFieldSelect "" "alias:name->!@#$%^&*_a::cast" +-- Right (SelectField {selField = ("name",[JArrow {jOp = JKey {jVal = "!@#$%^&*_a"}}]), selAggregateFunction = Nothing, selAggregateCast = Nothing, selCast = Just "cast", selAlias = Just "alias"}) +-- -- >>> P.parse pFieldSelect "" "*" -- Right (SelectField {selField = ("*",[]), selAggregateFunction = Nothing, selAggregateCast = Nothing, selCast = Nothing, selAlias = Nothing}) -- @@ -678,9 +700,15 @@ pDelimiter = char '.' "delimiter (.)" -- >>> P.parse pOrder "" "json_col->key.asc.nullslast" -- Right [OrderTerm {otTerm = ("json_col",[JArrow {jOp = JKey {jVal = "key"}}]), otDirection = Just OrderAsc, otNullOrder = Just OrderNullsLast}] -- +-- >>> P.parse pOrder "" "json_col->!@#$%^&*_a.asc.nullslast" +-- Right [OrderTerm {otTerm = ("json_col",[JArrow {jOp = JKey {jVal = "!@#$%^&*_a"}}]), otDirection = Just OrderAsc, otNullOrder = Just OrderNullsLast}] +-- -- >>> P.parse pOrder "" "clients(json_col->key).desc.nullsfirst" -- Right [OrderRelationTerm {otRelation = "clients", otRelTerm = ("json_col",[JArrow {jOp = JKey {jVal = "key"}}]), otDirection = Just OrderDesc, otNullOrder = Just OrderNullsFirst}] -- +-- >>> P.parse pOrder "" "clients(json_col->!@#$%^&*_a).desc.nullsfirst" +-- Right [OrderRelationTerm {otRelation = "clients", otRelTerm = ("json_col",[JArrow {jOp = JKey {jVal = "!@#$%^&*_a"}}]), otDirection = Just OrderDesc, otNullOrder = Just OrderNullsFirst}] +-- -- >>> P.parse pOrder "" "clients(name,id)" -- Left (line 1, column 8): -- unexpected '(' diff --git a/test/memory/memory-tests.sh b/test/memory/memory-tests.sh index db2ae2d5b..9d91518e4 100755 --- a/test/memory/memory-tests.sh +++ b/test/memory/memory-tests.sh @@ -102,7 +102,7 @@ postJsonArrayTest(){ echo "Running memory usage tests.." -jsonKeyTest "1M" "POST" "/rpc/leak?columns=blob" "24M" +jsonKeyTest "1M" "POST" "/rpc/leak?columns=blob" "26M" jsonKeyTest "1M" "POST" "/leak?columns=blob" "16M" jsonKeyTest "1M" "PATCH" "/leak?id=eq.1&columns=blob" "16M" diff --git a/test/spec/Feature/Query/JsonOperatorSpec.hs b/test/spec/Feature/Query/JsonOperatorSpec.hs index ce1e16c25..fea74093f 100644 --- a/test/spec/Feature/Query/JsonOperatorSpec.hs +++ b/test/spec/Feature/Query/JsonOperatorSpec.hs @@ -56,6 +56,21 @@ spec actualPgVersion = describe "json and jsonb operators" $ do [json| [{"myInt":1}] |] -- the value in the db is an int, but here we expect a string for now { matchHeaders = [matchContentTypeJson] } + it "accepts non reserved special characters in the key's name" $ + get "/json_arr?id=eq.10&select=data->!@#$%^%26*_d->>!@#$%^%26*_e::integer" `shouldRespondWith` + [json| [{"!@#$%^&*_e":3}] |] + { matchHeaders = [matchContentTypeJson] } + + it "fails when there is a reserved special character in the key's name" $ + get "/json_arr?id=eq.10&select=data->(!@#$%^%26*_d->>!@#$%^%26*_e::integer" `shouldRespondWith` + [json| { + "code":"PGRST100", + "details":"unexpected \"(\" expecting \"-\", digit or any non reserved character different from: .,>()", + "hint":null, + "message":"\"failed to parse select parameter (data->(!@#$%^&*_d->>!@#$%^&*_e::integer)\" (line 1, column 7)"} + |] + { matchStatus = 400 , matchHeaders = [] } + -- TODO the status code for the error is 404, this is because 42883 represents undefined function -- this works fine for /rpc/unexistent requests, but for this case a 500 seems more appropriate it "fails when a double arrow ->> is followed with a single arrow ->" $ do @@ -178,6 +193,11 @@ spec actualPgVersion = describe "json and jsonb operators" $ do get "/grandchild_entities?or=(jsonb_col->a->>b.eq.foo, jsonb_col->>b.eq.bar)&select=id" `shouldRespondWith` [json|[{id: 4}, {id: 5}]|] { matchStatus = 200, matchHeaders = [matchContentTypeJson] } + it "can filter when the key's name has non reserved special characters" $ + get "/json_arr?select=data->!@#$%^%26*_d&data->!@#$%^%26*_d->>!@#$%^%26*_e=eq.3" `shouldRespondWith` + [json| [{"!@#$%^&*_d": {"!@#$%^&*_e": 3}}] |] + { matchHeaders = [matchContentTypeJson] } + it "can filter by array indexes" $ do get "/json_arr?select=data&data->>0=eq.1" `shouldRespondWith` [json| [{"data":[1, 2, 3]}] |] diff --git a/test/spec/fixtures/data.sql b/test/spec/fixtures/data.sql index 6ccf5e896..68a83a230 100644 --- a/test/spec/fixtures/data.sql +++ b/test/spec/fixtures/data.sql @@ -512,6 +512,7 @@ INSERT INTO json_arr VALUES (6, '[{"a": [1,2,3]}, {"b": [4,5]}]'); INSERT INTO json_arr VALUES (7, '{"c": [1,2,3], "d": [4,5]}'); INSERT INTO json_arr VALUES (8, '{"c": [{"d": [4,5,6,7,8]}]}'); INSERT INTO json_arr VALUES (9, '[{"0xy1": [1,{"23-xy-45": [2, {"xy-6": [3]}]}]}]'); +INSERT INTO json_arr VALUES (10, '{"!@#$%^&*_a": [{"!@#$%^&*_b": 1}, {"!@#$%^&*_c": [2]}], "!@#$%^&*_d": {"!@#$%^&*_e": 3}}'); TRUNCATE TABLE jsonb_test CASCADE; INSERT INTO jsonb_test VALUES (1, '{ "a": {"b": 2} }');