fix: add error body when Prefer: count=exact is used and offset is ou…

…t of bounds * Adds error body when Prefer: count=exact is used and offset is out of bounds * Adds details to differentiate between negative limits, lower boundaries greater than upper boundaries and out of bound ranges
PostgREST · Sep 15, 2022 · d5e6625 · d5e6625
1 parent 334f500
commit d5e6625
Show file tree

Hide file tree

Showing 5 changed files with 139 additions and 18 deletions.
diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs
@@ -291,9 +291,13 @@ handleRead headersOnly identifier context@RequestContext{..} = do
             )
           ]
           ++ contentTypeHeaders context
+        rsOrErrBody = if status == HTTP.status416
+          then Error.errorPayload $ Error.ApiRequestError $ ApiRequestTypes.InvalidRange
+            $ ApiRequestTypes.OutOfBounds (show $ RangeQuery.rangeOffset iTopLevelRange) (maybe "0" show total)
+          else LBS.fromStrict rsBody
 
       failNotSingular iAcceptMediaType rsQueryTotal . response status headers $
-        if headersOnly then mempty else LBS.fromStrict rsBody
+        if headersOnly then mempty else rsOrErrBody
 
     RSPlan plan ->
       pure $ Wai.responseLBS HTTP.status200 (contentTypeHeaders context) $ LBS.fromStrict plan
@@ -501,14 +505,18 @@ handleInvoke invMethod proc context@RequestContext{..} = do
       let
         (status, contentRange) =
           RangeQuery.rangeStatusHeader iTopLevelRange rsQueryTotal rsTableTotal
+        rsOrErrBody = if status == HTTP.status416
+          then Error.errorPayload $ Error.ApiRequestError $ ApiRequestTypes.InvalidRange
+            $ ApiRequestTypes.OutOfBounds (show $ RangeQuery.rangeOffset iTopLevelRange) (maybe "0" show rsTableTotal)
+          else LBS.fromStrict rsBody
 
       failNotSingular iAcceptMediaType rsQueryTotal $
         if Proc.procReturnsVoid proc then
           response HTTP.status204 [contentRange] mempty
         else
           response status
             (contentTypeHeaders context ++ [contentRange])
-            (if invMethod == InvHead then mempty else LBS.fromStrict rsBody)
+            (if invMethod == InvHead then mempty else rsOrErrBody)
 
     RSPlan plan ->
       pure $ Wai.responseLBS HTTP.status200 (contentTypeHeaders context) $ LBS.fromStrict plan

diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs
@@ -32,7 +32,8 @@ import Network.HTTP.Types.Header (Header)
 import           PostgREST.MediaType     (MediaType (..))
 import qualified PostgREST.MediaType     as MediaType
 import           PostgREST.Request.Types (ApiRequestError (..),
-                                          QPError (..))
+                                          QPError (..),
+                                          RangeError (..))
 
 import PostgREST.DbStructure.Identifiers  (QualifiedIdentifier (..))
 import PostgREST.DbStructure.Proc         (ProcDescription (..),
@@ -60,7 +61,7 @@ instance PgrstError ApiRequestError where
   status InvalidBody{}           = HTTP.status400
   status InvalidFilters          = HTTP.status405
   status InvalidRpcMethod{}      = HTTP.status405
-  status InvalidRange            = HTTP.status416
+  status InvalidRange{}          = HTTP.status416
   status NotFound                = HTTP.status404
   status NoRelBetween{}          = HTTP.status400
   status NoRpc{}                 = HTTP.status404
@@ -90,10 +91,13 @@ instance JSON.ToJSON ApiRequestError where
     "message" .= T.decodeUtf8 errorMessage,
     "details" .= JSON.Null,
     "hint"    .= JSON.Null]
-  toJSON InvalidRange = JSON.object [
+  toJSON (InvalidRange rangeError) = JSON.object [
     "code"    .= ApiRequestErrorCode03,
-    "message" .= ("HTTP Range error" :: Text),
-    "details" .= JSON.Null,
+    "message" .= ("Requested range not satisfiable" :: Text),
+    "details" .= (case rangeError of
+                   NegativeLimit           -> "Limit should be greater than or equal to zero."
+                   LowerGTUpper            -> "The lower boundary must be lower than or equal to the upper boundary in the Range header."
+                   OutOfBounds lower total -> "An offset of " <> lower <> " was requested, but there are only " <> total <> " rows."),
     "hint"    .= JSON.Null]
   toJSON (ParseRequestError message details) = JSON.object [
     "code"    .= ApiRequestErrorCode04,

diff --git a/src/PostgREST/Request/ApiRequest.hs b/src/PostgREST/Request/ApiRequest.hs
@@ -36,7 +36,8 @@ import Control.Arrow             ((***))
 import Data.Aeson.Types          (emptyArray, emptyObject)
 import Data.List                 (lookup, union)
 import Data.Maybe                (fromJust)
-import Data.Ranged.Ranges        (emptyRange, rangeIntersection)
+import Data.Ranged.Ranges        (emptyRange, rangeIntersection,
+                                  rangeIsEmpty)
 import Network.HTTP.Types.Header (hCookie)
 import Network.HTTP.Types.URI    (parseSimpleQuery)
 import Network.Wai               (Request (..))
@@ -64,7 +65,8 @@ import PostgREST.Request.Preferences     (PreferCount (..),
                                           PreferResolution (..),
                                           PreferTransaction (..))
 import PostgREST.Request.QueryParams     (QueryParams (..))
-import PostgREST.Request.Types           (ApiRequestError (..))
+import PostgREST.Request.Types           (ApiRequestError (..),
+                                          RangeError (..))
 
 import qualified PostgREST.MediaType           as MediaType
 import qualified PostgREST.Request.Preferences as Preferences
@@ -217,7 +219,7 @@ getAction PathInfo{pathIsProc, pathIsDefSpec} 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
-  | isInvalidRange = Left InvalidRange
+  | isInvalidRange = Left $ InvalidRange (if rangeIsEmpty headerRange then LowerGTUpper else NegativeLimit)
   | 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)
   | method `elem` ["PATCH", "DELETE"] && not (null qsRanges) && null qsOrder = Left LimitNoOrderError

diff --git a/src/PostgREST/Request/Types.hs b/src/PostgREST/Request/Types.hs
@@ -27,6 +27,7 @@ module PostgREST.Request.Types
   , OrderNulls(..)
   , OrderTerm(..)
   , QPError(..)
+  , RangeError(..)
   , SingleVal
   , TrileanVal(..)
   , SimpleOperator(..)
@@ -52,7 +53,7 @@ data ApiRequestError
   | MediaTypeError [ByteString]
   | InvalidBody ByteString
   | InvalidFilters
-  | InvalidRange
+  | InvalidRange RangeError
   | InvalidRpcMethod ByteString
   | LimitNoOrderError
   | NotFound
@@ -66,6 +67,10 @@ data ApiRequestError
   | UnsupportedMethod ByteString
 
 data QPError = QPError Text Text
+data RangeError
+  = NegativeLimit
+  | LowerGTUpper
+  | OutOfBounds Text Text
 
 type CallRequest = CallQuery
 

diff --git a/test/spec/Feature/Query/RangeSpec.hs b/test/spec/Feature/Query/RangeSpec.hs
@@ -39,6 +39,35 @@ spec = do
               [json| [{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":6},{"id":7},{"id":8},{"id":9},{"id":10},{"id":11},{"id":12},{"id":13},{"id":14},{"id":15}] |]
               { matchHeaders = ["Content-Range" <:> "0-14/*"] }
 
+      context "of invalid range" $ do
+        it "refuses a range with nonzero start when there are no items" $
+          request methodPost "/rpc/getitemrange?offset=1"
+                  [("Prefer", "count=exact")] emptyRange
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"An offset of 1 was requested, but there are only 0 rows.",
+                "hint":null
+              }|]
+            { matchStatus  = 416
+            , matchHeaders = ["Content-Range" <:> "*/0"]
+            }
+
+        it "refuses a range requesting start past last item" $
+          request methodPost "/rpc/getitemrange?offset=100"
+                  [("Prefer", "count=exact")] defaultRange
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"An offset of 100 was requested, but there are only 15 rows.",
+                "hint":null
+              }|]
+            { matchStatus  = 416
+            , matchHeaders = ["Content-Range" <:> "*/15"]
+            }
+
     context "with range headers" $ do
       context "of acceptable range" $ do
         it "succeeds with partial content" $ do
@@ -82,20 +111,39 @@ spec = do
         it "fails with 416 for offside range" $
           request methodPost  "/rpc/getitemrange"
                   (rangeHdrs $ ByteRangeFromTo 1 0) emptyRange
-            `shouldRespondWith` 416
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"The lower boundary must be lower than or equal to the upper boundary in the Range header.",
+                "hint":null
+              }|]
+            { matchStatus = 416 }
 
         it "refuses a range with nonzero start when there are no items" $
           request methodPost "/rpc/getitemrange"
                   (rangeHdrsWithCount $ ByteRangeFromTo 1 2) emptyRange
-            `shouldRespondWith` "[]"
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"An offset of 1 was requested, but there are only 0 rows.",
+                "hint":null
+              }|]
             { matchStatus  = 416
             , matchHeaders = ["Content-Range" <:> "*/0"]
             }
 
         it "refuses a range requesting start past last item" $
           request methodPost "/rpc/getitemrange"
                   (rangeHdrsWithCount $ ByteRangeFromTo 100 199) defaultRange
-            `shouldRespondWith` "[]"
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"An offset of 100 was requested, but there are only 15 rows.",
+                "hint":null
+              }|]
             { matchStatus  = 416
             , matchHeaders = ["Content-Range" <:> "*/15"]
             }
@@ -193,11 +241,46 @@ spec = do
 
       it "fails if limit is negative" $
         get "/items?select=id&limit=-1"
-          `shouldRespondWith` [json|{"message":"HTTP Range error","code":"PGRST103","details":null,"hint":null}|]
+          `shouldRespondWith`
+            [json| {
+              "message":"Requested range not satisfiable",
+              "code":"PGRST103",
+              "details":"Limit should be greater than or equal to zero.",
+              "hint":null
+            }|]
           { matchStatus  = 416
           , matchHeaders = [matchContentTypeJson]
           }
 
+      context "of invalid range" $ do
+        it "refuses a range with nonzero start when there are no items" $
+          request methodGet "/menagerie?offset=1"
+                  [("Prefer", "count=exact")] ""
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"An offset of 1 was requested, but there are only 0 rows.",
+                "hint":null
+              }|]
+            { matchStatus  = 416
+            , matchHeaders = ["Content-Range" <:> "*/0"]
+            }
+
+        it "refuses a range requesting start past last item" $
+          request methodGet "/items?offset=100"
+                  [("Prefer", "count=exact")] ""
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"An offset of 100 was requested, but there are only 15 rows.",
+                "hint":null
+              }|]
+            { matchStatus  = 416
+            , matchHeaders = ["Content-Range" <:> "*/15"]
+            }
+
     context "when count=planned" $ do
       it "obtains a filtered range" $ do
         request methodGet "/items?select=id&id=gt.8"
@@ -343,20 +426,39 @@ spec = do
         it "fails with 416 for offside range" $
           request methodGet  "/items"
                   (rangeHdrs $ ByteRangeFromTo 1 0) ""
-            `shouldRespondWith` 416
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"The lower boundary must be lower than or equal to the upper boundary in the Range header.",
+                "hint":null
+              }|]
+            { matchStatus = 416 }
 
         it "refuses a range with nonzero start when there are no items" $
           request methodGet "/menagerie"
                   (rangeHdrsWithCount $ ByteRangeFromTo 1 2) ""
-            `shouldRespondWith` "[]"
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"An offset of 1 was requested, but there are only 0 rows.",
+                "hint":null
+              }|]
             { matchStatus  = 416
             , matchHeaders = ["Content-Range" <:> "*/0"]
             }
 
         it "refuses a range requesting start past last item" $
           request methodGet "/items"
                   (rangeHdrsWithCount $ ByteRangeFromTo 100 199) ""
-            `shouldRespondWith` "[]"
+            `shouldRespondWith`
+              [json| {
+                "message":"Requested range not satisfiable",
+                "code":"PGRST103",
+                "details":"An offset of 100 was requested, but there are only 15 rows.",
+                "hint":null
+              }|]
             { matchStatus  = 416
             , matchHeaders = ["Content-Range" <:> "*/15"]
             }