Re: Potential issue with alter system

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Potential issue with alter system
Date: 2017-05-04 19:49:29
Message-ID: 26400.1493927369@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> So I did this:

> postgres=# alter system set archive_command to 'rsynv -av %p
> postgres(at)52(dot)3(dot)141(dot)224:/data/archive/%f
> ';

> Note the new line. It properly created in postgresql.auto.conf:

> archive_command = 'rsynv -av %p postgres(at)52(dot)3(dot)141(dot)224:/data/archive/%f
> '
> (note the new line)

Ugh. That's broken --- we don't support newlines within values in
postgresql.conf files.

[ pokes around ... ] HEAD seems to reject this properly:

regression=# alter system set archive_command to 'rsynv -av %p postgres(at)52(dot)3(dot)141(dot)224:/data/archive/%f
regression'# ';
ERROR: parameter value for ALTER SYSTEM must not contain a newline

> This is 9.5.2 (Yes, I know... I am in the process of setting up
> replication to 9.5.6 for failover).

Ah. [ digs further... ] Yup, we fixed that in 9.5.3:

Author: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Branch: master Release: REL9_6_BR [99f3b5613] 2016-04-04 18:05:23 -0400
Branch: REL9_5_STABLE Release: REL9_5_3 [f3d17491c] 2016-04-04 18:05:23 -0400
Branch: REL9_4_STABLE Release: REL9_4_8 [28148e258] 2016-04-04 18:05:24 -0400

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.

If you have other entries you want to keep in the postgresql.auto.conf
file, you could get away with manually editing it to remove the newline.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nikita Glukhov 2017-05-04 19:50:36 Re: Fix freeing of dangling IndexScanDesc.xs_hitup in GiST
Previous Message Tom Lane 2017-05-04 19:16:40 Re: Fix freeing of dangling IndexScanDesc.xs_hitup in GiST