From 43ad6d6aa0a72c4e8317cc7d1195ce2e00729269 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Sun, 8 Jan 2023 03:08:30 +0000 Subject: [PATCH] feat: validate `?columns` mutation targets based on schema cache (#2542) This returns an error for trying to update or insert into invalid columns, without hitting the database. This change also switches from `json_populate_recordset` for these operations `json_to_recordset` which should make no functional difference except allowing future flexibility. --- CHANGELOG.md | 1 + postgrest.cabal | 1 + src/PostgREST/ApiRequest/Types.hs | 5 +++-- src/PostgREST/Error.hs | 6 +++++ src/PostgREST/Plan.hs | 29 ++++++++++++++++-------- src/PostgREST/Plan/MutatePlan.hs | 7 +++--- src/PostgREST/Plan/Types.hs | 24 ++++++++++++++++++++ src/PostgREST/Query/QueryBuilder.hs | 23 +++++++++---------- src/PostgREST/Query/SqlFragment.hs | 20 +++++++++++++++-- src/PostgREST/Response/OpenAPI.hs | 11 ++++----- src/PostgREST/SchemaCache.hs | 32 +++++++++++++++++---------- src/PostgREST/SchemaCache/Table.hs | 21 +++++++++++++----- test/spec/Feature/Query/InsertSpec.hs | 24 +++++++++++++++++++- test/spec/Feature/Query/UpdateSpec.hs | 22 ++++++++++++++++++ 14 files changed, 174 insertions(+), 52 deletions(-) create mode 100644 src/PostgREST/Plan/Types.hs diff --git a/CHANGELOG.md b/CHANGELOG.md index 0789fd8e7..689c8a65f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #2548, Fix regression when embedding views with partial references to multi column FKs - @wolfgangwalther - #2558, Fix regression when requesting limit=0 and `db-max-row` is set - @laurenceisla + - #2542, Return a clear error without hitting the database when trying to update or insert an unknown column with `?columns` - @aljungberg ## [10.1.0] - 2022-10-28 diff --git a/postgrest.cabal b/postgrest.cabal index c69e80096..f6495f68a 100644 --- a/postgrest.cabal +++ b/postgrest.cabal @@ -60,6 +60,7 @@ library PostgREST.Plan.CallPlan PostgREST.Plan.MutatePlan PostgREST.Plan.ReadPlan + PostgREST.Plan.Types PostgREST.RangeQuery PostgREST.ApiRequest PostgREST.ApiRequest.Preferences diff --git a/src/PostgREST/ApiRequest/Types.hs b/src/PostgREST/ApiRequest/Types.hs index 3d0ec4896..0dc49ce78 100644 --- a/src/PostgREST/ApiRequest/Types.hs +++ b/src/PostgREST/ApiRequest/Types.hs @@ -84,6 +84,7 @@ data ApiRequestError | UnacceptableFilter Text | UnacceptableSchema [Text] | UnsupportedMethod ByteString + | ColumnNotFound Text Text data QPError = QPError Text Text data RangeError @@ -147,7 +148,7 @@ type JsonPath = [JsonOperation] data JsonOperation = JArrow { jOp :: JsonOperand } | J2Arrow { jOp :: JsonOperand } - deriving (Eq) + deriving (Eq, Ord) -- | Represents the key(`->'key'`) or index(`->'1`::int`), the index is Text -- because we reuse our escaping functons and let pg do the casting with @@ -155,7 +156,7 @@ data JsonOperation data JsonOperand = JKey { jVal :: Text } | JIdx { jVal :: Text } - deriving (Eq) + deriving (Eq, Ord) -- | Boolean logic expression tree e.g. "and(name.eq.N,or(id.eq.1,id.eq.2))" is: -- diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index e6352b8ca..b484f54c3 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -81,6 +81,7 @@ instance PgrstError ApiRequestError where status UnacceptableSchema{} = HTTP.status406 status UnsupportedMethod{} = HTTP.status405 status LimitNoOrderError = HTTP.status400 + status ColumnNotFound{} = HTTP.status400 headers _ = [MediaType.toContentType MTApplicationJSON] @@ -216,6 +217,11 @@ instance JSON.ToJSON ApiRequestError where "message" .= ("Could not choose the best candidate function between: " <> T.intercalate ", " [pdSchema p <> "." <> pdName p <> "(" <> T.intercalate ", " [ppName a <> " => " <> ppType a | a <- pdParams p] <> ")" | p <- procs]), "details" .= JSON.Null, "hint" .= ("Try renaming the parameters or the function itself in the database so function overloading can be resolved" :: Text)] + toJSON (ColumnNotFound relName colName) = JSON.object [ + "code" .= ApiRequestErrorCode18, + "message" .= ("Column '" <> colName <> "' of relation '" <> relName <> "' does not exist" :: Text), + "details" .= JSON.Null, + "hint" .= JSON.Null] -- | -- If no relationship is found then: diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index e0fa416ff..dee719d94 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -27,7 +27,7 @@ import qualified Data.HashMap.Strict as HM import qualified Data.Set as S import qualified PostgREST.SchemaCache.Proc as Proc -import Data.Either.Combinators (mapLeft) +import Data.Either.Combinators (mapLeft, mapRight) import Data.List (delete) import Data.Tree (Tree (..)) @@ -54,14 +54,15 @@ import PostgREST.SchemaCache.Relationship (Cardinality (..), Relationship (..), RelationshipsMap, relIsToOne) -import PostgREST.SchemaCache.Table (tablePKCols) - -import PostgREST.Plan.CallPlan -import PostgREST.Plan.MutatePlan -import PostgREST.Plan.ReadPlan as ReadPlan +import PostgREST.SchemaCache.Table (Table (tableName), + tablePKCols) import PostgREST.ApiRequest.Preferences import PostgREST.ApiRequest.Types +import PostgREST.Plan.CallPlan +import PostgREST.Plan.MutatePlan +import PostgREST.Plan.ReadPlan as ReadPlan +import PostgREST.Plan.Types import qualified PostgREST.ApiRequest.QueryParams as QueryParams @@ -400,8 +401,9 @@ mutatePlan :: Mutation -> QualifiedIdentifier -> ApiRequest -> SchemaCache -> Re mutatePlan mutation qi ApiRequest{..} sCache readReq = mapLeft ApiRequestError $ case mutation of MutationCreate -> - Right $ Insert qi iColumns body ((,) <$> iPreferResolution <*> Just confCols) [] returnings pkCols - MutationUpdate -> Right $ Update qi iColumns body combinedLogic iTopLevelRange rootOrder returnings + mapRight (\typedColumns -> Insert qi typedColumns body ((,) <$> iPreferResolution <*> Just confCols) [] returnings pkCols) typedColumnsOrError + MutationUpdate -> + mapRight (\typedColumns -> Update qi typedColumns body combinedLogic iTopLevelRange rootOrder returnings) typedColumnsOrError MutationSingleUpsert -> if null qsLogic && qsFilterFields == S.fromList pkCols && @@ -409,7 +411,7 @@ mutatePlan mutation qi ApiRequest{..} sCache readReq = mapLeft ApiRequestError $ all (\case Filter _ (OpExpr False (Op OpEqual _)) -> True _ -> False) qsFiltersRoot - then Right $ Insert qi iColumns body (Just (MergeDuplicates, pkCols)) combinedLogic returnings mempty + then mapRight (\typedColumns -> Insert qi typedColumns body (Just (MergeDuplicates, pkCols)) combinedLogic returnings mempty) typedColumnsOrError else Left InvalidFilters MutationDelete -> Right $ Delete qi combinedLogic iTopLevelRange rootOrder returnings @@ -425,6 +427,15 @@ mutatePlan mutation qi ApiRequest{..} sCache readReq = mapLeft ApiRequestError $ rootOrder = maybe [] snd $ find (\(x, _) -> null x) qsOrder combinedLogic = foldr addFilterToLogicForest logic qsFiltersRoot body = payRaw <$> iPayload -- the body is assumed to be json at this stage(ApiRequest validates) + tbl = HM.lookup qi $ dbTables sCache + typedColumnsOrError = resolveOrError tbl `traverse` S.toList iColumns + +resolveOrError :: Maybe Table -> FieldName -> Either ApiRequestError TypedField +resolveOrError Nothing _ = Left NotFound +resolveOrError (Just table) field = + case resolveTableField table field of + Nothing -> Left $ ColumnNotFound (tableName table) field + Just typedField -> Right typedField callPlan :: ProcDescription -> ApiRequest -> ReadPlanTree -> CallPlan callPlan proc apiReq readReq = FunctionCall { diff --git a/src/PostgREST/Plan/MutatePlan.hs b/src/PostgREST/Plan/MutatePlan.hs index 75733eca5..0c9eaa0eb 100644 --- a/src/PostgREST/Plan/MutatePlan.hs +++ b/src/PostgREST/Plan/MutatePlan.hs @@ -4,20 +4,21 @@ module PostgREST.Plan.MutatePlan where import qualified Data.ByteString.Lazy as LBS -import qualified Data.Set as S import PostgREST.ApiRequest.Preferences (PreferResolution) import PostgREST.ApiRequest.Types (LogicTree, OrderTerm) +import PostgREST.Plan.Types (TypedField) import PostgREST.RangeQuery (NonnegRange) import PostgREST.SchemaCache.Identifiers (FieldName, QualifiedIdentifier) + import Protolude data MutatePlan = Insert { in_ :: QualifiedIdentifier - , insCols :: S.Set FieldName + , insCols :: [TypedField] , insBody :: Maybe LBS.ByteString , onConflict :: Maybe (PreferResolution, [FieldName]) , where_ :: [LogicTree] @@ -26,7 +27,7 @@ data MutatePlan } | Update { in_ :: QualifiedIdentifier - , updCols :: S.Set FieldName + , updCols :: [TypedField] , updBody :: Maybe LBS.ByteString , where_ :: [LogicTree] , mutRange :: NonnegRange diff --git a/src/PostgREST/Plan/Types.hs b/src/PostgREST/Plan/Types.hs new file mode 100644 index 000000000..8e4a41f15 --- /dev/null +++ b/src/PostgREST/Plan/Types.hs @@ -0,0 +1,24 @@ +module PostgREST.Plan.Types + ( TypedField(..) + , resolveTableField + + ) where + +import qualified Data.HashMap.Strict.InsOrd as HMI + +import PostgREST.SchemaCache.Identifiers (FieldName) +import PostgREST.SchemaCache.Table (Column (..), Table (..)) + +import Protolude + +-- | A TypedField is a field with sufficient information to be read from JSON with `json_to_recordset`. +data TypedField = TypedField + { tfName :: FieldName + , tfIRType :: Text -- ^ The initial type of the field, before any casting. + } deriving (Eq) + +resolveTableField :: Table -> FieldName -> Maybe TypedField +resolveTableField table fieldName = + case HMI.lookup fieldName (tableColumns table) of + Just column -> Just $ TypedField (colName column) (colNominalType column) + Nothing -> Nothing diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index 71c591de6..c23f2061d 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -17,7 +17,6 @@ module PostgREST.Query.QueryBuilder ) where import qualified Data.ByteString.Char8 as BS -import qualified Data.Set as S import qualified Hasql.DynamicStatements.Snippet as SQL import Data.Tree (Tree (..)) @@ -34,6 +33,7 @@ import PostgREST.ApiRequest.Types import PostgREST.Plan.CallPlan import PostgREST.Plan.MutatePlan import PostgREST.Plan.ReadPlan +import PostgREST.Plan.Types import PostgREST.Query.SqlFragment import PostgREST.RangeQuery (allRange) @@ -83,9 +83,8 @@ getSelectsJoins rr@(Node ReadPlan{select, relName, relToParent=Just rel, relAggA mutatePlanToQuery :: MutatePlan -> SQL.Snippet mutatePlanToQuery (Insert mainQi iCols body onConflct putConditions returnings _) = "WITH " <> normalizedBody body <> " " <> - "INSERT INTO " <> SQL.sql (fromQi mainQi) <> SQL.sql (if S.null iCols then " " else "(" <> cols <> ") ") <> - "SELECT " <> SQL.sql cols <> " " <> - SQL.sql ("FROM json_populate_recordset (null::" <> fromQi mainQi <> ", " <> selectBody <> ") _ ") <> + "INSERT INTO " <> SQL.sql (fromQi mainQi) <> SQL.sql (if null iCols then " " else "(" <> cols <> ") ") <> + pgFmtSelectFromJson iCols <> -- Only used for PUT (if null putConditions then mempty else "WHERE " <> intercalateSnippet " AND " (pgFmtLogicTree (QualifiedIdentifier mempty "_") <$> putConditions)) <> SQL.sql (BS.unwords [ @@ -97,18 +96,18 @@ mutatePlanToQuery (Insert mainQi iCols body onConflct putConditions returnings _ IgnoreDuplicates -> "DO NOTHING" MergeDuplicates -> - if S.null iCols + if null iCols then "DO NOTHING" - else "DO UPDATE SET " <> BS.intercalate ", " (pgFmtIdent <> const " = EXCLUDED." <> pgFmtIdent <$> S.toList iCols) + else "DO UPDATE SET " <> BS.intercalate ", " ((pgFmtIdent . tfName) <> const " = EXCLUDED." <> (pgFmtIdent . tfName) <$> iCols) ) onConflct, returningF mainQi returnings ]) where - cols = BS.intercalate ", " $ pgFmtIdent <$> S.toList iCols + cols = BS.intercalate ", " $ pgFmtIdent . tfName <$> iCols -- An update without a limit is always filtered with a WHERE mutatePlanToQuery (Update mainQi uCols body logicForest range ordts returnings) - | S.null uCols = + | null uCols = -- if there are no columns we cannot do UPDATE table SET {empty}, it'd be invalid syntax -- selecting an empty resultset from mainQi gives us the column names to prevent errors when using &select= -- the select has to be based on "returnings" to make computed overloaded functions not throw @@ -117,13 +116,13 @@ mutatePlanToQuery (Update mainQi uCols body logicForest range ordts returnings) | range == allRange = "WITH " <> normalizedBody body <> " " <> "UPDATE " <> mainTbl <> " SET " <> SQL.sql nonRangeCols <> " " <> - "FROM (SELECT * FROM json_populate_recordset (null::" <> mainTbl <> " , " <> SQL.sql selectBody <> " )) _ " <> + "FROM (" <> pgFmtSelectFromJson uCols <> ") AS _ " <> whereLogic <> " " <> SQL.sql (returningF mainQi returnings) | otherwise = "WITH " <> normalizedBody body <> ", " <> - "pgrst_update_body AS (SELECT * FROM json_populate_recordset (null::" <> mainTbl <> " , " <> SQL.sql selectBody <> " ) LIMIT 1), " <> + "pgrst_update_body AS (" <> pgFmtSelectFromJson uCols <> " LIMIT 1), " <> "pgrst_affected_rows AS (" <> "SELECT " <> SQL.sql rangeIdF <> " FROM " <> mainTbl <> whereLogic <> " " <> @@ -139,8 +138,8 @@ mutatePlanToQuery (Update mainQi uCols body logicForest range ordts returnings) whereLogic = if null logicForest then mempty else " WHERE " <> intercalateSnippet " AND " (pgFmtLogicTree mainQi <$> logicForest) mainTbl = SQL.sql (fromQi mainQi) emptyBodyReturnedColumns = if null returnings then "NULL" else BS.intercalate ", " (pgFmtColumn (QualifiedIdentifier mempty $ qiName mainQi) <$> returnings) - nonRangeCols = BS.intercalate ", " (pgFmtIdent <> const " = _." <> pgFmtIdent <$> S.toList uCols) - rangeCols = BS.intercalate ", " ((\col -> pgFmtIdent col <> " = (SELECT " <> pgFmtIdent col <> " FROM pgrst_update_body) ") <$> S.toList uCols) + nonRangeCols = BS.intercalate ", " (pgFmtIdent . tfName <> const " = _." <> pgFmtIdent . tfName <$> uCols) + rangeCols = BS.intercalate ", " ((\col -> pgFmtIdent (tfName col) <> " = (SELECT " <> pgFmtIdent (tfName col) <> " FROM pgrst_update_body) ") <$> uCols) (whereRangeIdF, rangeIdF) = mutRangeF mainQi (fst . otTerm <$> ordts) mutatePlanToQuery (Delete mainQi logicForest range ordts returnings) diff --git a/src/PostgREST/Query/SqlFragment.hs b/src/PostgREST/Query/SqlFragment.hs index 3dbee2276..237a163f5 100644 --- a/src/PostgREST/Query/SqlFragment.hs +++ b/src/PostgREST/Query/SqlFragment.hs @@ -30,6 +30,7 @@ module PostgREST.Query.SqlFragment , pgFmtLogicTree , pgFmtOrderTerm , pgFmtSelectItem + , pgFmtSelectFromJson , responseHeadersF , responseStatusF , returningF @@ -74,6 +75,7 @@ import PostgREST.ApiRequest.Types (Alias, Cast, Field, import PostgREST.MediaType (MTPlanFormat (..), MTPlanOption (..)) import PostgREST.Plan.ReadPlan (JoinCondition (..)) +import PostgREST.Plan.Types (TypedField (..)) import PostgREST.RangeQuery (NonnegRange, allRange, rangeLimit, rangeOffset) import PostgREST.SchemaCache.Identifiers (FieldName, @@ -120,8 +122,8 @@ ftsOperator = \case FilterFtsWebsearch -> "@@ websearch_to_tsquery" -- | --- These CTEs convert a json object into a json array, this way we can use json_populate_recordset for all json payloads --- Otherwise we'd have to use json_populate_record for json objects and json_populate_recordset for json arrays +-- These CTEs convert a json object into a json array, this way we can use json_to_recordset for all json payloads +-- Otherwise we'd have to use json_to_record for json objects and json_to_recordset for json arrays -- We do this in SQL to avoid processing the JSON in application code -- TODO: At this stage there shouldn't be a Maybe since ApiRequest should ensure that an INSERT/UPDATE has a body normalizedBody :: Maybe LBS.ByteString -> SQL.Snippet @@ -242,6 +244,20 @@ pgFmtSelectItem table (f@(fName, jp), Nothing, alias) = pgFmtField table f <> SQ -- Not quoting should be fine, we validate the input on Parsers. pgFmtSelectItem table (f@(fName, jp), Just cast, alias) = "CAST (" <> pgFmtField table f <> " AS " <> SQL.sql (encodeUtf8 cast) <> " )" <> SQL.sql (pgFmtAs fName jp alias) +pgFmtSelectFromJson :: [TypedField] -> SQL.Snippet +pgFmtSelectFromJson fields = + SQL.sql "SELECT " <> parsedCols <> " " <> + (if null fields + -- When we are inserting no columns (e.g. using default values), we can't use our ordinary `json_to_recordset` + -- because it can't extract records with no columns (there's no valid syntax for the `AS (colName colType,...)` + -- part). But we still need to ensure as many rows are created as there are array elements. + then SQL.sql ("FROM json_array_elements (" <> selectBody <> ") _ ") + else SQL.sql ("FROM json_to_recordset (" <> selectBody <> ") AS _ " <> "(" <> typedCols <> ") ") + ) + where + parsedCols = SQL.sql $ BS.intercalate ", " $ pgFmtIdent . tfName <$> fields + typedCols = BS.intercalate ", " $ pgFmtIdent . tfName <> const " " <> encodeUtf8 . tfIRType <$> fields + pgFmtOrderTerm :: QualifiedIdentifier -> OrderTerm -> SQL.Snippet pgFmtOrderTerm qi ot = fmtOTerm ot <> " " <> diff --git a/src/PostgREST/Response/OpenAPI.hs b/src/PostgREST/Response/OpenAPI.hs index 240551e54..8444e796e 100644 --- a/src/PostgREST/Response/OpenAPI.hs +++ b/src/PostgREST/Response/OpenAPI.hs @@ -34,7 +34,8 @@ import PostgREST.SchemaCache.Relationship (Cardinality (..), Relationship (..), RelationshipsMap) import PostgREST.SchemaCache.Table (Column (..), Table (..), - TablesMap) + TablesMap, + tableColumnsList) import PostgREST.Version (docsVersion, prettyVersion) import PostgREST.MediaType @@ -93,8 +94,8 @@ makeTableDef rels t = (tn, (mempty :: Schema) & description .~ tableDescription t & type_ ?~ SwaggerObject - & properties .~ fromList (makeProperty t rels <$> tableColumns t) - & required .~ fmap colName (filter (not . colNullable) $ tableColumns t)) + & properties .~ fromList (makeProperty t rels <$> tableColumnsList t) + & required .~ fmap colName (filter (not . colNullable) $ tableColumnsList t)) makeProperty :: Table -> RelationshipsMap -> Column -> (Text, Referenced Schema) makeProperty tbl rels col = (colName col, Inline s) @@ -238,7 +239,7 @@ makeParamDefs ti = & in_ .~ ParamQuery & type_ ?~ SwaggerString)) ] - <> concat [ makeObjectBody (tableName t) : makeRowFilters (tableName t) (tableColumns t) + <> concat [ makeObjectBody (tableName t) : makeRowFilters (tableName t) (tableColumnsList t) | t <- ti ] @@ -299,7 +300,7 @@ makePathItem t = ("/" ++ T.unpack tn, p $ tableInsertable t || tableUpdatable t p False = pr p True = pw tn = tableName t - rs = [ T.intercalate "." ["rowFilter", tn, colName c ] | c <- tableColumns t ] + rs = [ T.intercalate "." ["rowFilter", tn, colName c ] | c <- tableColumnsList t ] ref = Ref . Reference makeProcPathItem :: ProcDescription -> (FilePath, PathItem) diff --git a/src/PostgREST/SchemaCache.hs b/src/PostgREST/SchemaCache.hs index 31637b6df..c456173c0 100644 --- a/src/PostgREST/SchemaCache.hs +++ b/src/PostgREST/SchemaCache.hs @@ -26,13 +26,14 @@ module PostgREST.SchemaCache , schemaDescription ) where -import qualified Data.Aeson as JSON -import qualified Data.HashMap.Strict as HM -import qualified Data.Set as S -import qualified Hasql.Decoders as HD -import qualified Hasql.Encoders as HE -import qualified Hasql.Statement as SQL -import qualified Hasql.Transaction as SQL +import qualified Data.Aeson as JSON +import qualified Data.HashMap.Strict as HM +import qualified Data.HashMap.Strict.InsOrd as HMI +import qualified Data.Set as S +import qualified Hasql.Decoders as HD +import qualified Hasql.Encoders as HE +import qualified Hasql.Statement as SQL +import qualified Hasql.Transaction as SQL import Contravariant.Extras (contrazip2) import Text.InterpolatedString.Perl6 (q) @@ -52,8 +53,8 @@ import PostgREST.SchemaCache.Relationship (Cardinality (..), Junction (..), Relationship (..), RelationshipsMap) -import PostgREST.SchemaCache.Table (Column (..), Table (..), - TablesMap) +import PostgREST.SchemaCache.Table (Column (..), ColumnMap, + Table (..), TablesMap) import Protolude @@ -178,15 +179,20 @@ decodeTables = <*> column HD.bool <*> column HD.bool <*> arrayColumn HD.text - <*> compositeArrayColumn - (Column + <*> parseCols (compositeArrayColumn + (Column <$> compositeField HD.text <*> nullableCompositeField HD.text <*> compositeField HD.bool <*> compositeField HD.text + <*> compositeField HD.text <*> nullableCompositeField HD.int4 <*> nullableCompositeField HD.text - <*> compositeFieldArray HD.text) + <*> compositeFieldArray HD.text)) + + +parseCols :: HD.Row [Column] -> HD.Row ColumnMap +parseCols = fmap (HMI.fromList . map (\col@Column{colName} -> (colName, col))) decodeRels :: HD.Result [Relationship] decodeRels = @@ -521,6 +527,7 @@ tablesSqlQuery pgVer = ELSE format_type(a.atttypid, a.atttypmod) END END::text AS data_type, + t.oid AS data_type_id, information_schema._pg_char_max_length( information_schema._pg_truetypid(a.*, t.*), information_schema._pg_truetypmod(a.*, t.*) @@ -556,6 +563,7 @@ tablesSqlQuery pgVer = info.description, info.is_nullable::boolean, info.data_type, + info.data_type_id::regtype::text, info.character_maximum_length, info.column_default, coalesce(enum_info.vals, '{}')) order by info.position) as columns diff --git a/src/PostgREST/SchemaCache/Table.hs b/src/PostgREST/SchemaCache/Table.hs index 3a7f886a8..72a5498fa 100644 --- a/src/PostgREST/SchemaCache/Table.hs +++ b/src/PostgREST/SchemaCache/Table.hs @@ -1,14 +1,18 @@ -{-# LANGUAGE DeriveAnyClass #-} -{-# LANGUAGE DeriveGeneric #-} +{-# LANGUAGE DeriveAnyClass #-} +{-# LANGUAGE DeriveGeneric #-} +{-# LANGUAGE FlexibleInstances #-} module PostgREST.SchemaCache.Table ( Column(..) , Table(..) + , tableColumnsList , TablesMap + , ColumnMap ) where -import qualified Data.Aeson as JSON -import qualified Data.HashMap.Strict as HM +import qualified Data.Aeson as JSON +import qualified Data.HashMap.Strict as HM +import qualified Data.HashMap.Strict.InsOrd as HMI import PostgREST.SchemaCache.Identifiers (FieldName, QualifiedIdentifier (..), @@ -28,9 +32,12 @@ data Table = Table , tableUpdatable :: Bool , tableDeletable :: Bool , tablePKCols :: [FieldName] - , tableColumns :: [Column] + , tableColumns :: ColumnMap } - deriving (Show, Ord, Generic, JSON.ToJSON) + deriving (Show, Generic, JSON.ToJSON) + +tableColumnsList :: Table -> [Column] +tableColumnsList = HMI.elems . tableColumns instance Eq Table where Table{tableSchema=s1,tableName=n1} == Table{tableSchema=s2,tableName=n2} = s1 == s2 && n1 == n2 @@ -40,6 +47,7 @@ data Column = Column , colDescription :: Maybe Text , colNullable :: Bool , colType :: Text + , colNominalType :: Text , colMaxLen :: Maybe Int32 , colDefault :: Maybe Text , colEnum :: [Text] @@ -47,3 +55,4 @@ data Column = Column deriving (Eq, Show, Ord, Generic, JSON.ToJSON) type TablesMap = HM.HashMap QualifiedIdentifier Table +type ColumnMap = HMI.InsOrdHashMap FieldName Column diff --git a/test/spec/Feature/Query/InsertSpec.hs b/test/spec/Feature/Query/InsertSpec.hs index d14fd92d1..50053169d 100644 --- a/test/spec/Feature/Query/InsertSpec.hs +++ b/test/spec/Feature/Query/InsertSpec.hs @@ -420,6 +420,28 @@ spec actualPgVersion = do , matchHeaders = [] } + it "disallows ?columns which don't exist" $ + post "/articles?columns=helicopter" + [json|[ + {"id": 204, "body": "yyy"}, + {"id": 205, "body": "zzz"}]|] + `shouldRespondWith` + [json|{"code":"PGRST118","details":null,"hint":null,"message":"Column 'helicopter' of relation 'articles' does not exist"} |] + { matchStatus = 400 + , matchHeaders = [] + } + + it "returns missing table error even if also has invalid ?columns" $ + post "/garlic?columns=helicopter" + [json|[ + {"id": 204, "body": "yyy"}, + {"id": 205, "body": "zzz"}]|] + `shouldRespondWith` + [json|{} |] + { matchStatus = 404 + , matchHeaders = [] + } + it "disallows array elements that are not json objects" $ post "/articles?columns=id,body" [json|[ @@ -431,7 +453,7 @@ spec actualPgVersion = do "code": "22023", "details": null, "hint": null, - "message": "argument of json_populate_recordset must be an array of objects"}|] + "message": "argument of json_to_recordset must be an array of objects"}|] { matchStatus = 400 , matchHeaders = [] } diff --git a/test/spec/Feature/Query/UpdateSpec.hs b/test/spec/Feature/Query/UpdateSpec.hs index 23d0b6388..ff0be3dcf 100644 --- a/test/spec/Feature/Query/UpdateSpec.hs +++ b/test/spec/Feature/Query/UpdateSpec.hs @@ -308,6 +308,28 @@ spec = do request methodPatch "/articles?id=eq.2001&columns=body" [("Prefer", "return=representation")] [json| {"body": "Some real content", "smth": "here", "other": "stuff", "fake_id": 13} |] `shouldRespondWith` 200 + it "disallows ?columns which don't exist" $ do + request methodPatch "/articles?id=eq.1&columns=helicopter" + [("Prefer", "return=representation")] + [json|{"body": "yyy"}|] + `shouldRespondWith` + [json|{"code":"PGRST118","details":null,"hint":null,"message":"Column 'helicopter' of relation 'articles' does not exist"} |] + { matchStatus = 400 + , matchHeaders = [] + } + + it "returns missing table error even if also has invalid ?columns" $ do + request methodPatch "/garlic?columns=helicopter" + [("Prefer", "return=representation")] + [json|[ + {"id": 204, "body": "yyy"}, + {"id": 205, "body": "zzz"}]|] + `shouldRespondWith` + [json|{} |] + { matchStatus = 404 + , matchHeaders = [] + } + context "tables with self reference foreign keys" $ do it "embeds children after update" $ request methodPatch "/web_content?id=eq.0&select=id,name,web_content(name)"