diff --git a/CHANGELOG.md b/CHANGELOG.md index c263acdc9..b5a28985f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #2052, Dropped support for PostgreSQL 11 - @wolfgangwalther - #3508, PostgREST now fails to start when `server-port` and `admin-server-port` config options are the same - @develop7 - #3607, PostgREST now fails to start when the JWT secret is less than 32 characters long - @laurenceisla + - #3644, Fail schema cache lookup with invalid db-schemas config - @wolfgangwalther + - Previously, this would silently return 200 - OK on the root endpoint, but don't provide any usable endpoints. ## [12.2.1] - 2024-06-27 diff --git a/src/PostgREST/Query/SqlFragment.hs b/src/PostgREST/Query/SqlFragment.hs index 9329fb438..39b869d5d 100644 --- a/src/PostgREST/Query/SqlFragment.hs +++ b/src/PostgREST/Query/SqlFragment.hs @@ -157,10 +157,10 @@ pgBuildArrayLiteral vals = -- TODO: refactor by following https://github.com/PostgREST/postgrest/pull/1631#issuecomment-711070833 pgFmtIdent :: Text -> SQL.Snippet -pgFmtIdent x = SQL.sql $ escapeIdent x +pgFmtIdent x = SQL.sql . encodeUtf8 $ escapeIdent x -escapeIdent :: Text -> ByteString -escapeIdent x = encodeUtf8 $ "\"" <> T.replace "\"" "\"\"" (trimNullChars x) <> "\"" +escapeIdent :: Text -> Text +escapeIdent x = "\"" <> T.replace "\"" "\"\"" (trimNullChars x) <> "\"" -- Only use it if the input comes from the database itself, like on `jsonb_build_object('column_from_a_table', val)..` pgFmtLit :: Text -> Text @@ -181,7 +181,7 @@ trimNullChars = T.takeWhile (/= '\x0') -- >>> escapeIdentList ["schema_1", "schema_2", "SPECIAL \"@/\\#~_-"] -- "\"schema_1\", \"schema_2\", \"SPECIAL \"\"@/\\#~_-\"" escapeIdentList :: [Text] -> ByteString -escapeIdentList schemas = BS.intercalate ", " $ escapeIdent <$> schemas +escapeIdentList schemas = BS.intercalate ", " $ encodeUtf8 . escapeIdent <$> schemas asCsvF :: SQL.Snippet asCsvF = asCsvHeaderF <> " || '\n' || " <> asCsvBodyF diff --git a/src/PostgREST/SchemaCache.hs b/src/PostgREST/SchemaCache.hs index bcf5a287a..7db9808ca 100644 --- a/src/PostgREST/SchemaCache.hs +++ b/src/PostgREST/SchemaCache.hs @@ -46,6 +46,7 @@ import NeatInterpolation (trimming) import PostgREST.Config (AppConfig (..)) import PostgREST.Config.Database (TimezoneNames, toIsolationLevel) +import PostgREST.Query.SqlFragment (escapeIdent) import PostgREST.SchemaCache.Identifiers (AccessSet, FieldName, QualifiedIdentifier (..), RelIdentifier (..), @@ -363,7 +364,7 @@ allFunctions :: Bool -> SQL.Statement AppConfig RoutineMap allFunctions = SQL.Statement funcsSqlQuery params decodeFuncs where params = - (toList . configDbSchemas >$< arrayParam HE.text) <> + (map escapeIdent . toList . configDbSchemas >$< arrayParam HE.text) <> (configDbHoistedTxSettings >$< arrayParam HE.text) accessibleFuncs :: Bool -> SQL.Statement ([Schema], [Text]) RoutineMap @@ -464,7 +465,7 @@ funcsSqlQuery = encodeUtf8 [trimming| ) func_settings ON TRUE WHERE t.oid <> 'trigger'::regtype AND COALESCE(a.callable, true) AND prokind = 'f' - AND pn.nspname = ANY($$1) |] + AND p.pronamespace = ANY($$1::regnamespace[]) |] schemaDescription :: Bool -> SQL.Statement Schema (Maybe Text) schemaDescription = @@ -477,12 +478,13 @@ schemaDescription = pg_namespace n left join pg_description d on d.objoid = n.oid where - n.nspname = $$1 |] + n.oid = $$1::regnamespace |] accessibleTables :: Bool -> SQL.Statement [Schema] AccessSet accessibleTables = - SQL.Statement sql (arrayParam HE.text) decodeAccessibleIdentifiers + SQL.Statement sql params decodeAccessibleIdentifiers where + params = map escapeIdent >$< arrayParam HE.text sql = encodeUtf8 [trimming| SELECT n.nspname AS table_schema, @@ -490,7 +492,7 @@ accessibleTables = FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE c.relkind IN ('v','r','m','f','p') - AND n.nspname = ANY($$1) + AND c.relnamespace = ANY($$1::regnamespace[]) AND ( pg_has_role(c.relowner, 'USAGE') or has_table_privilege(c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER') @@ -608,7 +610,7 @@ addViewPrimaryKeys tabs keyDeps = allTables :: Bool -> SQL.Statement AppConfig TablesMap allTables = SQL.Statement tablesSqlQuery params decodeTables where - params = toList . configDbSchemas >$< arrayParam HE.text + params = map escapeIdent . toList . configDbSchemas >$< arrayParam HE.text -- | Gets tables with their PK cols tablesSqlQuery :: SqlQuery @@ -657,7 +659,7 @@ tablesSqlQuery = ON d.objoid = a.attrelid and d.objsubid = a.attnum LEFT JOIN pg_attrdef ad ON a.attrelid = ad.adrelid AND a.attnum = ad.adnum - JOIN (pg_class c JOIN pg_namespace nc ON c.relnamespace = nc.oid) + JOIN pg_class c ON a.attrelid = c.oid JOIN pg_type t ON a.atttypid = t.oid @@ -670,7 +672,7 @@ tablesSqlQuery = AND a.attnum > 0 AND NOT a.attisdropped AND c.relkind in ('r', 'v', 'f', 'm', 'p') - AND nc.nspname = ANY($$1) + AND c.relnamespace = ANY($$1::regnamespace[]) ), columns_agg AS ( SELECT @@ -856,8 +858,8 @@ allViewsKeyDependencies = -- * json transformation: https://gist.github.com/wolfgangwalther/3a8939da680c24ad767e93ad2c183089 where params = - (toList . configDbSchemas >$< arrayParam HE.text) <> - (configDbExtraSearchPath >$< arrayParam HE.text) + (map escapeIdent . toList . configDbSchemas >$< arrayParam HE.text) <> + (map escapeIdent . toList . configDbExtraSearchPath >$< arrayParam HE.text) sql = encodeUtf8 [trimming| with recursive pks_fks as ( @@ -887,18 +889,19 @@ allViewsKeyDependencies = ), views as ( select - c.oid as view_id, - n.nspname as view_schema, - c.relname as view_name, - r.ev_action as view_definition + c.oid as view_id, + c.relnamespace as view_schema_id, + n.nspname as view_schema, + c.relname as view_name, + r.ev_action as view_definition from pg_class c join pg_namespace n on n.oid = c.relnamespace join pg_rewrite r on r.ev_class = c.oid - where c.relkind in ('v', 'm') and n.nspname = ANY($$1 || $$2) + where c.relkind in ('v', 'm') and c.relnamespace = ANY($$1::regnamespace[] || $$2::regnamespace[]) ), transform_json as ( select - view_id, view_schema, view_name, + view_id, view_schema_id, view_schema, view_name, -- the following formatting is without indentation on purpose -- to allow simple diffs, with less whitespace noise replace( @@ -978,13 +981,13 @@ allViewsKeyDependencies = ), target_entries as( select - view_id, view_schema, view_name, + view_id, view_schema_id, view_schema, view_name, json_array_elements(view_definition->0->'targetList') as entry from transform_json ), results as( select - view_id, view_schema, view_name, + view_id, view_schema_id, view_schema, view_name, (entry->>'resno')::int as view_column, (entry->>'resorigtbl')::oid as resorigtbl, (entry->>'resorigcol')::int as resorigcol @@ -992,16 +995,17 @@ allViewsKeyDependencies = ), -- CYCLE detection according to PG docs: https://www.postgresql.org/docs/current/queries-with.html#QUERIES-WITH-CYCLE -- Can be replaced with CYCLE clause once PG v13 is EOL. - recursion(view_id, view_schema, view_name, view_column, resorigtbl, resorigcol, is_cycle, path) as( + recursion(view_id, view_schema_id, view_schema, view_name, view_column, resorigtbl, resorigcol, is_cycle, path) as( select r.*, false, ARRAY[resorigtbl] from results r - where view_schema = ANY ($$1) + where view_schema_id = ANY ($$1::regnamespace[]) union all select view.view_id, + view.view_schema_id, view.view_schema, view.view_name, view.view_column, @@ -1060,7 +1064,7 @@ mediaHandlers :: Bool -> SQL.Statement AppConfig MediaHandlerMap mediaHandlers = SQL.Statement sql params decodeMediaHandlers where - params = toList . configDbSchemas >$< arrayParam HE.text + params = map escapeIdent . toList . configDbSchemas >$< arrayParam HE.text sql = encodeUtf8 [trimming| with all_relations as ( @@ -1101,7 +1105,7 @@ mediaHandlers = join pg_type arg_name on arg_name.oid = proc.proargtypes[0] join pg_namespace arg_schema on arg_schema.oid = arg_name.typnamespace where - proc_schema.nspname = ANY($$1) and + proc.pronamespace = ANY($$1::regnamespace[]) and proc.pronargs = 1 and arg_name.oid in (select reltype from all_relations) union @@ -1117,7 +1121,7 @@ mediaHandlers = join media_types mtype on proc.prorettype = mtype.oid join pg_namespace typ_sch on typ_sch.oid = mtype.typnamespace where - pro_sch.nspname = ANY($$1) and NOT proretset + proc.pronamespace = ANY($$1::regnamespace[]) and NOT proretset and prokind = 'f'|] decodeMediaHandlers :: HD.Result MediaHandlerMap diff --git a/test/spec/Feature/Query/ErrorSpec.hs b/test/spec/Feature/Query/ErrorSpec.hs index d004e5c5c..a41f371fe 100644 --- a/test/spec/Feature/Query/ErrorSpec.hs +++ b/test/spec/Feature/Query/ErrorSpec.hs @@ -9,59 +9,6 @@ import Test.Hspec.Wai.JSON import Protolude hiding (get) -nonExistentSchema :: SpecWith ((), Application) -nonExistentSchema = do - describe "Non existent api schema" $ do - it "succeeds when requesting root path" $ - get "/" `shouldRespondWith` 200 - - it "gives 404 when requesting a nonexistent table in this nonexistent schema" $ - get "/nonexistent_table" `shouldRespondWith` 404 - - describe "Non existent URL" $ do - it "gives 404 on a single nested route" $ - get "/projects/nested" `shouldRespondWith` 404 - - it "gives 404 on a double nested route" $ - get "/projects/nested/double" `shouldRespondWith` 404 - - describe "Unsupported HTTP methods" $ do - it "should return 405 for CONNECT method" $ - request methodConnect "/" - [] - "" - `shouldRespondWith` - [json| - {"hint": null, - "details": null, - "code": "PGRST117", - "message":"Unsupported HTTP method: CONNECT"}|] - { matchStatus = 405 } - - it "should return 405 for TRACE method" $ - request methodTrace "/" - [] - "" - `shouldRespondWith` - [json| - {"hint": null, - "details": null, - "code": "PGRST117", - "message":"Unsupported HTTP method: TRACE"}|] - { matchStatus = 405 } - - it "should return 405 for OTHER method" $ - request "OTHER" "/" - [] - "" - `shouldRespondWith` - [json| - {"hint": null, - "details": null, - "code": "PGRST117", - "message":"Unsupported HTTP method: OTHER"}|] - { matchStatus = 405 } - pgErrorCodeMapping :: SpecWith ((), Application) pgErrorCodeMapping = do describe "PostreSQL error code mappings" $ do diff --git a/test/spec/Main.hs b/test/spec/Main.hs index 070c6905d..526bd45dc 100644 --- a/test/spec/Main.hs +++ b/test/spec/Main.hs @@ -126,7 +126,6 @@ main = do extraSearchPathApp = appDbs testCfgExtraSearchPath unicodeApp = appDbs testUnicodeCfg - nonexistentSchemaApp = appDbs testNonexistentSchemaCfg multipleSchemaApp = appDbs testMultipleSchemaCfg ignorePrivOpenApi = appDbs testIgnorePrivOpenApiCfg @@ -221,10 +220,6 @@ main = do parallel $ before asymJwkSetApp $ describe "Feature.Auth.AsymmetricJwtSpec" Feature.Auth.AsymmetricJwtSpec.spec - -- this test runs with a nonexistent db-schema - parallel $ before nonexistentSchemaApp $ - describe "Feature.Query.NonExistentSchemaErrorSpec" Feature.Query.ErrorSpec.nonExistentSchema - -- this test runs with an extra search path parallel $ before extraSearchPathApp $ do describe "Feature.ExtraSearchPathSpec" Feature.ExtraSearchPathSpec.spec diff --git a/test/spec/SpecHelper.hs b/test/spec/SpecHelper.hs index 7e64d9e4a..992cb735d 100644 --- a/test/spec/SpecHelper.hs +++ b/test/spec/SpecHelper.hs @@ -227,9 +227,6 @@ testCfgAsymJWKSet = , configJWKS = rightToMaybe $ parseSecret secret } -testNonexistentSchemaCfg :: AppConfig -testNonexistentSchemaCfg = baseCfg { configDbSchemas = fromList ["nonexistent"] } - testCfgExtraSearchPath :: AppConfig testCfgExtraSearchPath = baseCfg { configDbExtraSearchPath = ["public", "extensions", "EXTRA \"@/\\#~_-"] }