diff --git a/src/PostgREST/ApiRequest.hs b/src/PostgREST/ApiRequest.hs index 3dbce3d8a..2b4be203b 100644 --- a/src/PostgREST/ApiRequest.hs +++ b/src/PostgREST/ApiRequest.hs @@ -182,9 +182,9 @@ data ApiRequest = ApiRequest { -- | Examines HTTP request and translates it into user intent. userApiRequest :: AppConfig -> SchemaCache -> Request -> RequestBody -> Either ApiRequestError ApiRequest userApiRequest conf sCache req reqBody = do - qPrms <- first QueryParamError $ QueryParams.parse $ rawQueryString req pInfo <- getPathInfo conf $ pathInfo req act <- getAction pInfo $ requestMethod req + qPrms <- first QueryParamError $ QueryParams.parse (pathIsProc pInfo && act `elem` [ActionInvoke InvGet, ActionInvoke InvHead]) $ rawQueryString req mediaTypes <- getMediaTypes conf (requestHeaders req) act pInfo negotiatedSchema <- getSchema conf (requestHeaders req) (requestMethod req) apiRequest conf sCache req reqBody qPrms pInfo act mediaTypes negotiatedSchema @@ -253,7 +253,6 @@ apiRequest :: AppConfig -> SchemaCache -> Request -> RequestBody -> QueryParams. apiRequest conf sCache req reqBody queryparams@QueryParams{..} PathInfo{pathName, pathIsProc, pathIsRootSpec, pathIsDefSpec} action (acceptMediaType, contentMediaType) (schema, negotiatedByProfile) | 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 | method == "PUT" && topLevelRange /= allRange = Left PutRangeNotAllowedError | otherwise = do @@ -282,8 +281,6 @@ apiRequest conf sCache req reqBody queryparams@QueryParams{..} PathInfo{pathName , iBinaryField = bField } where - expectParams = pathIsProc && method /= "POST" - columns = case action of ActionMutate MutationCreate -> qsColumns ActionMutate MutationUpdate -> qsColumns diff --git a/src/PostgREST/ApiRequest/QueryParams.hs b/src/PostgREST/ApiRequest/QueryParams.hs index 97d9fa8bf..aafa4411f 100644 --- a/src/PostgREST/ApiRequest/QueryParams.hs +++ b/src/PostgREST/ApiRequest/QueryParams.hs @@ -114,39 +114,45 @@ data QueryParams = -- -- The canonical representation of the query string has parameters sorted alphabetically: -- --- >>> qsCanonical <$> parse "a=1&c=3&b=2&d" +-- >>> qsCanonical <$> parse True "a=1&c=3&b=2&d" -- Right "a=1&b=2&c=3&d=" -- -- 'select' is a reserved parameter that selects the fields to be returned: -- --- >>> qsSelect <$> parse "select=name,location" +-- >>> qsSelect <$> parse False "select=name,location" -- Right [Node {rootLabel = SelectField {selField = ("name",[]), selCast = Nothing, selAlias = Nothing}, subForest = []},Node {rootLabel = SelectField {selField = ("location",[]), selCast = Nothing, selAlias = Nothing}, subForest = []}] -- -- Filters are parameters whose value contains an operator, separated by a '.' from its value: -- --- >>> qsFilters <$> parse "a.b=eq.0" +-- >>> qsFilters <$> parse False "a.b=eq.0" -- Right [(["a"],Filter {field = ("b",[]), opExpr = OpExpr False (Op OpEqual "0")})] -- -- If the operator specified in a filter does not exist, parsing the query string fails: -- --- >>> qsFilters <$> parse "a.b=noop.0" --- Left (QPError "\"failed to parse filter (noop.0)\" (line 1, column 6)" "unknown single value operator noop") -parse :: ByteString -> Either QPError QueryParams -parse qs = - QueryParams - canonical - params - ranges - <$> pRequestOrder `traverse` order - <*> pRequestLogicTree `traverse` logic - <*> pRequestColumns columns - <*> pRequestSelect select - <*> pRequestFilter `traverse` filters - <*> (fmap snd <$> (pRequestFilter `traverse` filtersRoot)) - <*> pRequestFilter `traverse` filtersNotRoot - <*> pure (S.fromList (fst <$> filters)) - <*> pRequestOnConflict `traverse` onConflict +-- >>> qsFilters <$> parse False "a.b=noop.0" +-- Left (QPError "\"failed to parse filter (noop.0)\" (line 1, column 6)" "expecting operator (eq, gt, ...) unknown single value operator noop") +parse :: Bool -> ByteString -> Either QPError QueryParams +parse isRpcGet qs = do + rOrd <- pRequestOrder `traverse` order + rLogic <- pRequestLogicTree `traverse` logic + rCols <- pRequestColumns columns + rSel <- pRequestSelect select + (rFlts, params) <- L.partition hasOp <$> pRequestFilter isRpcGet `traverse` filters + (rFltsRoot, rFltsNotRoot) <- pure $ L.partition hasRootFilter rFlts + rOnConflict <- pRequestOnConflict `traverse` onConflict + + let rFltsFields = S.fromList (fst <$> filters) + params' = mapMaybe (\case {(_, Filter (fld, _) (NoOpExpr v)) -> Just (fld,v); _ -> Nothing}) params + rFltsRoot' = snd <$> rFltsRoot + + return $ QueryParams canonical params' ranges rOrd rLogic rCols rSel rFlts rFltsRoot' rFltsNotRoot rFltsFields rOnConflict where + hasRootFilter, hasOp :: (EmbedPath, Filter) -> Bool + hasRootFilter ([], _) = True + hasRootFilter _ = False + hasOp (_, Filter (_, _) (NoOpExpr _)) = False + hasOp _ = True + logic = filter (endingIn ["and", "or"] . fst) nonemptyParams select = fromMaybe "*" $ lookupParam "select" onConflict = lookupParam "on_conflict" @@ -173,32 +179,11 @@ parse qs = endingIn xx key = lastWord `elem` xx where lastWord = L.last $ T.split (== '.') key - (filters, params) = L.partition isParam filtersAndParams - isParam (k, v) = isEmbedPath k || hasOperator v || hasFtsOperator v - - filtersAndParams = filter (isFilterOrParam . fst) nonemptyParams - isFilterOrParam k = not (endingIn reservedEmbeddable k) && notElem k reserved + filters = filter (isFilter . fst) nonemptyParams + isFilter k = not (endingIn reservedEmbeddable k) && notElem k reserved reserved = ["select", "columns", "on_conflict"] reservedEmbeddable = ["order", "limit", "offset", "and", "or"] - (filtersNotRoot, filtersRoot) = L.partition isNotRoot filters - isNotRoot = flip T.isInfixOf "." . fst - - -- TODO: These checks are redundant to the parsers, should use parsers to differentiate params - hasOperator val = - case T.splitOn "." val of - "not" : _ : _ -> True - "is" : _ -> True - "in" : _ -> True - x : _ -> isJust (operator x) || isJust (ftsOperator x) - _ -> False - - hasFtsOperator val = - case T.splitOn "(" val of - x : _ : _ -> isJust $ ftsOperator x - _ -> False - - isEmbedPath = T.isInfixOf "." replaceLast x s = T.intercalate "." $ L.init (T.split (=='.') s) <> [x] ranges :: HM.HashMap Text (Range Integer) @@ -237,18 +222,8 @@ operator = \case "imatch" -> Just OpIMatch _ -> Nothing -ftsOperator :: Text -> Maybe FtsOperator -ftsOperator = \case - "fts" -> Just FilterFts - "plfts" -> Just FilterFtsPlain - "phfts" -> Just FilterFtsPhrase - "wfts" -> Just FilterFtsWebsearch - _ -> Nothing - - -- PARSERS - pRequestSelect :: Text -> Either QPError [Tree SelectItem] pRequestSelect selStr = mapError $ P.parse pFieldForest ("failed to parse select parameter (" <> toS selStr <> ")") (toS selStr) @@ -257,11 +232,25 @@ pRequestOnConflict :: Text -> Either QPError [FieldName] pRequestOnConflict oncStr = mapError $ P.parse pColumns ("failed to parse on_conflict parameter (" <> toS oncStr <> ")") (toS oncStr) -pRequestFilter :: (Text, Text) -> Either QPError (EmbedPath, Filter) -pRequestFilter (k, v) = mapError $ (,) <$> path <*> (Filter <$> fld <*> oper) +-- | +-- Parse `id=eq.1`(id, eq.1) into (EmbedPath, Filter) +-- +-- >>> pRequestFilter False ("id", "eq.1") +-- Right ([],Filter {field = ("id",[]), opExpr = OpExpr False (Op OpEqual "1")}) +-- +-- >>> pRequestFilter False ("id", "val") +-- Left (QPError "\"failed to parse filter (val)\" (line 1, column 4)" "unexpected end of input expecting operator (eq, gt, ...)") +-- +-- >>> pRequestFilter True ("id", "val") +-- Right ([],Filter {field = ("id",[]), opExpr = NoOpExpr "val"}) +pRequestFilter :: Bool -> (Text, Text) -> Either QPError (EmbedPath, Filter) +pRequestFilter isRpcGet (k, v) = mapError $ (,) <$> path <*> (Filter <$> fld <*> oper) where treePath = P.parse pTreePath ("failed to parse tree path (" ++ toS k ++ ")") $ toS k - oper = P.parse (pOpExpr pSingleVal) ("failed to parse filter (" ++ toS v ++ ")") $ toS v + oper = P.parse parseFlt ("failed to parse filter (" ++ toS v ++ ")") $ toS v + parseFlt = if isRpcGet + then pOpExpr pSingleVal <|> pure (NoOpExpr v) + else pOpExpr pSingleVal path = fst <$> treePath fld = snd <$> treePath @@ -592,12 +581,15 @@ pEmbedParams = do -- -- >>> P.parse (pOpExpr pSingleVal) "" "fts().value" -- Left (line 1, column 7): +-- expecting operator (eq, gt, ...) -- unknown single value operator fts() pOpExpr :: Parser SingleVal -> Parser OpExpr -pOpExpr pSVal = try ( string "not" *> pDelimiter *> (OpExpr True <$> pOperation)) <|> OpExpr False <$> pOperation +pOpExpr pSVal = do + boolExpr <- try (string "not" *> pDelimiter $> True) <|> pure False + OpExpr boolExpr <$> pOperation where pOperation :: Parser Operation - pOperation = pIn <|> pIs <|> try pFts <|> pOp "operator (eq, gt, ...)" + pOperation = pIn <|> pIs <|> try pFts <|> try pOp "operator (eq, gt, ...)" pIn = In <$> (try (string "in" *> pDelimiter) *> pListVal) pIs = Is <$> (try (string "is" *> pDelimiter) *> pTriVal) @@ -614,8 +606,11 @@ pOpExpr pSVal = try ( string "not" *> pDelimiter *> (OpExpr True <$> pOperation) "null or trilean value (unknown, true, false)" pFts = do - opStr <- try (P.many (noneOf ".(")) - op <- parseMaybe ("unknown fts operator " <> opStr) . ftsOperator $ toS opStr + op <- try (string "fts" $> FilterFts) + <|> try (string "plfts" $> FilterFtsPlain) + <|> try (string "phfts" $> FilterFtsPhrase) + <|> try (string "wfts" $> FilterFtsWebsearch) + lang <- optionMaybe $ try (between (char '(') (char ')') pIdentifier) pDelimiter >> Fts op (toS <$> lang) <$> pSVal diff --git a/src/PostgREST/ApiRequest/Types.hs b/src/PostgREST/ApiRequest/Types.hs index 0dc49ce78..df373dacc 100644 --- a/src/PostgREST/ApiRequest/Types.hs +++ b/src/PostgREST/ApiRequest/Types.hs @@ -76,7 +76,6 @@ data ApiRequestError | NoRelBetween Text Text (Maybe Text) Text RelationshipsMap | NoRpc Text Text [Text] Bool MediaType Bool [QualifiedIdentifier] [ProcDescription] | NotEmbedded Text - | ParseRequestError Text Text | PutRangeNotAllowedError | QueryParamError QPError | RelatedOrderNotToOne Text Text @@ -183,8 +182,9 @@ data Filter | FilterNullEmbed Bool FieldName deriving (Eq) -data OpExpr = - OpExpr Bool Operation +data OpExpr + = OpExpr Bool Operation + | NoOpExpr Text deriving (Eq) data Operation diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index b484f54c3..45c4ae8a7 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -72,7 +72,6 @@ instance PgrstError ApiRequestError where status NoRelBetween{} = HTTP.status400 status NoRpc{} = HTTP.status404 status NotEmbedded{} = HTTP.status400 - status ParseRequestError{} = HTTP.status400 status PutRangeNotAllowedError = HTTP.status400 status QueryParamError{} = HTTP.status400 status RelatedOrderNotToOne{} = HTTP.status400 @@ -109,11 +108,6 @@ instance JSON.ToJSON ApiRequestError where 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, - "message" .= message, - "details" .= details, - "hint" .= JSON.Null] toJSON InvalidFilters = JSON.object [ "code" .= ApiRequestErrorCode05, "message" .= ("Filters must include all and only primary key columns with 'eq' operators" :: Text), @@ -586,7 +580,7 @@ data ErrorCode | ApiRequestErrorCode01 | ApiRequestErrorCode02 | ApiRequestErrorCode03 - | ApiRequestErrorCode04 + | ApiRequestErrorCode04 -- no longer used (used to be mapped to ParseRequestError) | ApiRequestErrorCode05 | ApiRequestErrorCode06 | ApiRequestErrorCode07 diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index dee719d94..55c2afd84 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -33,7 +33,6 @@ import Data.Tree (Tree (..)) import PostgREST.ApiRequest (Action (..), ApiRequest (..), - InvokeMethod (..), Mutation (..), Payload (..)) import PostgREST.Config (AppConfig (..)) @@ -291,11 +290,9 @@ addFilters ApiRequest{..} rReq = QueryParams.QueryParams{..} = iQueryParams flts = case iAction of - ActionInvoke InvGet -> qsFilters - ActionInvoke InvHead -> qsFilters - ActionInvoke _ -> qsFilters - ActionRead _ -> qsFilters - _ -> qsFiltersNotRoot + ActionInvoke _ -> qsFilters + ActionRead _ -> qsFilters + _ -> qsFiltersNotRoot addFilterToNode :: (EmbedPath, Filter) -> Either ApiRequestError ReadPlanTree -> Either ApiRequestError ReadPlanTree addFilterToNode = diff --git a/src/PostgREST/Query/SqlFragment.hs b/src/PostgREST/Query/SqlFragment.hs index 237a163f5..4548911a5 100644 --- a/src/PostgREST/Query/SqlFragment.hs +++ b/src/PostgREST/Query/SqlFragment.hs @@ -278,6 +278,7 @@ pgFmtOrderTerm qi ot = pgFmtFilter :: QualifiedIdentifier -> Filter -> SQL.Snippet pgFmtFilter _ (FilterNullEmbed hasNot fld) = SQL.sql (pgFmtIdent fld) <> " IS " <> (if hasNot then "NOT" else mempty) <> " NULL" +pgFmtFilter _ (Filter _ (NoOpExpr _)) = mempty -- TODO unreachable because NoOpExpr is filtered on QueryParams pgFmtFilter table (Filter fld (OpExpr hasNot oper)) = notOp <> " " <> case oper of Op op val -> pgFmtFieldOp op <> " " <> case op of OpLike -> unknownLiteral (T.map star val) diff --git a/test/spec/Feature/Query/QuerySpec.hs b/test/spec/Feature/Query/QuerySpec.hs index 7b4997aa4..b995dfe7c 100644 --- a/test/spec/Feature/Query/QuerySpec.hs +++ b/test/spec/Feature/Query/QuerySpec.hs @@ -1002,7 +1002,8 @@ spec actualPgVersion = do { matchHeaders = [matchContentTypeJson] } it "fails if an operator is not given" $ - get "/ghostBusters?id=0" `shouldRespondWith` [json| {"details":"Failed to parse [(\"id\",\"0\")]","message":"Unexpected param or filter missing operator","code":"PGRST104","hint":null} |] + get "/ghostBusters?id=0" `shouldRespondWith` + [json| {"code":"PGRST100","details":"unexpected end of input expecting operator (eq, gt, ...)","hint":null,"message":"\"failed to parse filter (0)\" (line 1, column 2)"} |] { matchStatus = 400 , matchHeaders = [matchContentTypeJson] }