Hi Michael, Thanks for looking! I agree XLogRecordAssemble() never writes such a hole, CRC catches accidental corruption, and core WAL is trusted.
My only point was that DecodeXLogRecord() already distrusts these fields enough to cross-check them: it rejects hole_offset == 0, hole_length == 0, bimg_len == BLCKSZ when HAS_HOLE is set, and non-zero hole fields when it is not set. Bounding hole_offset + hole_length against BLCKSZ just completes that existing family of checks. Rahul's repro shows a re-CRCed record still passes decode and then crashes pg_waldump --save-fullpage. I agree the test in 0002 is quite large for such a small check -- happy to drop it entirely. To keep this minimal, we could keep just the two-line check in DecodeXLogRecord() with no extra test -- the existing HAS_HOLE error message, no new paths. I don't insist on the test or backpatch -- if you prefer, let's keep only the decode check, or close it if you think even that is not wanted. Should xlogreader be robust here, or may RestoreBlockImage() assume trusted input after CRC? I understand from your message that you lean towards this not being needed, given trusted WAL, CRC and the insertion bounds -- just wanted to understand where the line is. Happy to update or close as you suggest. Kind regards, Yuriy
