Re: [BUG FIX]Connection fails with whitespace after keepalives parameter value

The patch looks good to me.

The URI parsing process has become more stringent, allowing for accurate
handling of leading and trailing whitespace. Additionally, a feature has
been implemented to detect errors when extraneous data is present at the
end of the URI. The proper functioning of these changes has been verified
through newly added test cases. I've also tested by removing the whitespace
handling code from ECPGConnect() and applying your patch to libpq, which
worked successfully.

________________________________
差出人: Michael Paquier
送信: 2024 年 10 月 3 日 (木曜日) 9:42
宛先: Tom Lane
Cc: Fujii Masao; Sasaki, Yuto/佐佐木 悠人; pgsql-hackers(at)lists(dot)postgresql(dot)org
件名: Re: [BUG FIX]Connection fails with whitespace after keepalives parameter value

On Wed, Oct 02, 2024 at 05:39:31PM -0400, Tom Lane wrote:
> Interesting. This is unhappy about the space before a parameter name,
> not the space after a parameter value, so it's a different issue.

conninfo_uri_parse_options() parses the URI as a set of option/values,
where conninfo_uri_parse_params. If we were to be careful about
trailing and leading whitespaces for the parameter names, we need to
be careful about the special JDBC cases for "ssl" and "requiressl",
meaning that we should add more logic in conninfo_uri_decode() to
discard these. That would apply a extra layer of sanity into the
values as well.

> But it's weird that ecpg takes it while libpq doesn't. Could libecpg
> be modifying/reassembling the URI string? I didn't look.

ECPGconnect() has some custom logic to discard trailing and leading
spaces:
/* Skip spaces before keyword */
for (token1 = str; *token1 == ' '; token1++)
[...]
token1[e] = '\0'; //skips trailing spaces.

The argument for libpq where we could be consistent is appealing. How
about lifting things in libpq like the attached? I wouldn't backpatch
that, but we have tests for URIs and I didn't break anything.
--
Michael