From 10c363b588d734323533ea9186423d5dabccac58 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Fri, 1 May 2020 12:58:33 -0500 Subject: [PATCH] Remove PUT restriction for all columns/single row Fixes https://github.com/PostgREST/postgrest/issues/1452. The single row restriction can be lifted because the PUT will consider only the first object of the array. --- CHANGELOG.md | 1 + src/PostgREST/ApiRequest.hs | 11 +++++------ src/PostgREST/App.hs | 14 +++----------- src/PostgREST/Error.hs | 26 +++++++++----------------- src/PostgREST/Types.hs | 3 +-- test/Feature/UpsertSpec.hs | 28 ++++++++-------------------- 6 files changed, 27 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 496a60282..996032892 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #1471, Fix POST, PATCH, DELETE with ?select= and return=minimal and PATCH with empty body - @wolfgangwalther - #1500, Fix missing `openapi-server-proxy-uri` config option - @steve-chavez - #1508, Fix `Content-Profile` not working for POST RPC - @steve-chavez +- #1452, Fix PUT restriction for all columns - @steve-chavez ## [7.0.0] - 2020-04-03 diff --git a/src/PostgREST/ApiRequest.hs b/src/PostgREST/ApiRequest.hs index 471d6d5a2..9856e94aa 100644 --- a/src/PostgREST/ApiRequest.hs +++ b/src/PostgREST/ApiRequest.hs @@ -191,11 +191,10 @@ userApiRequest confSchemas rootSpec req reqBody (CTOther "application/x-www-form-urlencoded", _) -> let json = M.fromList . map (toS *** JSON.String . toS) . parseSimpleQuery $ toS reqBody keys = S.fromList $ M.keys json in - Right $ ProcessedJSON (JSON.encode json) PJObject keys + Right $ ProcessedJSON (JSON.encode json) keys (ct, _) -> Left $ toS $ "Content-Type not acceptable: " <> toMime ct - rpcPrmsToJson = ProcessedJSON (JSON.encode $ M.fromList $ second JSON.toJSON <$> rpcQParams) - PJObject (S.fromList $ fst <$> rpcQParams) + rpcPrmsToJson = ProcessedJSON (JSON.encode $ M.fromList $ second JSON.toJSON <$> rpcQParams) (S.fromList $ fst <$> rpcQParams) topLevelRange = fromMaybe allRange $ M.lookup "limit" ranges -- if no limit is specified, get all the request rows action = case method of @@ -334,14 +333,14 @@ payloadAttributes raw json = JSON.Object x -> S.fromList (M.keys x) == canonicalKeys _ -> False) arr in if areKeysUniform - then Just $ ProcessedJSON raw (PJArray $ V.length arr) canonicalKeys + then Just $ ProcessedJSON raw canonicalKeys else Nothing Just _ -> Nothing Nothing -> Just emptyPJArray - JSON.Object o -> Just $ ProcessedJSON raw PJObject (S.fromList $ M.keys o) + JSON.Object o -> Just $ ProcessedJSON raw (S.fromList $ M.keys o) -- truncate everything else to an empty array. _ -> Just emptyPJArray where - emptyPJArray = ProcessedJSON (JSON.encode emptyArray) (PJArray 0) S.empty + emptyPJArray = ProcessedJSON (JSON.encode emptyArray) S.empty diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index a0f2ae9a0..7057c4f49 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -219,22 +219,14 @@ app dbStructure proc cols conf apiRequest = else return $ responseLBS status headers rBody - (ActionSingleUpsert, TargetIdent (QualifiedIdentifier tSchema tName), Just ProcessedJSON{pjRaw, pjType, pjKeys}) -> + (ActionSingleUpsert, TargetIdent (QualifiedIdentifier tSchema tName), Just pJson) -> case mutateSqlParts tSchema tName of Left errorResponse -> return errorResponse - Right (sq, mq) -> do - let isSingle = case pjType of - PJArray len -> len == 1 - PJObject -> True - colNames = colName <$> tableCols dbStructure tSchema tName + Right (sq, mq) -> if topLevelRange /= allRange then return . errorResponseFor $ PutRangeNotAllowedError - else if not isSingle - then return . errorResponseFor $ PutSingletonError - else if S.fromList colNames /= pjKeys - then return . errorResponseFor $ PutPayloadIncompleteError else do - row <- H.statement (toS pjRaw) $ + row <- H.statement (toS $ pjRaw pJson) $ createWriteStatement sq mq (contentType == CTSingularJSON) False (contentType == CTTextCSV) (iPreferRepresentation apiRequest) [] pgVer let (_, queryTotal, _, body, gucHeaders) = row diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 51c0c177f..f325aa2ba 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -223,10 +223,8 @@ data SimpleError = GucHeadersError | BinaryFieldError ContentType | ConnectionLostError - | PutSingletonError | PutMatchingPkError | PutRangeNotAllowedError - | PutPayloadIncompleteError | JwtTokenMissing | JwtTokenInvalid Text | SingularityError Integer @@ -234,17 +232,15 @@ data SimpleError deriving (Show, Eq) instance PgrstError SimpleError where - status GucHeadersError = HT.status500 - status (BinaryFieldError _) = HT.status406 - status ConnectionLostError = HT.status503 - status PutSingletonError = HT.status400 - status PutMatchingPkError = HT.status400 - status PutRangeNotAllowedError = HT.status400 - status PutPayloadIncompleteError = HT.status400 - status JwtTokenMissing = HT.status500 - status (JwtTokenInvalid _) = HT.unauthorized401 - status (SingularityError _) = HT.status406 - status (ContentTypeError _) = HT.status415 + status GucHeadersError = HT.status500 + status (BinaryFieldError _) = HT.status406 + status ConnectionLostError = HT.status503 + status PutMatchingPkError = HT.status400 + status PutRangeNotAllowedError = HT.status400 + status JwtTokenMissing = HT.status500 + status (JwtTokenInvalid _) = HT.unauthorized401 + status (SingularityError _) = HT.status406 + status (ContentTypeError _) = HT.status415 headers (SingularityError _) = [toHeader CTSingularJSON] headers (JwtTokenInvalid m) = [toHeader CTApplicationJSON, invalidTokenHeader m] @@ -258,12 +254,8 @@ instance JSON.ToJSON SimpleError where toJSON ConnectionLostError = JSON.object [ "message" .= ("Database connection lost, retrying the connection." :: Text)] - toJSON PutSingletonError = JSON.object [ - "message" .= ("PUT payload must contain a single row" :: Text)] toJSON PutRangeNotAllowedError = JSON.object [ "message" .= ("Range header and limit/offset querystring parameters are not allowed for PUT" :: Text)] - toJSON PutPayloadIncompleteError = JSON.object [ - "message" .= ("You must specify all columns in the payload when using PUT" :: Text)] toJSON PutMatchingPkError = JSON.object [ "message" .= ("Payload values do not match URL in primary key column(s)" :: Text)] diff --git a/src/PostgREST/Types.hs b/src/PostgREST/Types.hs index 33e13906f..2cb11d586 100644 --- a/src/PostgREST/Types.hs +++ b/src/PostgREST/Types.hs @@ -309,9 +309,8 @@ data PayloadJSON = ProcessedJSON { -- | This is the raw ByteString that comes from the request body. -- We cache this instead of an Aeson Value because it was detected that for large payloads the encoding - -- had high memory usage, see #1005 for more details + -- had high memory usage, see https://github.com/PostgREST/postgrest/pull/1005 for more details pjRaw :: BL.ByteString - , pjType :: PJType -- | Keys of the object or if it's an array these keys are guaranteed to be the same across all its objects , pjKeys :: S.Set Text }| diff --git a/test/Feature/UpsertSpec.hs b/test/Feature/UpsertSpec.hs index 9c81a311a..7f7c3a8c5 100644 --- a/test/Feature/UpsertSpec.hs +++ b/test/Feature/UpsertSpec.hs @@ -180,26 +180,6 @@ spec = [json|{"message":"Range header and limit/offset querystring parameters are not allowed for PUT"}|] { matchStatus = 400 , matchHeaders = [matchContentTypeJson] } - it "fails if the payload has more than one row" $ - put "/tiobe_pls?name=eq.Go" - [str| [ { "name": "Go", "rank": 19 }, { "name": "Swift", "rank": 12 } ]|] - `shouldRespondWith` - [json|{"message":"PUT payload must contain a single row"}|] - { matchStatus = 400 , matchHeaders = [matchContentTypeJson] } - - it "fails if not all columns are specified" $ do - put "/tiobe_pls?name=eq.Go" - [str| [ { "name": "Go" } ]|] - `shouldRespondWith` - [json|{"message":"You must specify all columns in the payload when using PUT"}|] - { matchStatus = 400 , matchHeaders = [matchContentTypeJson] } - - put "/employees?first_name=eq.Susan&last_name=eq.Heidt" - [str| [ { "first_name": "Susan", "last_name": "Heidt", "salary": "48000" } ]|] - `shouldRespondWith` - [json|{"message":"You must specify all columns in the payload when using PUT"}|] - { matchStatus = 400 , matchHeaders = [matchContentTypeJson] } - it "rejects every other filter than pk cols eq's" $ do put "/tiobe_pls?rank=eq.19" [str| [ { "name": "Go", "rank": 19 } ]|] @@ -280,6 +260,14 @@ spec = put "/tiobe_pls?name=eq.Go" [str| [ { "name": "Go", "rank": 13 } ]|] `shouldRespondWith` 204 get "/tiobe_pls?name=eq.Go" `shouldRespondWith` [json| [ { "name": "Go", "rank": 13 } ]|] { matchHeaders = [matchContentTypeJson] } + it "succeeds if the payload has more than one row, but it only puts the first element" $ + request methodPut "/tiobe_pls?name=eq.Go" + [("Prefer", "return=representation"), ("Accept", "application/vnd.pgrst.object+json")] + [str| [ { "name": "Go", "rank": 19 }, { "name": "Swift", "rank": 12 } ] |] + `shouldRespondWith` + [json|{ "name": "Go", "rank": 19 }|] + { matchStatus = 200 , matchHeaders = [matchContentTypeSingular] } + it "succeeds on table with composite pk" $ do get "/employees?first_name=eq.Susan&last_name=eq.Heidt" `shouldRespondWith`