public inbox for [email protected]  

help / color / mirror / Atom feed

From: Michael Paquier <[email protected]>
To: Postgres hackers <[email protected]>
Subject: Propagate sanity checks of ProcessUtility() to standard_ProcessUtility()?
Date: Thu, 29 Feb 2024 16:20:53 +0900
Message-ID: <[email protected]> (raw)

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


Attachments:

  [text/x-diff] utility-checks.patch (1.3K, ../[email protected]/2-utility-checks.patch)
  download | inline diff:
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();
 


  [application/pgp-signature] signature.asc (833B, ../[email protected]/3-signature.asc)
  download

view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Propagate sanity checks of ProcessUtility() to standard_ProcessUtility()?
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox