Skip to content

Commit f2a00dc

Browse files
taimoorzaeemtaimoorzaeem
committed
fix: mixing offset and limit with Range header
1 parent db85faf commit f2a00dc

17 files changed

+204
-215
lines changed

‎src/PostgREST/ApiRequest.hs

+14-25
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
Module : PostgREST.Request.ApiRequest
33
Description : PostgREST functions to translate HTTP request to a domain type called ApiRequest.
44
-}
5-
{-# LANGUAGE LambdaCase #-}
6-
{-# LANGUAGE NamedFieldPuns #-}
5+
{-# LANGUAGE LambdaCase #-}
6+
{-# LANGUAGE NamedFieldPuns #-}
7+
{-# LANGUAGE RecordWildCards #-}
78
-- TODO: This module shouldn't depend on SchemaCache
89
module PostgREST.ApiRequest
910
( ApiRequest(..)
@@ -35,8 +36,7 @@ import Data.Either.Combinators (mapBoth)
3536
import Control.Arrow ((***))
3637
import Data.Aeson.Types (emptyArray, emptyObject)
3738
import Data.List (lookup)
38-
import Data.Ranged.Ranges (emptyRange, rangeIntersection,
39-
rangeIsEmpty)
39+
import Data.Ranged.Ranges (rangeIsEmpty)
4040
import Network.HTTP.Types.Header (RequestHeaders, hCookie)
4141
import Network.HTTP.Types.URI (parseSimpleQuery)
4242
import Network.Wai (Request (..))
@@ -50,8 +50,6 @@ import PostgREST.Config (AppConfig (..),
5050
OpenAPIMode (..))
5151
import PostgREST.MediaType (MediaType (..))
5252
import PostgREST.RangeQuery (NonnegRange, allRange,
53-
convertToLimitZeroRange,
54-
hasLimitZero,
5553
rangeRequested)
5654
import PostgREST.SchemaCache (SchemaCache (..))
5755
import PostgREST.SchemaCache.Identifiers (FieldName,
@@ -111,8 +109,7 @@ data Action
111109
-}
112110
data ApiRequest = ApiRequest {
113111
iAction :: Action -- ^ Action on the resource
114-
, iRange :: HM.HashMap Text NonnegRange -- ^ Requested range of rows within response
115-
, iTopLevelRange :: NonnegRange -- ^ Requested range of rows from the top level
112+
, iRange :: NonnegRange -- ^ Requested range of rows from the selected resource
116113
, iPayload :: Maybe Payload -- ^ Data sent by client and used for mutation actions
117114
, iPreferences :: Preferences.Preferences -- ^ Prefer header values
118115
, iQueryParams :: QueryParams.QueryParams
@@ -134,12 +131,11 @@ userApiRequest conf req reqBody sCache = do
134131
(schema, negotiatedByProfile) <- getSchema conf hdrs method
135132
act <- getAction resource schema method
136133
qPrms <- first QueryParamError $ QueryParams.parse (actIsInvokeSafe act) $ rawQueryString req
137-
(topLevelRange, ranges) <- getRanges method qPrms hdrs
134+
hRange <- getRange method qPrms hdrs
138135
(payload, columns) <- getPayload reqBody contentMediaType qPrms act
139136
return $ ApiRequest {
140137
iAction = act
141-
, iRange = ranges
142-
, iTopLevelRange = topLevelRange
138+
, iRange = hRange
143139
, iPayload = payload
144140
, iPreferences = Preferences.fromHeaders (configDbTxAllowOverride conf) (dbTimezones sCache) hdrs
145141
, iQueryParams = qPrms
@@ -217,24 +213,17 @@ getSchema AppConfig{configDbSchemas} hdrs method = do
217213
acceptProfile = T.decodeUtf8 <$> lookupHeader "Accept-Profile"
218214
lookupHeader = flip lookup hdrs
219215

220-
getRanges :: ByteString -> QueryParams -> RequestHeaders -> Either ApiRequestError (NonnegRange, HM.HashMap Text NonnegRange)
221-
getRanges method QueryParams{qsOrder,qsRanges} hdrs
222-
| isInvalidRange = Left $ InvalidRange (if rangeIsEmpty headerRange then LowerGTUpper else NegativeLimit)
223-
| method `elem` ["PATCH", "DELETE"] && not (null qsRanges) && null qsOrder = Left LimitNoOrderError
224-
| method == "PUT" && topLevelRange /= allRange = Left PutLimitNotAllowedError
225-
| otherwise = Right (topLevelRange, ranges)
216+
getRange :: ByteString -> QueryParams -> RequestHeaders -> Either ApiRequestError NonnegRange
217+
getRange method QueryParams{..} hdrs
218+
| rangeIsEmpty headerRange = Left $ InvalidRange LowerGTUpper -- A Range is empty unless its upper boundary is GT its lower boundary
219+
| method `elem` ["PATCH","DELETE"] && not (null qsLimit) && null qsOrder = Left LimitNoOrderError
220+
| method == "PUT" && offsetLimitPresent = Left PutLimitNotAllowedError
221+
| otherwise = Right headerRange
226222
where
227223
-- According to the RFC (https://www.rfc-editor.org/rfc/rfc9110.html#name-range),
228224
-- the Range header must be ignored for all methods other than GET
229225
headerRange = if method == "GET" then rangeRequested hdrs else allRange
230-
limitRange = fromMaybe allRange (HM.lookup "limit" qsRanges)
231-
headerAndLimitRange = rangeIntersection headerRange limitRange
232-
-- Bypass all the ranges and send only the limit zero range (0 <= x <= -1) if
233-
-- limit=0 is present in the query params (not allowed for the Range header)
234-
ranges = HM.insert "limit" (convertToLimitZeroRange limitRange headerAndLimitRange) qsRanges
235-
-- The only emptyRange allowed is the limit zero range
236-
isInvalidRange = topLevelRange == emptyRange && not (hasLimitZero limitRange)
237-
topLevelRange = fromMaybe allRange $ HM.lookup "limit" ranges -- if no limit is specified, get all the request rows
226+
offsetLimitPresent = not (null qsOffset && null qsLimit)
238227

239228
getPayload :: RequestBody -> MediaType -> QueryParams.QueryParams -> Action -> Either ApiRequestError (Maybe Payload, S.Set FieldName)
240229
getPayload reqBody contentMediaType QueryParams{qsColumns} action = do

‎src/PostgREST/ApiRequest/QueryParams.hs

+68-61
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@
99
module PostgREST.ApiRequest.QueryParams
1010
( parse
1111
, QueryParams(..)
12-
, pRequestRange
12+
, pTreePath
1313
) where
1414

1515
import qualified Data.ByteString.Char8 as BS
16-
import qualified Data.HashMap.Strict as HM
1716
import qualified Data.List as L
1817
import qualified Data.Set as S
1918
import qualified Data.Text as T
@@ -22,42 +21,42 @@ import qualified Network.HTTP.Base as HTTP
2221
import qualified Network.HTTP.Types.URI as HTTP
2322
import qualified Text.ParserCombinators.Parsec as P
2423

25-
import Control.Arrow ((***))
26-
import Data.Either.Combinators (mapLeft)
27-
import Data.List (init, last)
28-
import Data.Ranged.Boundaries (Boundary (..))
29-
import Data.Ranged.Ranges (Range (..))
30-
import Data.Tree (Tree (..))
31-
import Text.Parsec.Error (errorMessages,
32-
showErrorMessages)
33-
import Text.ParserCombinators.Parsec (GenParser, ParseError, Parser,
34-
anyChar, between, char, choice,
35-
digit, eof, errorPos, letter,
36-
lookAhead, many1, noneOf,
37-
notFollowedBy, oneOf,
38-
optionMaybe, sepBy, sepBy1,
39-
string, try, (<?>))
40-
41-
import PostgREST.RangeQuery (NonnegRange, allRange,
42-
rangeGeq, rangeLimit,
43-
rangeOffset, restrictRange)
24+
import Control.Arrow ((***))
25+
import Data.Either.Combinators (mapLeft)
26+
import Data.List (init, last)
27+
import Data.Tree (Tree (..))
28+
import PostgREST.ApiRequest.Types (AggregateFunction (..),
29+
EmbedParam (..), EmbedPath,
30+
Field, Filter (..),
31+
FtsOperator (..), Hint,
32+
JoinType (..),
33+
JsonOperand (..),
34+
JsonOperation (..),
35+
JsonPath, ListVal,
36+
LogicOperator (..),
37+
LogicTree (..), OpExpr (..),
38+
OpQuantifier (..),
39+
Operation (..),
40+
OrderDirection (..),
41+
OrderNulls (..),
42+
OrderTerm (..),
43+
QPError (..),
44+
QuantOperator (..),
45+
SelectItem (..),
46+
SimpleOperator (..),
47+
SingleVal, TrileanVal (..))
4448
import PostgREST.SchemaCache.Identifiers (FieldName)
45-
46-
import PostgREST.ApiRequest.Types (AggregateFunction (..),
47-
EmbedParam (..), EmbedPath, Field,
48-
Filter (..), FtsOperator (..),
49-
Hint, JoinType (..),
50-
JsonOperand (..),
51-
JsonOperation (..), JsonPath,
52-
ListVal, LogicOperator (..),
53-
LogicTree (..), OpExpr (..),
54-
OpQuantifier (..), Operation (..),
55-
OrderDirection (..),
56-
OrderNulls (..), OrderTerm (..),
57-
QPError (..), QuantOperator (..),
58-
SelectItem (..),
59-
SimpleOperator (..), SingleVal,
60-
TrileanVal (..))
49+
import Text.Parsec.Error (errorMessages,
50+
showErrorMessages)
51+
import Text.ParserCombinators.Parsec (GenParser, ParseError,
52+
Parser, anyChar, between,
53+
char, choice, digit, eof,
54+
errorPos, letter, lookAhead,
55+
many1, noneOf,
56+
notFollowedBy, oneOf,
57+
optionMaybe, sepBy, sepBy1,
58+
string, try, (<?>))
59+
import Text.Read (read)
6160

6261
import Protolude hiding (Sum, try)
6362

@@ -67,8 +66,10 @@ data QueryParams =
6766
-- ^ Canonical representation of the query params, sorted alphabetically
6867
, qsParams :: [(Text, Text)]
6968
-- ^ Parameters for RPC calls
70-
, qsRanges :: HM.HashMap Text (Range Integer)
71-
-- ^ Ranges derived from &limit and &offset params
69+
, qsOffset :: [(EmbedPath, Integer)]
70+
-- ^ &offset parameter
71+
, qsLimit :: [(EmbedPath, Integer)]
72+
-- ^ &limit parameter
7273
, qsOrder :: [(EmbedPath, [OrderTerm])]
7374
-- ^ &order parameters for each level
7475
, qsLogic :: [(EmbedPath, LogicTree)]
@@ -115,6 +116,8 @@ parse :: Bool -> ByteString -> Either QPError QueryParams
115116
parse isRpcRead qs = do
116117
rOrd <- pRequestOrder `traverse` order
117118
rLogic <- pRequestLogicTree `traverse` logic
119+
rOffset <- pRequestOffset `traverse` offset
120+
rLimit <- pRequestLimit `traverse` limit
118121
rCols <- pRequestColumns columns
119122
rSel <- pRequestSelect select
120123
(rFlts, params) <- L.partition hasOp <$> pRequestFilter isRpcRead `traverse` filters
@@ -125,7 +128,7 @@ parse isRpcRead qs = do
125128
params' = mapMaybe (\case {(_, Filter (fld, _) (NoOpExpr v)) -> Just (fld,v); _ -> Nothing}) params
126129
rFltsRoot' = snd <$> rFltsRoot
127130

128-
return $ QueryParams canonical params' ranges rOrd rLogic rCols rSel rFlts rFltsRoot' rFltsNotRoot rFltsFields rOnConflict
131+
return $ QueryParams canonical params' rOffset rLimit rOrd rLogic rCols rSel rFlts rFltsRoot' rFltsNotRoot rFltsFields rOnConflict
129132
where
130133
hasRootFilter, hasOp :: (EmbedPath, Filter) -> Bool
131134
hasRootFilter ([], _) = True
@@ -138,9 +141,8 @@ parse isRpcRead qs = do
138141
onConflict = lookupParam "on_conflict"
139142
columns = lookupParam "columns"
140143
order = filter (endingIn ["order"] . fst) nonemptyParams
141-
limits = filter (endingIn ["limit"] . fst) nonemptyParams
142-
-- Replace .offset ending with .limit to be able to match those params later in a map
143-
offsets = first (replaceLast "limit") <$> filter (endingIn ["offset"] . fst) nonemptyParams
144+
offset = filter (endingIn ["offset"] . fst) nonemptyParams
145+
limit = filter (endingIn ["limit"] . fst) nonemptyParams
144146
lookupParam :: Text -> Maybe Text
145147
lookupParam needle = toS <$> join (L.lookup needle qParams)
146148
nonemptyParams = mapMaybe (\(k, v) -> (k,) <$> v) qParams
@@ -155,7 +157,7 @@ parse isRpcRead qs = do
155157
. map (join (***) BS.unpack . second (fromMaybe mempty))
156158
$ qString
157159

158-
endingIn:: [Text] -> Text -> Bool
160+
endingIn :: [Text] -> Text -> Bool
159161
endingIn xx key = lastWord `elem` xx
160162
where lastWord = L.last $ T.split (== '.') key
161163

@@ -164,21 +166,6 @@ parse isRpcRead qs = do
164166
reserved = ["select", "columns", "on_conflict"]
165167
reservedEmbeddable = ["order", "limit", "offset", "and", "or"]
166168

167-
replaceLast x s = T.intercalate "." $ L.init (T.split (=='.') s) <> [x]
168-
169-
ranges :: HM.HashMap Text (Range Integer)
170-
ranges = HM.unionWith f limitParams offsetParams
171-
where
172-
f rl ro = Range (BoundaryBelow o) (BoundaryAbove $ o + l - 1)
173-
where
174-
l = fromMaybe 0 $ rangeLimit rl
175-
o = rangeOffset ro
176-
177-
limitParams =
178-
HM.fromList [(k, restrictRange (readMaybe v) allRange) | (k,v) <- limits]
179-
180-
offsetParams =
181-
HM.fromList [(k, maybe allRange rangeGeq (readMaybe v)) | (k,v) <- offsets]
182169

183170
simpleOperator :: Parser SimpleOperator
184171
simpleOperator =
@@ -243,11 +230,19 @@ pRequestOrder (k, v) = mapError $ (,) <$> path <*> ord'
243230
path = fst <$> treePath
244231
ord' = P.parse pOrder ("failed to parse order (" ++ toS v ++ ")") $ toS v
245232

246-
pRequestRange :: (Text, NonnegRange) -> Either QPError (EmbedPath, NonnegRange)
247-
pRequestRange (k, v) = mapError $ (,) <$> path <*> pure v
233+
pRequestOffset :: (Text, Text) -> Either QPError (EmbedPath, Integer)
234+
pRequestOffset (k,v) = mapError $ (,) <$> path <*> int
248235
where
249236
treePath = P.parse pTreePath ("failed to parse tree path (" ++ toS k ++ ")") $ toS k
250237
path = fst <$> treePath
238+
int = P.parse pInt ("failed to parse offset parameter (" <> toS v <> ")") $ toS v
239+
240+
pRequestLimit :: (Text, Text) -> Either QPError (EmbedPath, Integer)
241+
pRequestLimit (k,v) = mapError $ (,) <$> path <*> int
242+
where
243+
treePath = P.parse pTreePath ("failed to parse tree path (" ++ toS k ++ ")") $ toS k
244+
path = fst <$> treePath
245+
int = P.parse pInt ("failed to parse limit parameter (" <> toS v <> ")") $ toS v
251246

252247
pRequestLogicTree :: (Text, Text) -> Either QPError (EmbedPath, LogicTree)
253248
pRequestLogicTree (k, v) = mapError $ (,) <$> embedPath <*> logicTree
@@ -842,6 +837,18 @@ pLogicPath = do
842837
notOp = "not." <> op
843838
return (filter (/= "not") (init path), if "not" `elem` path then notOp else op)
844839

840+
pInt :: Parser Integer
841+
pInt = pPosInt <|> pNegInt
842+
where
843+
pPosInt :: Parser Integer
844+
pPosInt = many1 digit <&> read
845+
846+
pNegInt :: Parser Integer
847+
pNegInt = do
848+
_ <- char '-'
849+
n <- many1 digit
850+
return ((-1) * read n)
851+
845852
pColumns :: Parser [FieldName]
846853
pColumns = pFieldName `sepBy1` lexeme (char ',')
847854

‎src/PostgREST/ApiRequest/Types.hs

+1-2
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,7 @@ data RaiseError
108108
| NoDetail
109109
deriving Show
110110
data RangeError
111-
= NegativeLimit
112-
| LowerGTUpper
111+
= LowerGTUpper
113112
| OutOfBounds Text Text
114113
deriving Show
115114

‎src/PostgREST/Error.hs

+10-6
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ instance PgrstError ApiRequestError where
8484
status UnacceptableFilter{} = HTTP.status400
8585
status UnacceptableSchema{} = HTTP.status406
8686
status UnsupportedMethod{} = HTTP.status405
87-
status LimitNoOrderError = HTTP.status400
87+
status LimitNoOrderError{} = HTTP.status400
8888
status ColumnNotFound{} = HTTP.status400
8989
status GucHeadersError = HTTP.status500
9090
status GucStatusError = HTTP.status500
@@ -118,7 +118,6 @@ instance JSON.ToJSON ApiRequestError where
118118
ApiRequestErrorCode03
119119
"Requested range not satisfiable"
120120
(Just $ case rangeError of
121-
NegativeLimit -> "Limit should be greater than or equal to zero."
122121
LowerGTUpper -> "The lower boundary must be lower than or equal to the upper boundary in the Range header."
123122
OutOfBounds lower total -> JSON.String $ "An offset of " <> lower <> " was requested, but there are only " <> total <> " rows.")
124123
Nothing
@@ -141,7 +140,10 @@ instance JSON.ToJSON ApiRequestError where
141140
(Just $ JSON.String $ "Verify that '" <> resource <> "' is included in the 'select' query parameter.")
142141

143142
toJSON LimitNoOrderError = toJsonPgrstError
144-
ApiRequestErrorCode09 "A 'limit' was applied without an explicit 'order'" Nothing (Just "Apply an 'order' using unique column(s)")
143+
ApiRequestErrorCode09
144+
"A 'limit' was applied without an explicit 'order'"
145+
Nothing
146+
(Just "Apply an 'order' using unique column(s)")
145147

146148
toJSON (OffLimitsChangesError n maxs) = toJsonPgrstError
147149
ApiRequestErrorCode10
@@ -475,13 +477,15 @@ pgErrorStatus authed (SQL.SessionUsageError (SQL.QueryError _ _ (SQL.ResultError
475477
'0':'9':_ -> HTTP.status500 -- triggered action exception
476478
'0':'L':_ -> HTTP.status403 -- invalid grantor
477479
'0':'P':_ -> HTTP.status403 -- invalid role specification
478-
"23503" -> HTTP.status409 -- foreign_key_violation
479-
"23505" -> HTTP.status409 -- unique_violation
480-
"25006" -> HTTP.status405 -- read_only_sql_transaction
481480
"21000" -> -- cardinality_violation
482481
if BS.isSuffixOf "requires a WHERE clause" m
483482
then HTTP.status400 -- special case for pg-safeupdate, which we consider as client error
484483
else HTTP.status500 -- generic function or view server error, e.g. "more than one row returned by a subquery used as an expression"
484+
"2201W" -> HTTP.status400 -- invalid/negative limit param
485+
"2201X" -> HTTP.status400 -- invalid/negative offset param
486+
"23503" -> HTTP.status409 -- foreign_key_violation
487+
"23505" -> HTTP.status409 -- unique_violation
488+
"25006" -> HTTP.status405 -- read_only_sql_transaction
485489
'2':'5':_ -> HTTP.status500 -- invalid tx state
486490
'2':'8':_ -> HTTP.status403 -- invalid auth specification
487491
'2':'D':_ -> HTTP.status500 -- invalid tx termination

0 commit comments

Comments
 (0)