Fix latent crash in do_text_output_multiline().

tglsfdc · tglsfdc · commit 8a4930e3faff · 2016-05-23T14:16:40.000-04:00
do_text_output_multiline() would fail (typically with a null pointer
dereference crash) if its input string did not end with a newline.  Such
cases do not arise in our current sources; but it certainly could happen
in future, or in extension code's usage of the function, so we should fix
it.  To fix, replace "eol += len" with "eol = text + len".

While at it, make two cosmetic improvements: mark the input string const,
and rename the argument from "text" to "txt" to dodge pgindent strangeness
(since "text" is a typedef name).

Even though this problem is only latent at present, it seems like a good
idea to back-patch the fix, since it's a very simple/safe patch and it's
not out of the realm of possibility that we might in future back-patch
something that expects sane behavior from do_text_output_multiline().

Per report from Hao Lee.

Report: &lt;CAGoxFiFPAGyPAJLcFxTB5cGhTW2yOVBDYeqDugYwV4dEd1L_Ag@mail.gmail.com&gt;
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
@@ -1325,33 +1325,32 @@ do_tup_output(TupOutputState *tstate, Datum *values, bool *isnull)
  * Should only be used with a single-TEXT-attribute tupdesc.
  */
 void
-do_text_output_multiline(TupOutputState *tstate, char *text)
+do_text_output_multiline(TupOutputState *tstate, const char *txt)
 {
 	Datum		values[1];
 	bool		isnull[1] = {false};
 
-	while (*text)
+	while (*txt)
 	{
-		char	   *eol;
+		const char *eol;
 		int			len;
 
-		eol = strchr(text, '\n');
+		eol = strchr(txt, '\n');
 		if (eol)
 		{
-			len = eol - text;
-
+			len = eol - txt;
 			eol++;
 		}
 		else
 		{
-			len = strlen(text);
-			eol += len;
+			len = strlen(txt);
+			eol = txt + len;
 		}
 
-		values[0] = PointerGetDatum(cstring_to_text_with_len(text, len));
+		values[0] = PointerGetDatum(cstring_to_text_with_len(txt, len));
 		do_tup_output(tstate, values, isnull);
 		pfree(DatumGetPointer(values[0]));
-		text = eol;
+		txt = eol;
 	}
 }
 
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
@@ -284,7 +284,7 @@ typedef struct TupOutputState
 extern TupOutputState *begin_tup_output_tupdesc(DestReceiver *dest,
 						 TupleDesc tupdesc);
 extern void do_tup_output(TupOutputState *tstate, Datum *values, bool *isnull);
-extern void do_text_output_multiline(TupOutputState *tstate, char *text);
+extern void do_text_output_multiline(TupOutputState *tstate, const char *txt);
 extern void end_tup_output(TupOutputState *tstate);
 
 /*

Original file line number	Diff line number	Diff line change
`@@ -1325,33 +1325,32 @@ do_tup_output(TupOutputState tstate, Datum values, bool *isnull)`
`1325`	`1325`	`* Should only be used with a single-TEXT-attribute tupdesc.`
`1326`	`1326`	`*/`
`1327`	`1327`	`void`
`1328`		`-do_text_output_multiline(TupOutputState tstate, char text)`
	`1328`	`+do_text_output_multiline(TupOutputState tstate, const char txt)`
`1329`	`1329`	`{`
`1330`	`1330`	`Datum values[1];`
`1331`	`1331`	`bool isnull[1] = {false};`
`1332`	`1332`
`1333`		`- while (*text)`
	`1333`	`+ while (*txt)`
`1334`	`1334`	`{`
`1335`		`- char *eol;`
	`1335`	`+ const char *eol;`
`1336`	`1336`	`int len;`
`1337`	`1337`
`1338`		`- eol = strchr(text, '\n');`
	`1338`	`+ eol = strchr(txt, '\n');`
`1339`	`1339`	`if (eol)`
`1340`	`1340`	`{`
`1341`		`- len = eol - text;`
`1342`		`-`
	`1341`	`+ len = eol - txt;`
`1343`	`1342`	`eol++;`
`1344`	`1343`	`}`
`1345`	`1344`	`else`
`1346`	`1345`	`{`
`1347`		`- len = strlen(text);`
`1348`		`- eol += len;`
	`1346`	`+ len = strlen(txt);`
	`1347`	`+ eol = txt + len;`
`1349`	`1348`	`}`
`1350`	`1349`
`1351`		`- values[0] = PointerGetDatum(cstring_to_text_with_len(text, len));`
	`1350`	`+ values[0] = PointerGetDatum(cstring_to_text_with_len(txt, len));`
`1352`	`1351`	`do_tup_output(tstate, values, isnull);`
`1353`	`1352`	`pfree(DatumGetPointer(values[0]));`
`1354`		`- text = eol;`
	`1353`	`+ txt = eol;`
`1355`	`1354`	`}`
`1356`	`1355`	`}`
`1357`	`1356`