Re: libpq should not look up all host addresses at once

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: libpq should not look up all host addresses at once
Date: 2018-08-11 10:18:48
Message-ID: alpine.DEB.2.21.1808111122220.1705@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Tom,

> [...]
>
> So I think what this code should do is (1) look up each hostname as it
> needs it, not all at once, and (2) proceed on to the next hostname
> if it gets a DNS lookup failure, not fail the whole connection attempt
> immediately. As attached.

A quick test, and very quick glance at the code.

"git apply" gives "error: patch with only garbage at line 3". Patch
applies with "patch -p1".

Patch compiles, global "make check" ok, although I'm unsure whether the
feature is actually tested somewhere. I think not:-(

As you noted in another message, a small doc update should be needed.

Patch works as expected: I tried with failing dns queries, multiple ips
some of which or all failing connection attempts...

About the behavior from psql point of view:

* if dns works, error messages are only printed if all attempts failed:

sh> ./psql "host=127.0.0.17,local2.coelho.net"
psql: could not connect to server: Connection refused
Is the server running on host "127.0.0.17" and accepting
TCP/IP connections on port 5432?
could not connect to server: Connection refused
Is the server running on host "local2.coelho.net" (127.0.0.3) and accepting
TCP/IP connections on port 5432?
could not connect to server: Connection refused
Is the server running on host "local2.coelho.net" (127.0.0.2) and accepting
TCP/IP connections on port 5432?

But nothing shows if one succeeds at some point. I understand that libpq
is doing its job, but I'm wondering whether this is the best behavior.

Maybe the user would like to know that attempts are made and are failing?
This would suggest some callback mecanism so that the client is informed
of in progress issues? Maybe this is for future work.

Maybe psql should show these as warnings?

* when the password is required, there is no way to know for which host/ip
it is:

sh> psql "host=127.0.0.17,local2.coelho.net,local.coelho.net"
Password for user fabien:

* once connected, \conninfo shows only a partial information:

psql> \conninfo
You are connected to database "postgres" as user "fabien" on host
"local3.coelho.net" at port "5432".

But local3 is 127.0.0.4 and 127.0.0.1, which one is it?

* Code

atoi("5432+1") == 5432, so the port syntax check is loose, really.

I'd consider wrapping some of the logic. I'd check the port first, then
move the host resolution stuff into a function.

I would be fine with backpatching, as the current behavior is not somehow
broken.

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2018-08-11 12:15:44 Re: libpq should not look up all host addresses at once
Previous Message Michael Paquier 2018-08-11 09:50:03 Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack