zstd: Pass a maximum decompressed size for coredumps

[g2p]

This fixes handling of systemd coredumps, which don't necessarily include a decompressed size header.

Maximum size was picked at 512M from looking at existing core dumps.

Fixes https://bugs.launchpad.net/apport/+bug/2081708

[github-actions]

Everyone contributing to this PR have now signed the CLA. Thanks!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.02%. Comparing base (9c0b904) to head (00f2d13).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #397   +/-   ##
=======================================
  Coverage   83.01%   83.02%           
=======================================
  Files          99       99           
  Lines       20329    20337    +8     
  Branches     3206     3208    +2     
=======================================
+ Hits        16876    16884    +8     
  Misses       2933     2933           
  Partials      520      520           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bdrung]

Thanks. What happens when the coredump size is bigger than 512 MB? Shouldn't we determine the size dynamically from the coredump file?

Are you willing to work on the patch (linter fix, add test case, etc) or is it a drive-by fire-and-forget patch that we should polish?

[g2p]

Decompression is done in memory, so maybe it is better to keep a length limit.

I've reformatted with black and added a test case. It is kind of a drive-by otherwise, if it needs more significant reworking.

[bdrung]

Thanks. The merge request looks good. There is one question to ask before merging: What will happen when the core-dump size is bigger than COREDUMP_MAX_SIZE?

[bdrung]

The failing codecov/project can be ignored. That is caused by fluctuating coverage (the previous run had some failure code path covered).

[g2p]

It will blow up, as it did in some cases before the patch (systemd coredump on oracular).

The problem is that callers aren't expecting errors, for example when going through __len__ and get_complete_size.

So a more complete fix could involve validation, silently truncating the core dump (easy enough with another parameter to zstandard decompress), or skipping it if it's too large.

[schopin-pro]

I'm not asking you to implement this, but I think the best path forward would be to either move to on-disk decompression (e.g. in /var/tmp or $XDG_CACHE ?) or just skip the crash if it's too big. While they're still valuable in some use cases, truncated dumps are useless for us.

[bdrung]

I am reconsidering my standpoint. This change will be a step in the right direction. It will fix the crash for dumps that are smaller than 512 MiB, but still fail for bigger ones. This change does not introduce regressions. If the compressed data contains the decompressed size header, it will ignore the max_output_size parameter. I tested this on noble with zstd NEWS.md:

compressed_news = pathlib.Path("NEWS.md.zst").read_bytes()
assert len(compressed_news) > 1024
decompressed = zstandard.ZstdDecompressor().decompress(compressed_news, max_output_size=1024)
assert len(decompressed) > len(compressed_news)

I will work on __len__ to not use get_value to avoid needing to keep all the data in memory.

[bdrung]

This is a different approach to fix only the __len__: #423