fix: regression by reverting fix that returned 206 when first-pos=len…

…gth in Range header
PostgREST · Oct 21, 2023 · 317619b · 317619b
1 parent eb238ad
commit 317619b
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,7 +16,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Fixed
 
- - #2824, Fix range request with 0 rows and 0 offset return status 416 - @strengthless
+ - #2824, Fix regression by reverting fix that returned 206 when first position = length in a `Range` header - @laurenceisla, @strengthless
  - #3015, Fix unnecessary count() on RPC returning single - @steve-chavez
 
 ## [11.2.1] - 2023-10-03

diff --git a/src/PostgREST/RangeQuery.hs b/src/PostgREST/RangeQuery.hs
@@ -104,9 +104,9 @@ rangeStatusHeader topLevelRange queryTotal tableTotal =
     rangeStatus :: Integer -> Integer -> Maybe Integer -> Status
     rangeStatus _ _ Nothing = status200
     rangeStatus lower upper (Just total)
-      | lower >= total && lower /= upper && lower /= 0 = status416 -- 416 Range Not Satisfiable
-      | (1 + upper - lower) < total                    = status206 -- 206 Partial Content
-      | otherwise                                      = status200 -- 200 OK
+      | lower > total               = status416 -- 416 Range Not Satisfiable
+      | (1 + upper - lower) < total = status206 -- 206 Partial Content
+      | otherwise                   = status200 -- 200 OK
 
 contentRangeH :: (Integral a, Show a) => a -> a -> Maybe a -> Header
 contentRangeH lower upper total =

diff --git a/test/spec/Feature/Query/RangeSpec.hs b/test/spec/Feature/Query/RangeSpec.hs
@@ -145,6 +145,14 @@ spec = do
         it "returns whole range with status 200" $
           get "/items" `shouldRespondWith` 200
 
+      context "count with an empty body" $ do
+        it "returns empty body with Content-Range */0" $
+          request methodGet "/items?id=eq.0"
+            [("Prefer", "count=exact")] ""
+            `shouldRespondWith`
+              [json|[]|]
+              { matchHeaders = ["Content-Range" <:> "*/0"] }
+
       context "when I don't want the count" $ do
         it "returns range Content-Range with /*" $
           request methodGet "/menagerie"
@@ -211,11 +219,24 @@ spec = do
                              , "Content-Range" <:> "2-4/*" ]
             }
 
-      it "succeeds if offset equals 0 as a no-op" $
-        get "/items?select=id&offset=0&order=id"
-          `shouldRespondWith`
-            [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 "succeeds if offset equals 0 as a no-op" $ do
+        it  "no items" $ do
+          get "/items?offset=0&id=eq.0"
+            `shouldRespondWith`
+              [json|[]|]
+              { matchHeaders = ["Content-Range" <:> "*/*"] }
+
+          request methodGet "/items?offset=0&id=eq.0"
+            [("Prefer", "count=exact")] ""
+            `shouldRespondWith`
+              [json|[]|]
+              { matchHeaders = ["Content-Range" <:> "*/0"] }
+
+        it  "one or more items" $
+          get "/items?select=id&offset=0&order=id"
+            `shouldRespondWith`
+              [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/*"] }
 
       it "succeeds if offset is negative as a no-op" $
         get "/items?select=id&offset=-4&order=id"
@@ -413,15 +434,6 @@ spec = do
               matchHeader "Content-Range" "10-14/*"
             simpleStatus r `shouldBe` ok200
 
-        it "does not throw error when offset is 0 and and total is 0" $
-          request methodGet "/rpc/getitemrange?min=0&max=0"
-                  (rangeHdrsWithCount $ ByteRangeFromTo 0 1) mempty
-            `shouldRespondWith`
-            [json|[]|]
-            { matchStatus  = 200
-            , matchHeaders = ["Content-Range" <:> "*/0"]
-            }
-
       context "of invalid range" $ do
         it "fails with 416 for offside range" $
           request methodGet  "/items"
@@ -462,17 +474,3 @@ spec = do
             { matchStatus  = 416
             , matchHeaders = ["Content-Range" <:> "*/15"]
             }
-
-        it "refuses a range with first position the same as number of items" $
-          request methodGet "/rpc/getitemrange?min=1&max=2"
-                  (rangeHdrsWithCount $ ByteRangeFromTo 1 2) mempty
-            `shouldRespondWith`
-              [json| {
-                "message":"Requested range not satisfiable",
-                "code":"PGRST103",
-                "details":"An offset of 1 was requested, but there are only 1 rows.",
-                "hint":null
-              }|]
-            { matchStatus  = 416
-            , matchHeaders = ["Content-Range" <:> "*/1"]
-            }