From c7e27becd2e6eb93b20965b9f22701eaad42a764 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 1 Jan 2016 15:27:53 -0500 Subject: [PATCH] Teach flatten_reloptions() to quote option values safely. flatten_reloptions() supposed that it didn't really need to do anything beyond inserting commas between reloption array elements. However, in principle the value of a reloption could be nearly anything, since the grammar allows a quoted string there. Any restrictions on it would come from validity checking appropriate to the particular option, if any. A reloption value that isn't a simple identifier or number could thus lead to dump/reload failures due to syntax errors in CREATE statements issued by pg_dump. We've gotten away with not worrying about this so far with the core-supported reloptions, but extensions might allow reloption values that cause trouble, as in bug #13840 from Kouhei Sutou. To fix, split the reloption array elements explicitly, and then convert any value that doesn't look like a safe identifier to a string literal. (The details of the quoting rule could be debated, but this way is safe and requires little code.) While we're at it, also quote reloption names if they're not safe identifiers; that may not be a likely problem in the field, but we might as well try to be bulletproof here. It's been like this for a long time, so back-patch to all supported branches. Kouhei Sutou, adjusted some by me --- src/backend/utils/adt/ruleutils.c | 63 +++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 280808ae4f..1c7cb5bb30 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9858,18 +9858,59 @@ flatten_reloptions(Oid relid) Anum_pg_class_reloptions, &isnull); if (!isnull) { - Datum sep, - txt; + StringInfoData buf; + Datum *options; + int noptions; + int i; - /* - * We want to use array_to_text(reloptions, ', ') --- but - * DirectFunctionCall2(array_to_text) does not work, because - * array_to_text() relies on flinfo to be valid. So use - * OidFunctionCall2. - */ - sep = CStringGetTextDatum(", "); - txt = OidFunctionCall2(F_ARRAY_TO_TEXT, reloptions, sep); - result = TextDatumGetCString(txt); + initStringInfo(&buf); + + deconstruct_array(DatumGetArrayTypeP(reloptions), + TEXTOID, -1, false, 'i', + &options, NULL, &noptions); + + for (i = 0; i < noptions; i++) + { + char *option = TextDatumGetCString(options[i]); + char *name; + char *separator; + char *value; + + /* + * Each array element should have the form name=value. If the "=" + * is missing for some reason, treat it like an empty value. + */ + name = option; + separator = strchr(option, '='); + if (separator) + { + *separator = '\0'; + value = separator + 1; + } + else + value = ""; + + if (i > 0) + appendStringInfoString(&buf, ", "); + appendStringInfo(&buf, "%s=", quote_identifier(name)); + + /* + * In general we need to quote the value; but to avoid unnecessary + * clutter, do not quote if it is an identifier that would not + * need quoting. (We could also allow numbers, but that is a bit + * trickier than it looks --- for example, are leading zeroes + * significant? We don't want to assume very much here about what + * custom reloptions might mean.) + */ + if (quote_identifier(value) == value) + appendStringInfoString(&buf, value); + else + simple_quote_literal(&buf, value); + + pfree(option); + } + + result = buf.data; } ReleaseSysCache(tuple);