Thread: bogus assert in logicalmsg_desc

Hi,

while experimenting with logical messages, I ran into this assert in
logicalmsg_desc:

    Assert(prefix[xlrec->prefix_size] != '\0');

This seems to be incorrect, because LogLogicalMessage does this:

    xlrec.prefix_size = strlen(prefix) + 1;

So prefix_size includes the null byte, so the assert points out at the
first payload byte. And of course, the check should be "==" because we
expect the byte to be \0, not the other way around.

It's pretty simple to make this crash by writing a logical message where
the first payload byte is \0, e.g. like this:

    select pg_logical_emit_message(true, 'a'::text, '\x00'::bytea);

and then running pg_waldump on the WAL segment.

Attached is a patch addressing this. This was added in 14, so we should
backpatch to that version.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Aug 15, 2022 at 1:17 AM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
> Hi,
>
> while experimenting with logical messages, I ran into this assert in
> logicalmsg_desc:
>
>     Assert(prefix[xlrec->prefix_size] != '\0');
>
> This seems to be incorrect, because LogLogicalMessage does this:
>
>     xlrec.prefix_size = strlen(prefix) + 1;
>
> So prefix_size includes the null byte, so the assert points out at the
> first payload byte. And of course, the check should be "==" because we
> expect the byte to be \0, not the other way around.
>
> It's pretty simple to make this crash by writing a logical message where
> the first payload byte is \0, e.g. like this:
>
>     select pg_logical_emit_message(true, 'a'::text, '\x00'::bytea);
>
> and then running pg_waldump on the WAL segment.
>
> Attached is a patch addressing this. This was added in 14, so we should
> backpatch to that version.

+1

The patch looks good to me.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/