pgsql: Disallow newlines in parameter values to be set in ALTER SYSTEM.

Started by Tom Laneabout 10 years ago4 messagescomitters
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

Disallow newlines in parameter values to be set in ALTER SYSTEM.

As noted by Julian Schauder in bug #14063, the configuration-file parser
doesn't support embedded newlines in string literals. While there might
someday be a good reason to remove that restriction, there doesn't seem
to be one right now. However, ALTER SYSTEM SET could accept strings
containing newlines, since many of the variable-specific value-checking
routines would just see a newline as whitespace. This led to writing a
postgresql.auto.conf file that was broken and had to be removed manually.

Pending a reason to work harder, just throw an error if someone tries this.

In passing, fix several places in the ALTER SYSTEM logic that failed to
provide an errcode() for an ereport(), and thus would falsely log the
failure as an internal XX000 error.

Back-patch to 9.4 where ALTER SYSTEM was introduced.

Branch
------
REL9_5_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/f3d17491c49362b78da0c1a5b0691821dcc8c435

Modified Files
--------------
src/backend/utils/misc/guc.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Oleg Bartunov
oleg@sai.msu.su
In reply to: Tom Lane (#1)
Re: pgsql: Disallow newlines in parameter values to be set in ALTER SYSTEM.

On Tue, Apr 5, 2016 at 1:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Disallow newlines in parameter values to be set in ALTER SYSTEM.

it's also fixed problem with setting parameter to ''. Before

alter system SET shared_preload_libraries = '';

produced wrong postgresql.auto.conf and the only way was to use RESET

alter system RESET shared_preload_libraries;

Show quoted text

As noted by Julian Schauder in bug #14063, the configuration-file parser
doesn't support embedded newlines in string literals. While there might
someday be a good reason to remove that restriction, there doesn't seem
to be one right now. However, ALTER SYSTEM SET could accept strings
containing newlines, since many of the variable-specific value-checking
routines would just see a newline as whitespace. This led to writing a
postgresql.auto.conf file that was broken and had to be removed manually.

Pending a reason to work harder, just throw an error if someone tries this.

In passing, fix several places in the ALTER SYSTEM logic that failed to
provide an errcode() for an ereport(), and thus would falsely log the
failure as an internal XX000 error.

Back-patch to 9.4 where ALTER SYSTEM was introduced.

Branch
------
REL9_5_STABLE

Details
-------

http://git.postgresql.org/pg/commitdiff/f3d17491c49362b78da0c1a5b0691821dcc8c435

Modified Files
--------------
src/backend/utils/misc/guc.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#3Oleg Bartunov
oleg@sai.msu.su
In reply to: Oleg Bartunov (#2)
Re: pgsql: Disallow newlines in parameter values to be set in ALTER SYSTEM.

On Tue, Apr 5, 2016 at 9:05 AM, Oleg Bartunov <obartunov@gmail.com> wrote:

On Tue, Apr 5, 2016 at 1:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Disallow newlines in parameter values to be set in ALTER SYSTEM.

it's also fixed problem with setting parameter to ''. Before

alter system SET shared_preload_libraries = '';

produced wrong postgresql.auto.conf and the only way was to use RESET

alter system RESET shared_preload_libraries;

Ooops, it doesn't :(

FATAL: could not access file "": No such file or directory

cat /usr/local/pgsql-head/data/postgresql.auto.conf
# Do not edit this file manually!
# It will be overwritten by ALTER SYSTEM command.
shared_preload_libraries = '""'

Show quoted text

As noted by Julian Schauder in bug #14063, the configuration-file parser
doesn't support embedded newlines in string literals. While there might
someday be a good reason to remove that restriction, there doesn't seem
to be one right now. However, ALTER SYSTEM SET could accept strings
containing newlines, since many of the variable-specific value-checking
routines would just see a newline as whitespace. This led to writing a
postgresql.auto.conf file that was broken and had to be removed manually.

Pending a reason to work harder, just throw an error if someone tries
this.

In passing, fix several places in the ALTER SYSTEM logic that failed to
provide an errcode() for an ereport(), and thus would falsely log the
failure as an internal XX000 error.

Back-patch to 9.4 where ALTER SYSTEM was introduced.

Branch
------
REL9_5_STABLE

Details
-------

http://git.postgresql.org/pg/commitdiff/f3d17491c49362b78da0c1a5b0691821dcc8c435

Modified Files
--------------
src/backend/utils/misc/guc.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Oleg Bartunov (#3)
Re: pgsql: Disallow newlines in parameter values to be set in ALTER SYSTEM.

Oleg Bartunov <obartunov@gmail.com> writes:

On Tue, Apr 5, 2016 at 9:05 AM, Oleg Bartunov <obartunov@gmail.com> wrote:

it's also fixed problem with setting parameter to ''. Before
alter system SET shared_preload_libraries = '';

Ooops, it doesn't :(
FATAL: could not access file "": No such file or directory

cat /usr/local/pgsql-head/data/postgresql.auto.conf
# Do not edit this file manually!
# It will be overwritten by ALTER SYSTEM command.
shared_preload_libraries = '""'

I do not think this is a bug, or at least not an ALTER SYSTEM bug.
What you wrote there means a single list entry that is an empty string,
and ALTER SYSTEM reproduced that faithfully.

We do have a shortcoming here, which is that the SET syntax lacks any
way to explicitly set a GUC_LIST_INPUT variable to an empty list.
It's been like that for a long time with few complaints, but maybe
we should do something about it.

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers