Hi all, It's been brought to me that an extension may finish by breaking the assumptions ProcessUtility() relies on when calling standard_ProcessUtility(), causing breakages when passing down data to cascading utility hooks.
Isn't the state of the arguments given something we should check not only in the main entry point ProcessUtility() but also in standard_ProcessUtility(), to prevent issues if an extension incorrectly manipulates the arguments it needs to pass down to other modules that use the utility hook, like using a NULL query string? See the attached for the idea. Thanks, -- Michael
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 8de821f960..ee82551dde 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -88,6 +88,20 @@ static void ProcessUtilitySlow(ParseState *pstate, QueryCompletion *qc); static void ExecDropStmt(DropStmt *stmt, bool isTopLevel); + +/* + * Check the state of the arguments given to entry points for utility + * processing. + */ +#define UTILITY_CHECKS \ +do { \ + Assert(IsA(pstmt, PlannedStmt)); \ + Assert(pstmt->commandType == CMD_UTILITY); \ + Assert(queryString != NULL); /* required as of 8.4 */ \ + Assert(qc == NULL || qc->commandTag == CMDTAG_UNKNOWN); \ +} while (0) + + /* * CommandIsReadOnly: is an executable query read-only? * @@ -512,10 +526,7 @@ ProcessUtility(PlannedStmt *pstmt, DestReceiver *dest, QueryCompletion *qc) { - Assert(IsA(pstmt, PlannedStmt)); - Assert(pstmt->commandType == CMD_UTILITY); - Assert(queryString != NULL); /* required as of 8.4 */ - Assert(qc == NULL || qc->commandTag == CMDTAG_UNKNOWN); + UTILITY_CHECKS; /* * We provide a function hook variable that lets loadable plugins get @@ -559,6 +570,8 @@ standard_ProcessUtility(PlannedStmt *pstmt, ParseState *pstate; int readonly_flags; + UTILITY_CHECKS; + /* This can recurse, so check for excessive recursion */ check_stack_depth();
signature.asc
Description: PGP signature