| 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 |
| 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 |