Re: BUG #19598: pg_waldump: -s/-e accept out-of-range WAL locations and silently use the low 32 bits

From: Zexin Li <lizi(dot)openmind(at)gmail(dot)com>
To: masao(dot)fujii(at)gmail(dot)com
Cc: pgsql-bugs(at)lists(dot)postgresql(dot)org, malis(at)pgrust(dot)com, ayushtiwari(dot)slg01(at)gmail(dot)com
Subject: Re: BUG #19598: pg_waldump: -s/-e accept out-of-range WAL locations and silently use the low 32 bits
Date: 2026-08-14 06:33:11
Message-ID: CAAP6ZkS_3OH3yhhAGK6vu+2V1C2Hv4K6SpRuZL415R-gxjdTSg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Wed, Aug 12, 2026, Fujii Masao wrote:
> I've pushed the patch. Thanks!

Thank you for committing this!

Attached is the separate patch you suggested for the remaining
frontend sscanf("%X/%08X") call sites: the LSNs that pg_basebackup
and pg_rewind read from server responses, pg_rewind reads from
timeline history files, and pg_combinebackup reads from backup_label
files. The backend's copies of these parsers are left untouched,
and parse_manifest.c is already being handled by Ayush's patch.

* Two of the converted call sites read a location out of a longer
line, so the patch adds pg_parse_lsn_prefix(), which reports the
first character after the location instead of requiring the string
to end there, and reimplements pg_parse_lsn() on top of it, keeping
a single implementation of the syntax rules. Each tool keeps its
existing error message.

* Malformed metadata now fails with each tool's existing error
instead of silently proceeding with a different location: components
wider than eight hex digits used to wrap around or be truncated, and
whitespace, signs, "0x" prefixes, and trailing characters used to be
consumed or ignored. Two error-path details change: pg_rewind's
history-file parser now requires the switchpoint to be followed by
whitespace or end of line, where trailing characters used to be
ignored, and an overlong second component in a backup_label LSN now
fails pg_combinebackup's "could not parse" check rather than its
"improper terminator" check.

* The new TAP tests (corrupted timeline history files for pg_rewind,
a corrupted backup_label for pg_combinebackup) fail without the code
change and pass with it. I could not find a way to exercise the
server-response call sites with malformed input in a TAP test, so
they are covered by the existing suites only; make check-world
passes here on current master.

I'd appreciate any feedback .

Regards,
Zexin Li

On Wed, Aug 12, 2026 05:09 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

> On Tue, Aug 11, 2026 at 9:41 AM Zexin Li <lizi(dot)openmind(at)gmail(dot)com> wrote:
> >
> > On Sun, Aug 10, 2026, Fujii Masao wrote:
> > > The main change from v2 is that pg_lsn_in_safe() now also uses
> > > pg_parse_lsn(), leaving only the backend-specific soft-error handling
> > > there. This avoids duplicating the LSN syntax checks.
> >
> > Thank you for the review. v3 looks good to me.
>
> I've pushed the patch. Thanks!
>
> Regards,
>
> --
> Fujii Masao
>

Attachment Content-Type Size
v1-0001-Use-pg_parse_lsn-for-server-supplied-LSNs.patch application/octet-stream 19.1 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Andrey Rachitskiy 2026-08-14 06:44:46 Re: BUG #19616: pgoutput sends stream abort ('A') to clients that did not enable streaming
Previous Message Hayato Kuroda (Fujitsu) 2026-08-14 06:10:23 RE: BUG #19616: pgoutput sends stream abort ('A') to clients that did not enable streaming