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
listed).
>> 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!
--
Michael