| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
| Cc: | Japin Li <japinli(at)hotmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Osama Abdul Qader <osamaabdulqader(dot)cs(at)gmail(dot)com> |
| Subject: | Re: Fix detection of truncated zstd-compressed backups |
| Date: | 2026-08-14 04:28:17 |
| Message-ID: | C558D480-3BDB-4A64-976D-DF47EB90E439@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Aug 14, 2026, at 06:59, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
>> On Aug 14, 2026, at 04:57, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>>
>>> On 13 Aug 2026, at 07:11, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>>> PFA v7: addressed Jipan’s comment.
>>
>> Sorry for being slow on this, things are quite busy but I hope to have a review
>> soon. While poking at this I realized that our compression code in pg_dump
>> likely has the same issue. I hacked up a quick PoC diff (attached) but it's
>> untested (can one actually test the data-in-zstd-internal-buffers case at all?)
>> and mainly a sketch. If you want to pick it up and rework into this patchset
>> to tackle it treewide then that would be fantastic.
>>
>> --
>> Daniel Gustafsson
>>
>> <pg_dump.diff.txt>
>
> I can work on this today.
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.
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.
Before the fix, the output contains:
```
custom zstd: exit status 0
directory zstd: exit status 0
custom lz4: exit status 0
directory lz4: exit status 0
pg_restore: error: could not uncompress data: (null)
custom gzip: exit status 1
pg_restore: error: could not close data file "/tmp/pgdump-trunc.1rxbf0/gzip-dir-bad/3931.dat": Undefined error: 0
directory gzip: exit status 1
```
This shows that gzip reports failure, while zstd and lz4 silently accept the truncated dump files.
After the fix, zstd and lz4 report failures as well:
```
pg_restore: error: could not decompress data: compressed stream is incomplete
custom zstd: exit status 1
pg_restore: error: could not decompress data: compressed stream is incomplete
directory zstd: exit status 1
pg_restore: error: could not decompress data: compressed stream is incomplete
custom lz4: exit status 1
pg_restore: error: could not read from input file: Input/output error
directory lz4: exit status 1
pg_restore: error: could not uncompress data: (null)
custom gzip: exit status 1
pg_restore: error: could not close data file "/tmp/pgdump-trunc.XjUiJ4/gzip-dir-bad/3931.dat": Undefined error: 0
directory gzip: exit status 1
```
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.
See 0002 for the fix. I added tests only for zstd and lz4, since gzip is not changed.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| nocfbot_test_pgdump_comp.sh | application/octet-stream | 2.7 KB |
| v8-0001-Fix-detection-of-truncated-compressed-backups.patch | application/octet-stream | 9.9 KB |
| v8-0002-Fix-detection-of-truncated-zstd-and-LZ4-dump-data.patch | application/octet-stream | 11.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bertrand Drouvot | 2026-08-14 04:40:56 | Re: Introduce XID age based replication slot invalidation |
| Previous Message | Michael Paquier | 2026-08-14 03:51:33 | Re: Tighten ACL check in repack_is_permitted_for_relation() |