From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Christoph Berg <myon(at)debian(dot)org> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: pg17.3 PQescapeIdentifier() ignores len |
Date: | 2025-02-15 16:16:52 |
Message-ID: | fvwrbxnsqnihwcrsrxkcl2qobnrua7syxnupnqpt6jevzn77g5@ozgmpn7dicdn |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2025-02-15 13:33:54 +0100, Christoph Berg wrote:
> Re: Andres Freund
> > > > The fprintf suggests that since 5dc1e42b4 PQescapeIdentifier ignores its len.
> > >
> > > Ugh, yes. Need something like the attached.
> >
> > I just pushed this fix, together with an expansion of test_escape.c. With the
> > expanded test both uses of strlen() are detected.
>
> FTR, this is also caught by pygresql's regression tests:
>
> test_inserttable_with_dotted_table_name (tests.test_classic_connection.TestInserttable.test_inserttable_with_dotted_table_name) ... ERROR
>
> https://ci.debian.net/packages/p/pygresql/testing/amd64/57838998/
> https://qa.debian.org/excuses.php?package=postgresql-17
>
> What's missing in the PG regression tests to see that problem?
Well, the expanded tests added as part of the fix would catch it, but I agree,
it's a problem this wasn't caught beforehand.
I don't think that common uses of PQescapeIdentifier/Literal are likely to
catch the problem, so it's perhaps not too surprising it wasn't caught. Which,
I guess, shows that we really need more explicit edge-case coverage of at
least the most crucial APIs (we barely have any). There's pretty much no way
that pg_regress or TAP test style tests are going to catch a problem like
this.
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Christoph Berg | 2025-02-15 16:55:12 | Re: pg17.3 PQescapeIdentifier() ignores len |
Previous Message | Andrew Dunstan | 2025-02-15 13:10:39 | Re: Decision by Monday: PQescapeString() vs. encoding violation |