From 404c45bac64d312033dcf0373f7b1c0133b03afc 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 0ab839dc73..3783e977dd 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9856,18 +9856,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);