diff --git a/CHANGELOG.md b/CHANGELOG.md index a74472ac9a..781d2751b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file. From versio - Log host, port and pg version of listener database connection by @mkleczek in #4617 #4618 - Optimize requests with `Prefer: count=exact` that do not use ranges or `db-max-rows` by @laurenceisla in #3957 + Removed unnecessary double count when building the `Content-Range`. +- Add `Prefer: timeout` header for per-request `statement_timeout` by @taimoorzaeem in #4381 ### Changed diff --git a/docs/references/api/preferences.rst b/docs/references/api/preferences.rst index 96dd4f48fb..02693b7493 100644 --- a/docs/references/api/preferences.rst +++ b/docs/references/api/preferences.rst @@ -15,6 +15,7 @@ The following preferences are supported. - ``Prefer: missing``. See :ref:`prefer_missing`. - ``Prefer: max-affected``, See :ref:`prefer_max_affected`. - ``Prefer: tx``. See :ref:`prefer_tx`. +- ``Prefer: timeout``. See :ref:`prefer_timeout`. .. _prefer_handling: @@ -296,3 +297,62 @@ With :ref:`RPC `, the preference is honored completely on the basis o .. note:: It is important for functions to return ``SETOF`` or ``TABLE`` when called with ``max-affected`` preference. A violation of this would cause a :ref:`PGRST128 ` error. + +.. _prefer_timeout: + +Timeout +======= + +You can set `statement_timeout `_ for the request using this preference. This works in combination with ``handling=strict`` preference in the same header. + +The header only accepts integer value indicating the ``seconds`` that are set as timeout value. ``0`` and negative values are ignored and not applied. To demonstrate, see the following example: + +.. code-block:: postgres + + CREATE FUNCTION test.sleep(seconds) + RETURNS VOID AS $$ + SELECT pg_sleep(seconds); + $$ LANGUAGE SQL; + +.. code-block:: bash + + curl -i "http://localhost:3000/rpc/sleep?seconds=5" \ + -H "Prefer: handling=strict, timeout=2" + +.. code-block:: http + + HTTP/1.1 500 Internal Server Error + +.. code-block:: json + + { + "code": "57014", + "details": null, + "hint": null, + "message": "canceling statement due to statement timeout" + } + +It is important to note the timeout value cannot exceed the ``statement_timeout`` set :ref:`per-role ` or per-database. The role's timeout setting takes precedence over the database level timeout. This restriction prevents misuse of this feature. PostgREST returns a :ref:`PGRST129 ` error in this case. + +.. code-block:: postgres + + ALTER ROLE postgrest_test_anonymous SET statement_timeout = '3s'; + +.. code-block:: bash + + curl -i "http://localhost:3000/rpc/sleep?seconds=4" \ + -H "Prefer: handling=strict, timeout=5" + +.. code-block:: http + + HTTP/1.1 400 Bad Request + Content-Type: application/json; charset=utf-8 + +.. code-block:: json + + { + "code": "PGRST129", + "message": "Timeout preference exceeded maximum allowed", + "details": "The maximum timeout allowed is 3s", + "hint": "Reduce the timeout" + } diff --git a/docs/references/errors.rst b/docs/references/errors.rst index 1802828579..e5c205d57c 100644 --- a/docs/references/errors.rst +++ b/docs/references/errors.rst @@ -271,6 +271,10 @@ Related to the HTTP request elements. | | | See :ref:`prefer_max_affected`. | | PGRST128 | | | +---------------+-------------+-------------------------------------------------------------+ +| .. _pgrst129: | 400 | ``timeout`` preference exceeds ``statement_timeout`` value | +| | | of role. See :ref:`prefer_timeout`. | +| PGRST129 | | | ++---------------+-------------+-------------------------------------------------------------+ .. _pgrst2**: diff --git a/src/PostgREST/ApiRequest/Preferences.hs b/src/PostgREST/ApiRequest/Preferences.hs index 55369efbb1..db82d1b14f 100644 --- a/src/PostgREST/ApiRequest/Preferences.hs +++ b/src/PostgREST/ApiRequest/Preferences.hs @@ -6,7 +6,7 @@ -- -- [1] https://datatracker.ietf.org/doc/html/rfc7240 -- -{-# LANGUAGE NamedFieldPuns #-} +{-# LANGUAGE RecordWildCards #-} module PostgREST.ApiRequest.Preferences ( Preferences(..) , PreferCount(..) @@ -17,6 +17,7 @@ module PostgREST.ApiRequest.Preferences , PreferTransaction(..) , PreferTimezone(..) , PreferMaxAffected(..) + , PreferTimeout(..) , fromHeaders , shouldCount , shouldExplainCount @@ -43,6 +44,7 @@ import Protolude -- >>> deriving instance Show PreferHandling -- >>> deriving instance Show PreferTimezone -- >>> deriving instance Show PreferMaxAffected +-- >>> deriving instance Show PreferTimeout -- >>> deriving instance Show Preferences -- | Preferences recognized by the application. @@ -56,6 +58,7 @@ data Preferences , preferHandling :: Maybe PreferHandling , preferTimezone :: Maybe PreferTimezone , preferMaxAffected :: Maybe PreferMaxAffected + , preferTimeout :: Maybe PreferTimeout , invalidPrefs :: [ByteString] } @@ -77,12 +80,13 @@ data Preferences -- ( PreferTimezone "America/Los_Angeles" ) -- , preferMaxAffected = Just -- ( PreferMaxAffected 100 ) +-- , preferTimeout = Nothing -- , invalidPrefs = [] -- } -- -- Multiple headers can also be used: -- --- >>> pPrint $ fromHeaders True sc [("Prefer", "resolution=ignore-duplicates"), ("Prefer", "count=exact"), ("Prefer", "missing=null"), ("Prefer", "handling=lenient"), ("Prefer", "invalid"), ("Prefer", "max-affected=5999")] +-- >>> pPrint $ fromHeaders True sc [("Prefer", "resolution=ignore-duplicates"), ("Prefer", "count=exact"), ("Prefer", "missing=null"), ("Prefer", "handling=lenient"), ("Prefer", "invalid"), ("Prefer", "max-affected=5999"), ("Prefer", "timeout=10")] -- Preferences -- { preferResolution = Just IgnoreDuplicates -- , preferRepresentation = Nothing @@ -93,6 +97,8 @@ data Preferences -- , preferTimezone = Nothing -- , preferMaxAffected = Just -- ( PreferMaxAffected 5999 ) +-- , preferTimeout = Just +-- ( PreferTimeout 10 ) -- , invalidPrefs = [ "invalid" ] -- } -- @@ -124,6 +130,7 @@ data Preferences -- , preferHandling = Just Strict -- , preferTimezone = Nothing -- , preferMaxAffected = Nothing +-- , preferTimeout = Nothing -- , invalidPrefs = [ "anything" ] -- } -- @@ -138,7 +145,8 @@ fromHeaders allowTxDbOverride acceptedTzNames headers = , preferHandling = parsePrefs [Strict, Lenient] , preferTimezone = if isTimezonePrefAccepted then PreferTimezone <$> timezonePref else Nothing , preferMaxAffected = PreferMaxAffected <$> maxAffectedPref - , invalidPrefs = filter isUnacceptable prefs + , preferTimeout = PreferTimeout <$> timeoutPref + , invalidPrefs = filter (not . isPrefValid) prefs } where mapToHeadVal :: ToHeaderValue a => [a] -> [ByteString] @@ -159,10 +167,17 @@ fromHeaders allowTxDbOverride acceptedTzNames headers = isTimezonePrefAccepted = ((S.member . decodeUtf8 <$> timezonePref) <*> pure acceptedTzNames) == Just True maxAffectedPref = listStripPrefix "max-affected=" prefs >>= readMaybe . BS.unpack - - isUnacceptable p = p `notElem` acceptedPrefs && - (isNothing (BS.stripPrefix "timezone=" p) || not isTimezonePrefAccepted) && - isNothing (BS.stripPrefix "max-affected=" p) + timeoutPref = listStripPrefix "timeout=" prefs >>= readOnlyPositiveInt . readMaybe . BS.unpack + where + -- 0 and -ve values for timeout are meaningless, we handle them leniently and ignore them + readOnlyPositiveInt (Just i) | i > 0 = Just i + readOnlyPositiveInt _ = Nothing + + isPrefValid p = + p `elem` acceptedPrefs || + (isJust (BS.stripPrefix "timezone=" p) && isTimezonePrefAccepted) || + isJust (BS.stripPrefix "max-affected=" p) || + isJust (BS.stripPrefix "timeout=" p) parsePrefs :: ToHeaderValue a => [a] -> Maybe a parsePrefs vals = @@ -172,7 +187,7 @@ fromHeaders allowTxDbOverride acceptedTzNames headers = prefMap = Map.fromList . fmap (\pref -> (toHeaderValue pref, pref)) prefAppliedHeader :: Preferences -> Maybe HTTP.Header -prefAppliedHeader Preferences {preferResolution, preferRepresentation, preferCount, preferTransaction, preferMissing, preferHandling, preferTimezone, preferMaxAffected } = +prefAppliedHeader Preferences{..} = if null prefsVals then Nothing else Just (HTTP.hPreferenceApplied, combined) @@ -187,6 +202,7 @@ prefAppliedHeader Preferences {preferResolution, preferRepresentation, preferCou , toHeaderValue <$> preferHandling , toHeaderValue <$> preferTimezone , if preferHandling == Just Strict then toHeaderValue <$> preferMaxAffected else Nothing + , if preferHandling == Just Strict then toHeaderValue <$> preferTimeout else Nothing ] -- | @@ -289,3 +305,10 @@ newtype PreferMaxAffected = PreferMaxAffected Int64 instance ToHeaderValue PreferMaxAffected where toHeaderValue (PreferMaxAffected n) = "max-affected=" <> show n + +-- | +-- Statement Timeout per request +newtype PreferTimeout = PreferTimeout Int64 + +instance ToHeaderValue PreferTimeout where + toHeaderValue (PreferTimeout n) = "timeout=" <> show n diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 64e31b1943..6ae3c3cf18 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -169,7 +169,7 @@ postgrestResponse appState conf@AppConfig{..} maybeSchemaCache authResult@AuthRe prefs = ApiRequest.userPreferences conf req timezones (parseTime, apiReq@ApiRequest{..}) <- withTiming $ liftEither . mapLeft Error.ApiRequestError $ ApiRequest.userApiRequest conf prefs req body - (planTime, plan) <- withTiming $ liftEither $ Plan.actionPlan iAction conf apiReq sCache + (planTime, plan) <- withTiming $ liftEither $ Plan.actionPlan iAction conf apiReq authResult sCache let mainQ = Query.mainQuery plan conf apiReq authResult configDbPreRequest tx = MainTx.mainTx mainQ conf authResult apiReq plan sCache diff --git a/src/PostgREST/AppState.hs b/src/PostgREST/AppState.hs index 7841d30804..7874c478ae 100644 --- a/src/PostgREST/AppState.hs +++ b/src/PostgREST/AppState.hs @@ -63,7 +63,8 @@ import PostgREST.Config (AppConfig (..), readAppConfig) import PostgREST.Config.Database (queryDbSettings, queryPgVersion, - queryRoleSettings) + queryRoleSettings, + queryRoleTimeoutSettings) import PostgREST.Config.PgVersion (PgVersion (..), minimumPgVersion) import PostgREST.SchemaCache (SchemaCache (..), @@ -461,7 +462,17 @@ readInDbConfig startingUp appState@AppState{stateObserver=observer} = do Right x -> pure x else pure mempty - readAppConfig dbSettings (configFilePath conf) (Just $ configDbUri conf) roleSettings roleIsolationLvl >>= \case + roleTimeoutSettings <- + if configDbConfig conf then do + rSettings <- usePool appState (queryRoleTimeoutSettings $ configDbPreparedStatements conf) + case rSettings of + Left e -> do + observer $ QueryRoleSettingsErrorObs e + pure mempty + Right x -> pure x + else + pure mempty + readAppConfig dbSettings (configFilePath conf) (Just $ configDbUri conf) roleSettings roleTimeoutSettings roleIsolationLvl >>= \case Left err -> if startingUp then panic err -- die on invalid config if the program is starting up diff --git a/src/PostgREST/CLI.hs b/src/PostgREST/CLI.hs index a635aa0814..f5b63f2afb 100644 --- a/src/PostgREST/CLI.hs +++ b/src/PostgREST/CLI.hs @@ -30,7 +30,7 @@ import Protolude main :: CLI -> IO () main CLI{cliCommand, cliPath} = do conf <- - either panic identity <$> Config.readAppConfig mempty cliPath Nothing mempty mempty + either panic identity <$> Config.readAppConfig mempty cliPath Nothing mempty mempty mempty case cliCommand of Client adminCmd -> runClientCommand conf adminCmd Run runCmd -> runAppCommand conf runCmd diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs index a5706a74d8..da7a1ec242 100644 --- a/src/PostgREST/Config.hs +++ b/src/PostgREST/Config.hs @@ -57,7 +57,8 @@ import System.Environment (getEnvironment) import System.Posix.Types (FileMode) import PostgREST.Config.Database (RoleIsolationLvl, - RoleSettings) + RoleSettings, + RoleTimeoutSettings) import PostgREST.Config.JSPath (FilterExp (..), JSPath, JSPathExp (..), dumpJSPath, pRoleClaimKey) @@ -117,6 +118,10 @@ data AppConfig = AppConfig , configAdminServerHost :: Text , configAdminServerPort :: Maybe Int , configRoleSettings :: RoleSettings + -- Cached statement timeout settings converted to number of seconds. They + -- are never applied, only used to check max allowed timeout for a role + -- when "Prefer: timeout=" header is used. + , configRoleTimeoutSettings :: RoleTimeoutSettings , configRoleIsoLvl :: RoleIsolationLvl , configInternalSCQuerySleep :: Maybe Int32 , configInternalSCLoadSleep :: Maybe Int32 @@ -227,13 +232,13 @@ instance JustIfMaybe a (Maybe a) where -- | Reads and parses the config and overrides its parameters from env vars, -- files or db settings. -readAppConfig :: [(Text, Text)] -> Maybe FilePath -> Maybe Text -> RoleSettings -> RoleIsolationLvl -> IO (Either Text AppConfig) -readAppConfig dbSettings optPath prevDbUri roleSettings roleIsolationLvl = do +readAppConfig :: [(Text, Text)] -> Maybe FilePath -> Maybe Text -> RoleSettings -> RoleTimeoutSettings -> RoleIsolationLvl -> IO (Either Text AppConfig) +readAppConfig dbSettings optPath prevDbUri roleSettings roleTimeoutSettings roleIsolationLvl = do env <- readPGRSTEnvironment -- if no filename provided, start with an empty map to read config from environment conf <- maybe (return $ Right M.empty) loadConfig optPath - case C.runParser (parser optPath env dbSettings roleSettings roleIsolationLvl) =<< mapLeft show conf of + case C.runParser (parser optPath env dbSettings roleSettings roleTimeoutSettings roleIsolationLvl) =<< mapLeft show conf of Left err -> return . Left $ "Error in config " <> err Right parsedConfig -> @@ -250,8 +255,8 @@ readAppConfig dbSettings optPath prevDbUri roleSettings roleIsolationLvl = do readSecretFile =<< readDbUriFile prevDbUri parsedConfig -parser :: Maybe FilePath -> Environment -> [(Text, Text)] -> RoleSettings -> RoleIsolationLvl -> C.Parser C.Config AppConfig -parser optPath env dbSettings roleSettings roleIsolationLvl = +parser :: Maybe FilePath -> Environment -> [(Text, Text)] -> RoleSettings -> RoleTimeoutSettings -> RoleIsolationLvl -> C.Parser C.Config AppConfig +parser optPath env dbSettings roleSettings roleTimeoutSettings roleIsolationLvl = AppConfig <$> parseAppSettings "app.settings" <*> (fromMaybe False <$> optBool "db-aggregates-enabled") @@ -305,6 +310,7 @@ parser optPath env dbSettings roleSettings roleIsolationLvl = (optString "server-host")) <*> parseAdminServerPort "admin-server-port" <*> pure roleSettings + <*> pure roleTimeoutSettings <*> pure roleIsolationLvl <*> optInt "internal-schema-cache-query-sleep" <*> optInt "internal-schema-cache-load-sleep" diff --git a/src/PostgREST/Config/Database.hs b/src/PostgREST/Config/Database.hs index aff4b5b8a3..59ac34b0ea 100644 --- a/src/PostgREST/Config/Database.hs +++ b/src/PostgREST/Config/Database.hs @@ -5,7 +5,9 @@ module PostgREST.Config.Database , queryDbSettings , queryPgVersion , queryRoleSettings + , queryRoleTimeoutSettings , RoleSettings + , RoleTimeoutSettings , RoleIsolationLvl , TimezoneNames , toIsolationLevel @@ -29,6 +31,7 @@ import NeatInterpolation (trimming) import Protolude type RoleSettings = (HM.HashMap ByteString (HM.HashMap ByteString ByteString)) +type RoleTimeoutSettings = HM.HashMap ByteString Int64 type RoleIsolationLvl = HM.HashMap ByteString SQL.IsolationLevel type TimezoneNames = Set Text -- cache timezone names for prefer timezone= @@ -183,6 +186,70 @@ queryRoleSettings pgVer prepared = rows :: HD.Result [(Text, Maybe Text, [(Text, Text)])] rows = HD.rowList $ (,,) <$> column HD.text <*> nullableColumn HD.text <*> compositeArrayColumn ((,) <$> compositeField HD.text <*> compositeField HD.text) +-- | Query Statement Timeout values for all roles +-- +-- For roles without statement_timeout, we fetch database/cluster +-- statement_timeout as an implicit timeout for that role. +-- +-- In our logic, we cache statement_timeout values after transforming +-- them to seconds. The transformation results in having a case of: +-- +-- +----------+--------------------+---------------------------+ +-- | Role | Timeout Setting | Cached Timeout in Seconds | +-- +----------+--------------------+---------------------------+ +-- | role1 | 0s | 0 | +-- +----------+--------------------+---------------------------+ +-- | role2 | 300ms | (0.3 gets truncated to 0) | +-- +----------+--------------------+---------------------------+ +-- +-- For role1, 0s setting mean statement_timeout is "disabled". +-- For role2, statement_timeout is 300ms +-- +-- However, both get cached to zero. So, to differentiate these +-- two distinct cases, the query caches "disabled" timeout as NULL +-- and < 1s timeouts as 0. +-- +-- Final Query Output: +-- +----------+--------------------+---------------------------+ +-- | Role | Timeout Setting | Cached Timeout in Seconds | +-- +----------+--------------------+---------------------------+ +-- | role1 | 0s | NULL | +-- +----------+--------------------+---------------------------+ +-- | role2 | 300ms | 0 | +-- +----------+--------------------+---------------------------+ +queryRoleTimeoutSettings :: Bool -> Session RoleTimeoutSettings +queryRoleTimeoutSettings prepared = + let transaction = if prepared then SQL.transaction else SQL.unpreparedTransaction in + transaction SQL.ReadCommitted SQL.Read $ SQL.statement mempty $ SQL.Statement sql HE.noParams (processRows <$> rows) prepared + where + sql = encodeUtf8 [trimming| + WITH role_timeouts AS ( + SELECT + r.rolname, + CASE + WHEN s.setting IS NOT NULL THEN substr(s.setting, strpos(s.setting, '=') + 1) + ELSE (SELECT setting || unit FROM pg_settings WHERE name = 'statement_timeout') + END AS statement_timeout + FROM pg_auth_members m + JOIN pg_roles r ON r.oid = m.roleid + LEFT JOIN LATERAL unnest(r.rolconfig) AS s(setting) ON true + WHERE m.member = current_user::regrole::oid + AND (s.setting LIKE 'statement_timeout=%' OR s.setting IS NULL) + ) + SELECT + rolname, + nullif(extract(epoch FROM statement_timeout::interval)::numeric, 0)::int as statement_timeout_in_seconds + FROM role_timeouts + |] + + processRows :: [(Text, Maybe Int64)] -> RoleTimeoutSettings + processRows rs = HM.fromList $ first encodeUtf8 <$> rowsWRoleTimeouts + where + rowsWRoleTimeouts = [ (x, y) | (x, Just y) <- rs ] + + rows :: HD.Result [(Text, Maybe Int64)] + rows = HD.rowList $ (,) <$> column HD.text <*> nullableColumn HD.int8 + column :: HD.Value a -> HD.Row a column = HD.column . HD.nonNullable diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 57ba2b4b46..35d19d04b5 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -100,6 +100,7 @@ data ApiRequestError | InvalidResourcePath | OpenAPIDisabled | MaxAffectedRpcViolation + | TimeoutConstraintError Int64 deriving Show data QPError = QPError Text Text @@ -143,6 +144,7 @@ instance PgrstError ApiRequestError where status InvalidResourcePath = HTTP.status404 status OpenAPIDisabled = HTTP.status404 status MaxAffectedRpcViolation = HTTP.status400 + status TimeoutConstraintError{} = HTTP.status400 headers _ = mempty @@ -190,6 +192,7 @@ instance ErrorBody ApiRequestError where code OpenAPIDisabled = "PGRST126" code NotImplemented{} = "PGRST127" code MaxAffectedRpcViolation = "PGRST128" + code TimeoutConstraintError{} = "PGRST129" -- MESSAGE: Text message (QueryParamError (QPError msg _)) = msg @@ -216,6 +219,7 @@ instance ErrorBody ApiRequestError where message OpenAPIDisabled = "Root endpoint metadata is disabled" message (NotImplemented _) = "Feature not implemented" message MaxAffectedRpcViolation = "Function must return SETOF or TABLE when max-affected preference is used with handling=strict" + message TimeoutConstraintError{} = "Timeout preference exceeded maximum allowed" -- DETAILS: Maybe JSON.Value details (QueryParamError (QPError _ dets)) = Just $ JSON.String dets @@ -231,6 +235,7 @@ instance ErrorBody ApiRequestError where details (InvalidPreferences prefs) = Just $ JSON.String $ T.decodeUtf8 ("Invalid preferences: " <> BS.intercalate ", " prefs) details (MaxAffectedViolationError n) = Just $ JSON.String $ T.unwords ["The query affects", show n, "rows"] details (NotImplemented details') = Just $ JSON.String details' + details (TimeoutConstraintError t) = Just $ JSON.String $ "The maximum timeout allowed is " <> show t <> "s" details _ = Nothing @@ -238,6 +243,7 @@ instance ErrorBody ApiRequestError where hint (NotEmbedded resource) = Just $ JSON.String $ "Verify that '" <> resource <> "' is included in the 'select' query parameter." hint (PGRSTParseError raiseErr) = Just $ JSON.String $ pgrstParseErrorHint raiseErr hint (UnacceptableSchema _ schemas) = Just $ JSON.String $ "Only the following schemas are exposed: " <> T.intercalate ", " schemas + hint TimeoutConstraintError{} = Just $ JSON.String "Reduce the timeout" hint _ = Nothing diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index e0d632ee40..7a7a110109 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -38,6 +38,7 @@ import Data.Maybe (fromJust) import Data.Tree (Tree (..)) import PostgREST.ApiRequest (ApiRequest (..)) +import PostgREST.Auth.Types (AuthResult (..)) import PostgREST.Config (AppConfig (..)) import PostgREST.Error (ApiRequestError (..), Error (..), @@ -144,29 +145,42 @@ data InfoPlan | RoutineInfoPlan Routine -- info about function | SchemaInfoPlan -- info about schema cache -actionPlan :: Action -> AppConfig -> ApiRequest -> SchemaCache -> Either Error ActionPlan -actionPlan act conf apiReq sCache = case act of - ActDb dbAct -> Db <$> dbActionPlan dbAct conf apiReq sCache +actionPlan :: Action -> AppConfig -> ApiRequest -> AuthResult -> SchemaCache -> Either Error ActionPlan +actionPlan act conf apiReq authResult sCache = case act of + ActDb dbAct -> Db <$> dbActionPlan dbAct conf apiReq authResult sCache ActRelationInfo ident -> pure . NoDb $ RelInfoPlan ident ActRoutineInfo ident inv -> let crPln = callReadPlan ident conf sCache apiReq inv in NoDb . RoutineInfoPlan . crProc <$> crPln ActSchemaInfo -> pure $ NoDb SchemaInfoPlan -dbActionPlan :: DbAction -> AppConfig -> ApiRequest -> SchemaCache -> Either Error DbActionPlan -dbActionPlan dbAct conf apiReq sCache = case dbAct of - ActRelationRead identifier headersOnly -> - toDbActPlan <$> wrappedReadPlan identifier conf sCache apiReq headersOnly - ActRelationMut identifier mut -> - toDbActPlan <$> mutateReadPlan mut apiReq identifier conf sCache - ActRoutine identifier invMethod -> - toDbActPlan <$> callReadPlan identifier conf sCache apiReq invMethod - ActSchemaRead tSchema headersOnly -> - MayUseDb <$> inspectPlan apiReq headersOnly tSchema - where - toDbActPlan pl = case pMedia pl of - MTVndPlan{} -> DbCrud True pl - _ -> DbCrud False pl +dbActionPlan :: DbAction -> AppConfig -> ApiRequest -> AuthResult -> SchemaCache -> Either Error DbActionPlan +dbActionPlan dbAct conf apiReq authResult sCache = do + failPreferTimeout (iPreferences apiReq) conf authResult + case dbAct of + ActRelationRead identifier headersOnly -> + toDbActPlan <$> wrappedReadPlan identifier conf sCache apiReq headersOnly + ActRelationMut identifier mut -> + toDbActPlan <$> mutateReadPlan mut apiReq identifier conf sCache + ActRoutine identifier invMethod -> + toDbActPlan <$> callReadPlan identifier conf sCache apiReq invMethod + ActSchemaRead tSchema headersOnly -> + MayUseDb <$> inspectPlan apiReq headersOnly tSchema + where + toDbActPlan pl = case pMedia pl of + MTVndPlan{} -> DbCrud True pl + _ -> DbCrud False pl + +-- | Fail when Prefer: timeout value is greater than the statement_timeout +-- setting of the role. +failPreferTimeout :: Preferences -> AppConfig -> AuthResult -> Either Error () +failPreferTimeout Preferences{preferTimeout=(Just (PreferTimeout t)), preferHandling=Just Strict} AppConfig{..} AuthResult{..} = + case roleTimeout of + Nothing -> Right () + Just rt -> when (t > rt) $ Left $ ApiRequestError $ TimeoutConstraintError rt + where + roleTimeout = HM.lookup authRole configRoleTimeoutSettings +failPreferTimeout _ _ _ = Right () wrappedReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Bool -> Either Error CrudPlan wrappedReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferences{..},..} headersOnly = do diff --git a/src/PostgREST/Query/PreQuery.hs b/src/PostgREST/Query/PreQuery.hs index d3a35bee3f..1240651d32 100644 --- a/src/PostgREST/Query/PreQuery.hs +++ b/src/PostgREST/Query/PreQuery.hs @@ -17,7 +17,9 @@ import qualified Hasql.DynamicStatements.Snippet as SQL hiding (sql) import PostgREST.ApiRequest (ApiRequest (..)) -import PostgREST.ApiRequest.Preferences (PreferTimezone (..), +import PostgREST.ApiRequest.Preferences (PreferHandling (..), + PreferTimeout (..), + PreferTimezone (..), Preferences (..)) import PostgREST.Auth.Types (AuthResult (..)) import PostgREST.Config (AppConfig (..)) @@ -35,11 +37,11 @@ import Protolude hiding (Handler) -- sets transaction variables txVarQuery :: DbActionPlan -> AppConfig -> AuthResult -> ApiRequest -> SQL.Snippet -txVarQuery dbActPlan AppConfig{..} AuthResult{..} ApiRequest{..} = +txVarQuery dbActPlan AppConfig{..} AuthResult{..} ApiRequest{iPreferences=Preferences{..}, ..} = -- To ensure `GRANT SET ON PARAMETER TO authenticator` works, the role settings must be set before the impersonated role. -- Otherwise the GRANT SET would have to be applied to the impersonated role. See https://github.com/PostgREST/postgrest/issues/3045 "select " <> intercalateSnippet ", " ( - searchPathSql : roleSettingsSql ++ roleSql ++ claimsSql ++ [methodSql, pathSql] ++ headersSql ++ cookiesSql ++ timezoneSql ++ funcSettingsSql ++ appSettingsSql + searchPathSql : roleSettingsSql ++ roleSql ++ claimsSql ++ [methodSql, pathSql] ++ headersSql ++ cookiesSql ++ timezoneSql ++ timeoutSql ++ funcSettingsSql ++ appSettingsSql ) where methodSql = setConfigWithConstantName ("request.method", iMethod) @@ -50,7 +52,12 @@ txVarQuery dbActPlan AppConfig{..} AuthResult{..} ApiRequest{..} = roleSql = [setConfigWithConstantName ("role", authRole)] roleSettingsSql = setConfigWithDynamicName <$> HM.toList (fromMaybe mempty $ HM.lookup authRole configRoleSettings) appSettingsSql = setConfigWithDynamicName . join bimap toUtf8 <$> configAppSettings - timezoneSql = maybe mempty (\(PreferTimezone tz) -> [setConfigWithConstantName ("timezone", tz)]) $ preferTimezone iPreferences + timezoneSql = maybe mempty (\(PreferTimezone tz) -> [setConfigWithConstantName ("timezone", tz)]) preferTimezone + timeoutSql = maybe mempty (\(PreferTimeout t) -> + if preferHandling == Just Strict + then [setConfigWithConstantName ("statement_timeout", show t <> "s")] + else mempty + ) preferTimeout funcSettingsSql = setConfigWithDynamicName . join bimap toUtf8 <$> funcSettings searchPathSql = let schemas = escapeIdentList (iSchema : configDbExtraSearchPath) in diff --git a/src/PostgREST/Response.hs b/src/PostgREST/Response.hs index 8498e7925f..767a6d4e67 100644 --- a/src/PostgREST/Response.hs +++ b/src/PostgREST/Response.hs @@ -299,4 +299,4 @@ responsePreferences plan ApiRequest{iPreferences=Preferences{..}, iQueryParams=Q CallReadPlan{} -> preferMaxAffected _ -> Nothing - in Preferences preferResolution' preferRepresentation' preferCount preferTransaction preferMissing' preferHandling preferTimezone preferMaxAffected' [] + in Preferences preferResolution' preferRepresentation' preferCount preferTransaction preferMissing' preferHandling preferTimezone preferMaxAffected' preferTimeout [] diff --git a/test/io/__snapshots__/test_cli/test_schema_cache_snapshot[dbRoutines].yaml b/test/io/__snapshots__/test_cli/test_schema_cache_snapshot[dbRoutines].yaml index 61a871c89a..992a73ff8e 100644 --- a/test/io/__snapshots__/test_cli/test_schema_cache_snapshot[dbRoutines].yaml +++ b/test/io/__snapshots__/test_cli/test_schema_cache_snapshot[dbRoutines].yaml @@ -252,6 +252,23 @@ pdSchema: public pdVolatility: Volatile +- - qiName: reload_pgrst_schema + qiSchema: public + - - pdDescription: null + pdFuncSettings: [] + pdHasVariadic: false + pdName: reload_pgrst_schema + pdParams: [] + pdReturnType: + contents: + contents: + qiName: void + qiSchema: pg_catalog + tag: Scalar + tag: Single + pdSchema: public + pdVolatility: Volatile + - - qiName: one_sec_timeout qiSchema: public - - pdDescription: null diff --git a/test/io/fixtures/roles.sql b/test/io/fixtures/roles.sql index 1a63822538..0857180b06 100644 --- a/test/io/fixtures/roles.sql +++ b/test/io/fixtures/roles.sql @@ -1,18 +1,23 @@ DROP ROLE IF EXISTS postgrest_test_anonymous, postgrest_test_author, postgrest_test_serializable, postgrest_test_repeatable_read, - postgrest_test_w_superuser_settings; + postgrest_test_w_superuser_settings, postgrest_test_timeout_ms, + postgrest_test_timeout_s, postgrest_test_no_timeout; CREATE ROLE postgrest_test_anonymous; CREATE ROLE postgrest_test_author; CREATE ROLE postgrest_test_serializable; CREATE ROLE postgrest_test_repeatable_read; CREATE ROLE postgrest_test_w_superuser_settings; +CREATE ROLE postgrest_test_timeout_ms; +CREATE ROLE postgrest_test_timeout_s; +CREATE ROLE postgrest_test_no_timeout; -- no timeout for this role, default to db or cluster level statement_timeout GRANT postgrest_test_anonymous, postgrest_test_author, postgrest_test_serializable, postgrest_test_repeatable_read, - postgrest_test_w_superuser_settings TO :PGUSER; + postgrest_test_w_superuser_settings, postgrest_test_timeout_ms, + postgrest_test_timeout_s, postgrest_test_no_timeout TO :PGUSER; ALTER ROLE :PGUSER SET pgrst.db_anon_role = 'postgrest_test_anonymous'; ALTER ROLE postgrest_test_serializable SET default_transaction_isolation = 'serializable'; @@ -23,3 +28,6 @@ ALTER ROLE postgrest_test_w_superuser_settings SET log_min_messages = 'fatal'; ALTER ROLE postgrest_test_anonymous SET statement_timeout TO '5s'; ALTER ROLE postgrest_test_author SET statement_timeout TO '10s'; + +ALTER ROLE postgrest_test_timeout_ms SET statement_timeout TO '10ms'; +ALTER ROLE postgrest_test_timeout_s SET statement_timeout TO '10s'; diff --git a/test/io/fixtures/schema.sql b/test/io/fixtures/schema.sql index b84889d27d..af46201dec 100644 --- a/test/io/fixtures/schema.sql +++ b/test/io/fixtures/schema.sql @@ -111,6 +111,11 @@ end $_$ volatile security definer language plpgsql ; create function reload_pgrst_config() returns void as $_$ begin perform pg_notify('pgrst', 'reload config'); +end $_$ language plpgsql; + +create function reload_pgrst_schema() returns void as $_$ +begin + perform pg_notify('pgrst', 'reload schema'); end $_$ language plpgsql ; create or replace function sleep(seconds double precision) returns void as $$ diff --git a/test/io/test_prefer_header.py b/test/io/test_prefer_header.py new file mode 100644 index 0000000000..ca9678a659 --- /dev/null +++ b/test/io/test_prefer_header.py @@ -0,0 +1,163 @@ +"Prefer: timeout header tests for PostgREST (involves IO)" + +from config import SECRET +from util import jwtauthheader, execute_sql_statement_using_superuser +from postgrest import ( + run, + sleep_until_postgrest_scache_reload, + sleep_until_postgrest_config_reload, +) + + +def test_prefer_timeout_header_with_strict_handling(defaultenv): + "Test Prefer: timeout header with handling=strict" + + with run(env=defaultenv) as postgrest: + # Fails when hits timeout + headers = {"Prefer": "handling=strict, timeout=1"} + response = postgrest.session.get("/rpc/sleep?seconds=2", headers=headers) + assert response.status_code == 500 + assert response.json() == { + "code": "57014", + "message": "canceling statement due to statement timeout", + "details": None, + "hint": None, + } + + # Fails when sleep equals the timeout + headers = {"Prefer": "handling=strict, timeout=2"} + response = postgrest.session.get("/rpc/sleep?seconds=2", headers=headers) + assert response.status_code == 500 + assert response.json() == { + "code": "57014", + "message": "canceling statement due to statement timeout", + "details": None, + "hint": None, + } + + # Succeeds when timeout is not hit + headers = {"Prefer": "handling=strict, timeout=2"} + response = postgrest.session.get("/rpc/sleep?seconds=1", headers=headers) + assert response.status_code == 204 + assert response.headers["Preference-Applied"] == "handling=strict, timeout=2" + + +def test_prefer_timeout_header_with_lenient_handling(defaultenv): + "Test Prefer: timeout header with handling=lenient" + + with run(env=defaultenv) as postgrest: + # Timeout header is ignored for lenient handling + headers = {"Prefer": "handling=lenient, timeout=1"} + response = postgrest.session.get("/rpc/sleep?seconds=2", headers=headers) + + assert response.status_code == 204 + assert response.headers["Preference-Applied"] == "handling=lenient" + + +def test_prefer_timeout_header_with_role_timeout(defaultenv): + "Test Prefer: timeout header with role timeout" + + env = { + **defaultenv, + "PGRST_JWT_SECRET": SECRET, + } + + with run(env=env) as postgrest: + # should fail when timeout is more than role's statement timeout + authheader = jwtauthheader({"role": "postgrest_test_timeout_ms"}, SECRET) + headers = { + **authheader, + "Prefer": "handling=strict, timeout=5", # role timeout is 10ms, so this should fail + } + + response = postgrest.session.get("/rpc/sleep?seconds=2", headers=headers) + assert response.status_code == 400 + assert response.json() == { + "code": "PGRST129", + "message": "Timeout preference exceeded maximum allowed", + "details": "The maximum timeout allowed is 0s", + "hint": "Reduce the timeout", + } + + # should fail when timeout is more than role's statement timeout + authheader = jwtauthheader({"role": "postgrest_test_timeout_s"}, SECRET) + headers = { + **authheader, + "Prefer": "handling=strict, timeout=15", # role timeout is 10s, so this should fail + } + + response = postgrest.session.get("/rpc/sleep?seconds=2", headers=headers) + assert response.json() == { + "code": "PGRST129", + "message": "Timeout preference exceeded maximum allowed", + "details": "The maximum timeout allowed is 10s", + "hint": "Reduce the timeout", + } + assert response.status_code == 400 + + # should succeed when timeout is less than role's statement timeout + authheader = jwtauthheader({"role": "postgrest_test_timeout_s"}, SECRET) + headers = { + **authheader, + "Prefer": "handling=strict, timeout=5", # role timeout is 10s, so this should succeed + } + + response = postgrest.session.get("/rpc/sleep?seconds=2", headers=headers) + assert response.status_code == 204 + assert response.headers["Preference-Applied"] == "handling=strict, timeout=5" + + +def test_prefer_timeout_header_with_global_timeout(defaultenv): + "Test Prefer: timeout header with database or cluster level timeout" + + env = {**defaultenv, "PGRST_JWT_SECRET": SECRET} + + with run(env=env) as postgrest: + # set global statement_timeout to 3s using postgres superuser + execute_sql_statement_using_superuser( + env, "ALTER DATABASE postgres SET statement_timeout TO '3s';" + ) + + # reload schema and config + response = postgrest.session.post("/rpc/reload_pgrst_schema") + assert response.status_code == 204 + sleep_until_postgrest_scache_reload() + + response = postgrest.session.post("/rpc/reload_pgrst_config") + assert response.status_code == 204 + sleep_until_postgrest_config_reload() + + # should fail when timeout is more than global statement timeout + authheader = jwtauthheader({"role": "postgrest_test_no_timeout"}, SECRET) + headers = { + **authheader, + "Prefer": "handling=strict, timeout=5", # global timeout is 3s, so this should fail + } + response = postgrest.session.get("/rpc/sleep?seconds=4", headers=headers) + assert response.json() == { + "code": "PGRST129", + "message": "Timeout preference exceeded maximum allowed", + "details": "The maximum timeout allowed is 3s", + "hint": "Reduce the timeout", + } + assert response.status_code == 400 + + # reset global statement_timeout using postgres superuser + execute_sql_statement_using_superuser( + env, "ALTER DATABASE postgres RESET statement_timeout;" + ) + + +def test_prefer_timeout_header_with_negative_timeout(defaultenv): + "Test Prefer: timeout header with negative and zero timeout" + + with run(env=defaultenv) as postgrest: + # -ve timeout values are ignored + headers = {"Prefer": "handling=strict, timeout=-1"} + response = postgrest.session.get("/rpc/sleep?seconds=2", headers=headers) + assert response.status_code == 204 + + # 0 timeout value is ignored + headers = {"Prefer": "handling=stricte timeout=0"} + response = postgrest.session.get("/rpc/sleep?seconds=2", headers=headers) + assert response.status_code == 204 diff --git a/test/io/util.py b/test/io/util.py index e1a8d0977d..3c5a0497e1 100644 --- a/test/io/util.py +++ b/test/io/util.py @@ -1,5 +1,6 @@ import threading import jwt +import os class Thread(threading.Thread): @@ -46,3 +47,12 @@ def parse_server_timings_header(header): _, duration = duration_text.split("=") timings[name.strip()] = float(duration) return timings + + +def execute_sql_statement_using_superuser(env, statement): + "Execute SQL statement with psql using superuser" + + superuser = "postgres" + os.system( + f'psql -d {env["PGDATABASE"]} -U {superuser} -h {env["PGHOST"]} --set ON_ERROR_STOP=1 -a -c "{statement}"' + ) diff --git a/test/spec/SpecHelper.hs b/test/spec/SpecHelper.hs index a35d7db920..d62eb3a6b1 100644 --- a/test/spec/SpecHelper.hs +++ b/test/spec/SpecHelper.hs @@ -164,6 +164,7 @@ baseCfg = let secret = encodeUtf8 "reallyreallyreallyreallyverysafe" in , configAdminServerHost = "localhost" , configAdminServerPort = Nothing , configRoleSettings = mempty + , configRoleTimeoutSettings = mempty , configRoleIsoLvl = mempty , configInternalSCQuerySleep = Nothing , configInternalSCLoadSleep = Nothing