From: Tom Lane <tgl@sss.pgh.pa.us>
To: Chapman Flack <jcflack@acm.org>
cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: tighten generic_option_name, or store more carefully in catalog?
In-reply-to: <683BBE52.8050900@acm.org>
References: <6830EB30.8090904@acm.org> <3020890.1748735715@sss.pgh.pa.us>
 <683BA390.7000903@acm.org> <3029560.1748740373@sss.pgh.pa.us>
 <683BBE52.8050900@acm.org>
Comments: In-reply-to Chapman Flack <jcflack@acm.org>
	message dated "Sat, 31 May 2025 22:43:30 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <557962.1748888019.0@sss.pgh.pa.us>
Date: Mon, 02 Jun 2025 14:13:43 -0400
Message-ID: <557970.1748888023@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/557970.1748888023%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <557962.1748888019.1@sss.pgh.pa.us>

Here's a proposed patch for the "=" issue.  Whether or not we should
rethink FDW validation behavior, doing so surely couldn't be
back-patched.  But I think this much should be.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="v1-disallow-equal-in-option-names.patch";
	charset="us-ascii"
Content-ID: <557962.1748888019.2@sss.pgh.pa.us>
Content-Description: v1-disallow-equal-in-option-names.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/c=
ommon/reloptions.c
index 46c1dce222d..50747c16396 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1243,8 +1243,9 @@ transformRelOptions(Datum oldOptions, List *defList,=
 const char *namspace,
 		}
 		else
 		{
-			text	   *t;
+			const char *name;
 			const char *value;
+			text	   *t;
 			Size		len;
 =

 			/*
@@ -1291,11 +1292,19 @@ transformRelOptions(Datum oldOptions, List *defLis=
t, const char *namspace,
 			 * have just "name", assume "name=3Dtrue" is meant.  Note: the
 			 * namespace is not output.
 			 */
+			name =3D def->defname;
 			if (def->arg !=3D NULL)
 				value =3D defGetString(def);
 			else
 				value =3D "true";
 =

+			/* Insist that name not contain "=3D", else "a=3Db=3Dc" is ambiguous *=
/
+			if (strchr(name, '=3D') !=3D NULL)
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("invalid option name \"%s\": must not contain \"=3D\"",
+								name)));
+
 			/*
 			 * This is not a great place for this test, but there's no other
 			 * convenient place to filter the option out. As WITH (oids =3D
@@ -1303,7 +1312,7 @@ transformRelOptions(Datum oldOptions, List *defList,=
 const char *namspace,
 			 * amount of ugly.
 			 */
 			if (acceptOidsOff && def->defnamespace =3D=3D NULL &&
-				strcmp(def->defname, "oids") =3D=3D 0)
+				strcmp(name, "oids") =3D=3D 0)
 			{
 				if (defGetBoolean(def))
 					ereport(ERROR,
@@ -1313,11 +1322,11 @@ transformRelOptions(Datum oldOptions, List *defLis=
t, const char *namspace,
 				continue;
 			}
 =

-			len =3D VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
+			len =3D VARHDRSZ + strlen(name) + 1 + strlen(value);
 			/* +1 leaves room for sprintf's trailing null */
 			t =3D (text *) palloc(len + 1);
 			SET_VARSIZE(t, len);
-			sprintf(VARDATA(t), "%s=3D%s", def->defname, value);
+			sprintf(VARDATA(t), "%s=3D%s", name, value);
 =

 			astate =3D accumArrayResult(astate, PointerGetDatum(t),
 									  false, TEXTOID,
diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/for=
eigncmds.c
index c14e038d54f..8d2d7431544 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -71,15 +71,26 @@ optionListToArray(List *options)
 	foreach(cell, options)
 	{
 		DefElem    *def =3D lfirst(cell);
+		const char *name;
 		const char *value;
 		Size		len;
 		text	   *t;
 =

+		name =3D def->defname;
 		value =3D defGetString(def);
-		len =3D VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
+
+		/* Insist that name not contain "=3D", else "a=3Db=3Dc" is ambiguous */
+		if (strchr(name, '=3D') !=3D NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("invalid option name \"%s\": must not contain \"=3D\"",
+							name)));
+
+		len =3D VARHDRSZ + strlen(name) + 1 + strlen(value);
+		/* +1 leaves room for sprintf's trailing null */
 		t =3D palloc(len + 1);
 		SET_VARSIZE(t, len);
-		sprintf(VARDATA(t), "%s=3D%s", def->defname, value);
+		sprintf(VARDATA(t), "%s=3D%s", name, value);
 =

 		astate =3D accumArrayResult(astate, PointerGetDatum(t),
 								  false, TEXTOID,

------- =_aaaaaaaaaa0--