Re: Fix detection of truncated zstd-compressed backups

> On 14 Aug 2026, at 06:28, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:

Sorry for the long delay in responding.

> I just checked pg_dump/pg_restore. The problem exists only with zstd and lz4, gzip doesn't have the problem.
>
> Daniel’s PoC covers the custom-archive-format path, but not the directory-format path.

Thanks for expanding the fix, I ran out of time after realizing there was an
issue through my hack.

> For the directory-format path, we can reproduce the problem by simply truncating one byte from a compressed data file. For the custom-archive path, reproducing the problem is less straightforward because the compressed data is stored inside length-prefixed archive blocks. Simply truncating the file can make archive parsing fail before the decompressor sees the truncated frame. I created a repro script, see the attached shell script.

This reasoning should be added as a comment in the test file to aid future
readers.

> pg_restore: error: could not uncompress data: (null)

While not strictly related we should take the opportunity to fix this while in
here. Perhaps use "unkown error" in case zp->msg is null when erroring out?

> While testing, I also found a small issue in LZ4Stream_read_internal(). Its error branches call pg_log_error() and then return -1, but callers immediately call pg_fatal() when the return value <0. This results in duplicate error messages. So, I removed those pg_log_error() calls.

Makes sense. The gets function does however not exit with pg_fatal, do we need
any special handling there?

> See 0002 for the fix. I added tests only for zstd and lz4, since gzip is not changed.

A few comments on the patches:

+typedef enum
+{
+ ASTREAMER_STREAM_NEW,
+ ASTREAMER_FRAME_INCOMPLETE,
+ ASTREAMER_FRAME_COMPLETE,
+} astreamer_decompression_state;
This needs better commenting. Perhaps something along the lines of:

-/* State of the most recently processed compressed frame. */
+/*
+ * State of the most recently processed compressed frame. When decompression
+ * requires more input data to complete, or a bigger output buffer to store
+ * the result the state is set to ASTREAMER_FRAME_INCOMPLETE. Exactly how to
+ * resolve an _INCOMPLETE state is compression library dependent. Before a
+ * stream has decompressed any frames is has the state ASTREAMER_STREAM_NEW.
+ */

+ bool frame_finished;
Nitpick: I'm not a fan of using a local variable with the same name (and
function) as a struct member. Maybe also a comment explaining why we're not
pulling out an LZ4State from the private member?

+truncate_custom_compressed_data
This function needs comments to explain why it's necessary and what it's doing.

+ $pos = index($data, $magic);
+ die "compressed frame magic not found in $path" if $pos < 5;
+ $pos -= 5;
If the function takes the magic as a parameter it should not make any
assumptions about the length of the magic. This should either be inferred from
the parameter (best option IMHO) or passed in separately.

+SKIP:
+{
+ skip "zstd compression not supported by this build", 1 if !$supports_zstd;
Please add a comment explaining why this testcase isn't applicable to LZ4.

--
Daniel Gustafsson