Horton

Thread

Commits

  1. Correct pg_recvlogical server version test.

  2. Empty search_path in Autovacuum and non-psql/pgbench clients.

  1. pg_recvlogical broken in back branches

    Euler Taveira <euler@timbira.com.br> — 2018-04-17T06:01:33Z

    Hi,

An issue [1] reported that pg_recvlogical emitted an error in 9.6.8. I
confirmed that it was broken in recent minor versions (9.6.8, 9.5.12,
9.4.17 -- using same server/client version). It was broken by commit
582edc369cdbd348d68441fc50fa26a84afd0c1a and its siblings.

$ postgres --version
postgres (PostgreSQL) 9.6.8
$ pg_recvlogical --version
pg_recvlogical (PostgreSQL) 9.6.8
$ pg_recvlogical -d postgres --slot test_slot --create-slot -P test_decoding
ERRO:  syntax error
pg_recvlogical: could not clear search_path: ERRO:  syntax error

Replication protocol supports queries since 10 so the code seems correct to it.

  * The capacity to run normal SQL queries was added in PostgreSQL
  * 10, so the search path cannot be changed (by us or attackers) on
  * earlier versions.

It seems version >= 10 should be checked in old clients too. Version
9.6.8 could not connect to 9.4.17.

$ pg_recvlogical --version
pg_recvlogical (PostgreSQL) 9.6.8

$ psql -p 9994 postgres
psql (9.6.8, servidor 9.4.17)
Digite "help" para ajuda.
$ pg_recvlogical -p 9994 -d postgres --slot test_slot --create-slot -P
test_decoding
pg_recvlogical: could not clear search_path: ERRO:  syntax error


A proposed fix is attached. It should be applied to 9.4, 9.5, 9.6, and
10. (Although, client version 10 can connect to server version 10,
client version 10 can't connect to server version 9.6.)

Comments?


[1] https://github.com/eulerto/wal2json/issues/61


-- 
   Euler Taveira                                   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

  2. Re: pg_recvlogical broken in back branches

    Michael Paquier <michael@paquier.xyz> — 2018-04-17T06:38:13Z

    On Tue, Apr 17, 2018 at 03:01:33AM -0300, Euler Taveira wrote:
> A proposed fix is attached. It should be applied to 9.4, 9.5, 9.6, and
> 10. (Although, client version 10 can connect to server version 10,
> client version 10 can't connect to server version 9.6.)
> 
> Comments?

The exact same fix has already applied on all stable branches:
- af5fbb1286 -> REL9_4_STABLE
- 24ff0fe877 -> REL9_5_STABLE
- 59743deca9 -> REL9_6_STABLE
- e7d3a37d99 -> REL_10_STABLE
- 8d2814f274 -> master
Visibly you missed it. 
--
Michael

  3. Re: pg_recvlogical broken in back branches

    Euler Taveira <euler@timbira.com.br> — 2018-04-17T11:07:29Z

    2018-04-17 3:38 GMT-03:00 Michael Paquier <michael@paquier.xyz>:
> The exact same fix has already applied on all stable branches:
>
Sorry about the noise. I've only checked the REL9_6_8 tag and the tarball.


-- 
   Euler Taveira                                   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

  4. Re: pg_recvlogical broken in back branches

    Noah Misch <noah@leadboat.com> — 2018-04-22T21:55:51Z

    On Tue, Apr 17, 2018 at 03:38:13PM +0900, Michael Paquier wrote:
> On Tue, Apr 17, 2018 at 03:01:33AM -0300, Euler Taveira wrote:
> > A proposed fix is attached. It should be applied to 9.4, 9.5, 9.6, and
> > 10. (Although, client version 10 can connect to server version 10,
> > client version 10 can't connect to server version 9.6.)
> > 
> > Comments?
> 
> The exact same fix has already applied on all stable branches:
> - af5fbb1286 -> REL9_4_STABLE
> - 24ff0fe877 -> REL9_5_STABLE
> - 59743deca9 -> REL9_6_STABLE
> - e7d3a37d99 -> REL_10_STABLE
> - 8d2814f274 -> master

That change is testing the wrong variable.  I plan to repair it as attached.
I ran check-world with the following and found no similar defects:

--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -6106,4 +6106,5 @@ int
 PQserverVersion(const PGconn *conn)
 {
+	Assert(conn);
 	if (!conn)
 		return 0;

  5. Re: pg_recvlogical broken in back branches

    Michael Paquier <michael@paquier.xyz> — 2018-04-22T22:49:05Z

    On Sun, Apr 22, 2018 at 02:55:51PM -0700, Noah Misch wrote:
> That change is testing the wrong variable.  I plan to repair it as
> attached.

You are right here.  Ditto

The whole GetConnection() business in src/bin/pg_basebackup is confusing
with the presence of a global variable.  I am wondering if we should
change GetConnection() to SetConnection().  Most of the routines of
streamutil.c also use "conn" as a local variable while being defined as
a global variable in streamutil.h, which don't help much.
--
Michael

  6. Re: pg_recvlogical broken in back branches

    Magnus Hagander <magnus@hagander.net> — 2018-04-25T13:57:49Z

    On Mon, Apr 23, 2018 at 12:49 AM, Michael Paquier <michael@paquier.xyz>
wrote:

> On Sun, Apr 22, 2018 at 02:55:51PM -0700, Noah Misch wrote:
> > That change is testing the wrong variable.  I plan to repair it as
> > attached.
>
> You are right here.  Ditto
>

Ouch indeed. +1 for the fix.

Noah, you are planning to push it yourself, right?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  7. Re: pg_recvlogical broken in back branches

    Noah Misch <noah@leadboat.com> — 2018-04-26T01:53:38Z

    On Wed, Apr 25, 2018 at 03:57:49PM +0200, Magnus Hagander wrote:
> On Mon, Apr 23, 2018 at 12:49 AM, Michael Paquier <michael@paquier.xyz> wrote:
> > On Sun, Apr 22, 2018 at 02:55:51PM -0700, Noah Misch wrote:
> > > That change is testing the wrong variable.  I plan to repair it as
> > > attached.
> >
> > You are right here.  Ditto
> >
> 
> Ouch indeed. +1 for the fix.
> 
> Noah, you are planning to push it yourself, right?

Right.  Done.

  8. Re: pg_recvlogical broken in back branches

    Michael Paquier <michael@paquier.xyz> — 2018-04-26T02:20:34Z

    On Wed, Apr 25, 2018 at 06:53:38PM -0700, Noah Misch wrote:
> Right.  Done.

Thanks, Noah.
--
Michael