Re: libpq: Fix wrong connection status on invalid "connect_timeout"

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Lars Kanis <lars(at)greiz-reinsdorf(dot)de>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: libpq: Fix wrong connection status on invalid "connect_timeout"
Date: 2019-10-21 02:40:20
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Oct 18, 2019 at 02:01:23PM +0200, Lars Kanis wrote:
> Am 18.10.19 um 05:06 schrieb Michael Paquier:
>> So attached is a patch to skip trailing whitespaces as well,
>> which also fixes the issue with ECPG. I have refactored the parsing
>> logic a bit while on it. The comment at the top of parse_int_param()
>> needs to be reworked a bit more.
> I tested this and it looks good to me. Maybe you could omit some
> redundant 'end' checks, as in the attached patch. Or was your intention
> to verify non-NULL 'end'?

Yes. Here are the connection patterns I have tested. These now pass:
'postgresql:///postgres?host=/tmp&port=5432 &user=postgres'
'postgresql:///postgres?host=/tmp&port= 5432&user=postgres'
And these fail (overflow on third one):
'postgresql:///postgres?host=/tmp&port=5432 s &user=postgres'
'postgresql:///postgres?host=/tmp&port= s 5432&user=postgres'
'postgresql:///postgres?host=/tmp&port= 5000000000&user=postgres'

Before the patch any trailing characters caused a failures even if
there were just whitespaces as trailing characters (first case

>> Perhaps we could add directly regression
>> tests for libpq. I'll start a new thread about that once we are done
>> here, the topic is larger.
> We have around 650 tests on ruby-pg to ensure everything runs as
> expected and I always wondered how the API of libpq is being verified.

For advanced test scenarios like connection handling, we use perl's
TAP tests. The situation regarding libpq-related testing is a bit
messy though. We have some tests in src/test/recovery/ for a couple
of things, and we should have more things to stress anything related
to the protocol (say message injection, etc.).

I'll try to start a new thread about that with a patch adding some
basics for discussion.

I have applied the parsing fix and your fix as two separate commits as
these are at the end two separate bugs, then back-patched down to v12.
Ed has been credited for the report, and I have marked the author as
you, Lars. Thanks!

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Abelard Hoffman 2019-10-21 02:58:33 Re: jsonb_set() strictness considered harmful to data
Previous Message 李杰 (慎追) 2019-10-21 02:36:04 回复:Bug about drop index concurrently