diff --git a/CHANGELOG.md b/CHANGELOG.md index 6590ddc75..3fd493555 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #1959, An accidental full table PATCH(without filters) is not possible anymore, it requires filters or a `limit` parameter - @steve-chavez, @laurenceisla - #2317, Increase the `db-pool-timeout` to 1 hour to prevent frequent high connection latency - @steve-chavez - #2341, The search path now correctly identifies schemas with uppercase and special characters in their names (regression) - @laurenceisla - - #2364, "404 Not Found" on nested routes doesn't start an empty database transaction - @steve-chavez + - #2364, "404 Not Found" on nested routes and "405 Method Not Allowed" errors no longer start an empty database transaction - @steve-chavez ### Changed diff --git a/postgrest.cabal b/postgrest.cabal index 9d79664b8..7cfb7a55f 100644 --- a/postgrest.cabal +++ b/postgrest.cabal @@ -2,7 +2,7 @@ name: postgrest version: 9.0.1.20220630 synopsis: REST API for any Postgres database description: Reads the schema of a PostgreSQL database and creates RESTful routes - for tables, views, and functions, supporting all HTTP verbs that security + for tables, views, and functions, supporting all HTTP methods that security permits. license: MIT license-file: LICENSE diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 365cf561d..c8f80e267 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -54,12 +54,12 @@ class (JSON.ToJSON a) => PgrstError a where errorResponseFor err = responseLBS (status err) (headers err) $ errorPayload err instance PgrstError ApiRequestError where - status ActionInappropriate = HTTP.status405 status AmbiguousRelBetween{} = HTTP.status300 status AmbiguousRpc{} = HTTP.status300 status MediaTypeError{} = HTTP.status415 status InvalidBody{} = HTTP.status400 status InvalidFilters = HTTP.status405 + status InvalidRpcMethod{} = HTTP.status405 status InvalidRange = HTTP.status416 status NotFound = HTTP.status404 status NoRelBetween{} = HTTP.status400 @@ -69,6 +69,7 @@ instance PgrstError ApiRequestError where status PutRangeNotAllowedError = HTTP.status400 status QueryParamError{} = HTTP.status400 status UnacceptableSchema{} = HTTP.status406 + status UnsupportedMethod{} = HTTP.status405 status LimitNoOrderError = HTTP.status400 headers _ = [MediaType.toContentType MTApplicationJSON] @@ -79,9 +80,9 @@ instance JSON.ToJSON ApiRequestError where "message" .= message, "details" .= details, "hint" .= JSON.Null] - toJSON ActionInappropriate = JSON.object [ + toJSON (InvalidRpcMethod method) = JSON.object [ "code" .= ApiRequestErrorCode01, - "message" .= ("Bad Request" :: Text), + "message" .= ("Cannot use the " <> T.decodeUtf8 method <> " method on RPC"), "details" .= JSON.Null, "hint" .= JSON.Null] toJSON (InvalidBody errorMessage) = JSON.object [ @@ -133,6 +134,12 @@ instance JSON.ToJSON ApiRequestError where "details" .= JSON.Null, "hint" .= JSON.Null] + toJSON (UnsupportedMethod method) = JSON.object [ + "code" .= ApiRequestErrorCode17, + "message" .= ("Unsupported HTTP method: " <> T.decodeUtf8 method), + "details" .= JSON.Null, + "hint" .= JSON.Null] + toJSON (NoRelBetween parent child schema) = JSON.object [ "code" .= SchemaCacheErrorCode00, "message" .= ("Could not find a relationship between '" <> parent <> "' and '" <> child <> "' in the schema cache" :: Text), @@ -317,7 +324,6 @@ data Error | PgErr PgError | PutMatchingPkError | SingularityError Integer - | UnsupportedVerb Text instance PgrstError Error where status (ApiRequestError err) = status err @@ -332,7 +338,6 @@ instance PgrstError Error where status (PgErr err) = status err status PutMatchingPkError = HTTP.status400 status SingularityError{} = HTTP.status406 - status UnsupportedVerb{} = HTTP.status405 headers (ApiRequestError err) = headers err headers (JwtTokenInvalid m) = [MediaType.toContentType MTApplicationJSON, invalidTokenHeader m] @@ -398,12 +403,6 @@ instance JSON.ToJSON Error where "details" .= T.unwords ["Results contain", show n, "rows,", T.decodeUtf8 (MediaType.toMime MTSingularJSON), "requires 1 row"], "hint" .= JSON.Null] - toJSON (UnsupportedVerb verb) = JSON.object [ - "code" .= ApiRequestErrorCode17, - "message" .= ("Unsupported HTTP verb: " <> verb), - "details" .= JSON.Null, - "hint" .= JSON.Null] - toJSON (PgErr err) = JSON.toJSON err toJSON (ApiRequestError err) = JSON.toJSON err diff --git a/src/PostgREST/Request/ApiRequest.hs b/src/PostgREST/Request/ApiRequest.hs index 45df97998..847f17a16 100644 --- a/src/PostgREST/Request/ApiRequest.hs +++ b/src/PostgREST/Request/ApiRequest.hs @@ -97,7 +97,6 @@ data Action | ActionInvoke InvokeMethod | ActionInfo | ActionInspect {isHead :: Bool} - | ActionUnknown Text deriving Eq -- | The path info that will be mapped to a target (used to handle validations and errors before defining the Target) data PathInfo @@ -152,7 +151,7 @@ targetToJsonRpcParams target params = if it is an action we are able to perform. -} data ApiRequest = ApiRequest { - iAction :: Action -- ^ Similar but not identical to HTTP verb, e.g. Create/Invoke both POST + iAction :: Action -- ^ Similar but not identical to HTTP method, e.g. Create/Invoke both POST , iRange :: HM.HashMap Text NonnegRange -- ^ Requested range of rows within response , iTopLevelRange :: NonnegRange -- ^ Requested range of rows from the top level , iTarget :: Target -- ^ The target, be it calling a proc or accessing a table @@ -176,9 +175,10 @@ data ApiRequest = ApiRequest { -- | Examines HTTP request and translates it into user intent. userApiRequest :: AppConfig -> DbStructure -> Request -> RequestBody -> Either ApiRequestError ApiRequest userApiRequest conf dbStructure req reqBody = do - qPrms <- first QueryParamError (QueryParams.parse (rawQueryString req)) + qPrms <- first QueryParamError $ QueryParams.parse $ rawQueryString req pInfo <- getPathInfo conf $ pathInfo req - apiRequest conf dbStructure req reqBody qPrms pInfo + act <- getAction pInfo $ requestMethod req + apiRequest conf dbStructure req reqBody qPrms pInfo act getPathInfo :: AppConfig -> [Text] -> Either ApiRequestError PathInfo getPathInfo AppConfig{configOpenApiMode, configDbRootSpec} path = @@ -191,10 +191,30 @@ getPathInfo AppConfig{configOpenApiMode, configDbRootSpec} path = ["rpc", pName] -> Right $ PathInfo pName True False False _ -> Left NotFound -apiRequest :: AppConfig -> DbStructure -> Request -> RequestBody -> QueryParams.QueryParams -> PathInfo -> Either ApiRequestError ApiRequest -apiRequest conf@AppConfig{..} dbStructure req reqBody queryparams@QueryParams{..} path@PathInfo{pathName, pathIsProc, pathIsRootSpec, pathIsDefSpec} +getAction :: PathInfo -> ByteString -> Either ApiRequestError Action +getAction PathInfo{pathIsProc, pathIsDefSpec} method = + if pathIsProc && method `notElem` ["HEAD", "GET", "POST"] + then Left $ InvalidRpcMethod method + else case method of + -- The HEAD method is identical to GET except that the server MUST NOT return a message-body in the response + -- From https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.4 + "HEAD" | pathIsDefSpec -> Right $ ActionInspect{isHead=True} + | pathIsProc -> Right $ ActionInvoke InvHead + | otherwise -> Right $ ActionRead{isHead=True} + "GET" | pathIsDefSpec -> Right $ ActionInspect{isHead=False} + | pathIsProc -> Right $ ActionInvoke InvGet + | otherwise -> Right $ ActionRead{isHead=False} + "POST" | pathIsProc -> Right $ ActionInvoke InvPost + | otherwise -> Right $ ActionMutate MutationCreate + "PATCH" -> Right $ ActionMutate MutationUpdate + "PUT" -> Right $ ActionMutate MutationSingleUpsert + "DELETE" -> Right $ ActionMutate MutationDelete + "OPTIONS" -> Right ActionInfo + _ -> Left $ UnsupportedMethod method + +apiRequest :: AppConfig -> DbStructure -> Request -> RequestBody -> QueryParams.QueryParams -> PathInfo -> Action -> Either ApiRequestError ApiRequest +apiRequest conf@AppConfig{..} dbStructure req reqBody queryparams@QueryParams{..} path@PathInfo{pathName, pathIsProc, pathIsRootSpec, pathIsDefSpec} action | isJust profile && fromJust profile `notElem` configDbSchemas = Left $ UnacceptableSchema $ toList configDbSchemas - | pathIsProc && method `notElem` ["HEAD", "GET", "POST"] = Left ActionInappropriate | isInvalidRange = Left InvalidRange | shouldParsePayload && isLeft payload = either (Left . InvalidBody) witness payload | not expectParams && not (L.null qsParams) = Left $ ParseRequestError "Unexpected param or filter missing operator" ("Failed to parse " <> show qsParams) @@ -266,24 +286,6 @@ apiRequest conf@AppConfig{..} dbStructure req reqBody queryparams@QueryParams{.. (MTOctetStream, True) -> Right $ RawPay reqBody (ct, _) -> Left $ "Content-Type not acceptable: " <> MediaType.toMime ct topLevelRange = fromMaybe allRange $ HM.lookup "limit" ranges -- if no limit is specified, get all the request rows - action = - case method of - -- The HEAD method is identical to GET except that the server MUST NOT return a message-body in the response - -- From https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.4 - "HEAD" | pathIsDefSpec -> ActionInspect{isHead=True} - | pathIsProc -> ActionInvoke InvHead - | otherwise -> ActionRead{isHead=True} - "GET" | pathIsDefSpec -> ActionInspect{isHead=False} - | pathIsProc -> ActionInvoke InvGet - | otherwise -> ActionRead{isHead=False} - "POST" -> if pathIsProc - then ActionInvoke InvPost - else ActionMutate MutationCreate - "PATCH" -> ActionMutate MutationUpdate - "PUT" -> ActionMutate MutationSingleUpsert - "DELETE" -> ActionMutate MutationDelete - "OPTIONS" -> ActionInfo - _ -> ActionUnknown $ T.decodeUtf8 method defaultSchema = NonEmptyList.head configDbSchemas profile @@ -305,7 +307,7 @@ apiRequest conf@AppConfig{..} dbStructure req reqBody queryparams@QueryParams{.. target | pathIsProc = (`TargetProc` pathIsRootSpec) <$> callFindProc schema pathName | pathIsDefSpec = Right $ TargetDefaultSpec schema - | otherwise = Right $ TargetIdent $ QualifiedIdentifier schema pathName + | otherwise = Right $ TargetIdent $ QualifiedIdentifier schema pathName where callFindProc procSch procNam = findProc (QualifiedIdentifier procSch procNam) payloadColumns (preferParameters == Just SingleObject) (dbProcs dbStructure) diff --git a/src/PostgREST/Request/Types.hs b/src/PostgREST/Request/Types.hs index 6964baa98..cf8287934 100644 --- a/src/PostgREST/Request/Types.hs +++ b/src/PostgREST/Request/Types.hs @@ -47,13 +47,13 @@ import Protolude data ApiRequestError - = ActionInappropriate - | AmbiguousRelBetween Text Text [Relationship] + = AmbiguousRelBetween Text Text [Relationship] | AmbiguousRpc [ProcDescription] | MediaTypeError [ByteString] | InvalidBody ByteString | InvalidFilters | InvalidRange + | InvalidRpcMethod ByteString | LimitNoOrderError | NotFound | NoRelBetween Text Text Text @@ -63,6 +63,7 @@ data ApiRequestError | PutRangeNotAllowedError | QueryParamError QPError | UnacceptableSchema [Text] + | UnsupportedMethod ByteString data QPError = QPError Text Text diff --git a/test/spec/Feature/OptionsSpec.hs b/test/spec/Feature/OptionsSpec.hs index e6bf9e35f..1dd2fea33 100644 --- a/test/spec/Feature/OptionsSpec.hs +++ b/test/spec/Feature/OptionsSpec.hs @@ -16,7 +16,7 @@ import SpecHelper spec :: PgVersion -> SpecWith ((), Application) spec actualPgVersion = describe "Allow header" $ do context "a table" $ do - it "includes read/write verbs for writeable table" $ do + it "includes read/write methods for writeable table" $ do r <- request methodOptions "/items" [] "" liftIO $ simpleHeaders r `shouldSatisfy` @@ -24,7 +24,7 @@ spec actualPgVersion = describe "Allow header" $ do when (actualPgVersion >= pgVersion100) $ context "a partitioned table" $ do - it "includes read/write verbs for writeable partitioned tables" $ do + it "includes read/write methods for writeable partitioned tables" $ do r <- request methodOptions "/car_models" [] "" liftIO $ simpleHeaders r `shouldSatisfy` @@ -37,50 +37,50 @@ spec actualPgVersion = describe "Allow header" $ do context "a view" $ do context "auto updatable" $ do - it "includes read/write verbs for auto updatable views with pk" $ do + it "includes read/write methods for auto updatable views with pk" $ do r <- request methodOptions "/projects_auto_updatable_view_with_pk" [] "" liftIO $ simpleHeaders r `shouldSatisfy` matchHeader "Allow" "OPTIONS,GET,HEAD,POST,PUT,PATCH,DELETE" - it "includes read/write verbs for auto updatable views without pk" $ do + it "includes read/write methods for auto updatable views without pk" $ do r <- request methodOptions "/projects_auto_updatable_view_without_pk" [] "" liftIO $ simpleHeaders r `shouldSatisfy` matchHeader "Allow" "OPTIONS,GET,HEAD,POST,PATCH,DELETE" context "non auto updatable" $ do - it "includes read verbs for non auto updatable views" $ do + it "includes read methods for non auto updatable views" $ do r <- request methodOptions "/projects_view_without_triggers" [] "" liftIO $ simpleHeaders r `shouldSatisfy` matchHeader "Allow" "OPTIONS,GET,HEAD" - it "includes read/write verbs for insertable, updatable and deletable views with pk" $ do + it "includes read/write methods for insertable, updatable and deletable views with pk" $ do r <- request methodOptions "/projects_view_with_all_triggers_with_pk" [] "" liftIO $ simpleHeaders r `shouldSatisfy` matchHeader "Allow" "OPTIONS,GET,HEAD,POST,PUT,PATCH,DELETE" - it "includes read/write verbs for insertable, updatable and deletable views without pk" $ do + it "includes read/write methods for insertable, updatable and deletable views without pk" $ do r <- request methodOptions "/projects_view_with_all_triggers_without_pk" [] "" liftIO $ simpleHeaders r `shouldSatisfy` matchHeader "Allow" "OPTIONS,GET,HEAD,POST,PATCH,DELETE" - it "includes read and insert verbs for insertable views" $ do + it "includes read and insert methods for insertable views" $ do r <- request methodOptions "/projects_view_with_insert_trigger" [] "" liftIO $ simpleHeaders r `shouldSatisfy` matchHeader "Allow" "OPTIONS,GET,HEAD,POST" - it "includes read and update verbs for updatable views" $ do + it "includes read and update methods for updatable views" $ do r <- request methodOptions "/projects_view_with_update_trigger" [] "" liftIO $ simpleHeaders r `shouldSatisfy` matchHeader "Allow" "OPTIONS,GET,HEAD,PATCH" - it "includes read and delete verbs for deletable views" $ do + it "includes read and delete methods for deletable views" $ do r <- request methodOptions "/projects_view_with_delete_trigger" [] "" liftIO $ simpleHeaders r `shouldSatisfy` diff --git a/test/spec/Feature/Query/ErrorSpec.hs b/test/spec/Feature/Query/ErrorSpec.hs index 897c22666..681694d23 100644 --- a/test/spec/Feature/Query/ErrorSpec.hs +++ b/test/spec/Feature/Query/ErrorSpec.hs @@ -35,7 +35,7 @@ spec = do {"hint": null, "details": null, "code": "PGRST117", - "message":"Unsupported HTTP verb: CONNECT"}|] + "message":"Unsupported HTTP method: CONNECT"}|] { matchStatus = 405 } it "should return 405 for TRACE method" $ @@ -47,7 +47,7 @@ spec = do {"hint": null, "details": null, "code": "PGRST117", - "message":"Unsupported HTTP verb: TRACE"}|] + "message":"Unsupported HTTP method: TRACE"}|] { matchStatus = 405 } it "should return 405 for OTHER method" $ @@ -59,5 +59,5 @@ spec = do {"hint": null, "details": null, "code": "PGRST117", - "message":"Unsupported HTTP verb: OTHER"}|] + "message":"Unsupported HTTP method: OTHER"}|] { matchStatus = 405 } diff --git a/test/spec/Feature/Query/RpcSpec.hs b/test/spec/Feature/Query/RpcSpec.hs index 36d0ce21f..33fd43f4a 100644 --- a/test/spec/Feature/Query/RpcSpec.hs +++ b/test/spec/Feature/Query/RpcSpec.hs @@ -516,11 +516,11 @@ spec actualPgVersion = simpleStatus p `shouldBe` internalServerError500 isErrorFormat (simpleBody p) `shouldBe` True - context "unsupported verbs" $ do + context "unsupported method" $ do it "DELETE fails" $ request methodDelete "/rpc/sayhello" [] "" `shouldRespondWith` - [json|{"message":"Bad Request","code":"PGRST101","details":null,"hint":null}|] + [json|{"message":"Cannot use the DELETE method on RPC","code":"PGRST101","details":null,"hint":null}|] { matchStatus = 405 , matchHeaders = [matchContentTypeJson] }