diff --git a/programs/fileio.c b/programs/fileio.c index 78a906d533b..0d45b9915a6 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -517,6 +517,7 @@ struct FIO_ctx_s { int nbFilesProcessed; size_t totalBytesInput; size_t totalBytesOutput; + int trailingDataIgnored; /* set when the current input had trailing data after its last frame */ }; static int FIO_shouldDisplayFileSummary(FIO_ctx_t const* fCtx) @@ -2684,6 +2685,27 @@ FIO_zstdErrorHelp(const FIO_prefs_t* const prefs, srcFileName, ZSTD_WINDOWLOG_MAX); } +/* FIO_isCorruptionError() : + * @return : 1 if the decoding error says that the archive is damaged, + * 0 for errors that do not (memory limit, dictionary mismatch, ...) */ +static int FIO_isCorruptionError(size_t const err) +{ + ZSTD_ErrorCode const code = ZSTD_getErrorCode(err); + return (code == ZSTD_error_corruption_detected) + || (code == ZSTD_error_checksum_wrong) + || (code == ZSTD_error_literals_headerWrong) + || (code == ZSTD_error_srcSize_wrong); +} + +/* FIO_displayCorruptedArchive() : + * summary line for a damaged or cut short archive, + * so that the user knows how much of the data was decoded before the failure */ +static void FIO_displayCorruptedArchive(const char* srcFileName, U64 nbDecoded) +{ + DISPLAYLEVEL(1, "zstd: %s: corrupted or incomplete archive; decoded %llu bytes \n", + srcFileName, (unsigned long long)nbDecoded); +} + /** FIO_decompressFrame() : * @return : size of decoded zstd frame, or an error code */ @@ -2720,6 +2742,8 @@ FIO_decompressZstdFrame(FIO_ctx_t* const fCtx, dRess_t* ress, DISPLAYLEVEL(1, "%s : Decoding error (36) : %s \n", srcFileName, ZSTD_getErrorName(readSizeHint)); FIO_zstdErrorHelp(prefs, ress, readSizeHint, srcFileName); + if (FIO_isCorruptionError(readSizeHint)) + FIO_displayCorruptedArchive(srcFileName, alreadyDecoded + frameSize); AIO_WritePool_releaseIoJob(writeJob); return FIO_ERROR_FRAME_DECODING; } @@ -2748,6 +2772,7 @@ FIO_decompressZstdFrame(FIO_ctx_t* const fCtx, dRess_t* ress, if (readSize==0) { DISPLAYLEVEL(1, "%s : Read error (39) : premature end \n", srcFileName); + FIO_displayCorruptedArchive(srcFileName, alreadyDecoded + frameSize); AIO_WritePool_releaseIoJob(writeJob); return FIO_ERROR_FRAME_DECODING; } @@ -2997,6 +3022,9 @@ static int FIO_decompressFrames(FIO_ctx_t* const fCtx, for ( ; ; ) { /* check magic number -> version */ size_t const toRead = 4; + /* readSomething is only set by a previous iteration, which then decoded a frame successfully + * (any failure returns). Data that is not a frame after that is trailing data. */ + unsigned const afterFrame = readSomething; const BYTE* buf; AIO_ReadPool_fillBuffer(ress.readCtx, toRead); buf = (const BYTE*)ress.readCtx->srcBuffer; @@ -3012,6 +3040,11 @@ static int FIO_decompressFrames(FIO_ctx_t* const fCtx, if (passThrough) { return FIO_passThrough(&ress); } + if (afterFrame) { + DISPLAYLEVEL(1, "zstd: WARNING: %s: trailing data after the last frame is ignored \n", srcFileName); + fCtx->trailingDataIgnored = 1; + break; + } DISPLAYLEVEL(1, "zstd: %s: unknown header \n", srcFileName); return 1; } @@ -3049,6 +3082,11 @@ static int FIO_decompressFrames(FIO_ctx_t* const fCtx, #endif } else if (passThrough) { return FIO_passThrough(&ress); + } else if (afterFrame) { + /* level 1, not 2: writing to stdout lowers the display level to 1, and the user must still be told */ + DISPLAYLEVEL(1, "zstd: WARNING: %s: trailing data after the last frame is ignored \n", srcFileName); + fCtx->trailingDataIgnored = 1; + break; } else { DISPLAYLEVEL(1, "zstd: %s: unsupported format \n", srcFileName); return 1; @@ -3167,6 +3205,7 @@ static int FIO_decompressSrcFile(FIO_ctx_t* const fCtx, FIO_prefs_t* const prefs AIO_ReadPool_setFile(ress.readCtx, srcFile); + fCtx->trailingDataIgnored = 0; result = FIO_decompressDstFile(fCtx, prefs, ress, dstFileName, srcFileName, &srcFileStat); AIO_ReadPool_setFile(ress.readCtx, NULL); @@ -3178,6 +3217,7 @@ static int FIO_decompressSrcFile(FIO_ctx_t* const fCtx, FIO_prefs_t* const prefs } if ( prefs->removeSrcFile /* --rm */ && (result==0) /* decompression successful */ + && !fCtx->trailingDataIgnored /* the input was not entirely decoded */ && strcmp(srcFileName, stdinmark) ) /* not stdin */ { /* We must clear the handler, since after this point calling it would * delete both the source and destination files. diff --git a/tests/cli-tests/decompression/corrupted-archive.sh b/tests/cli-tests/decompression/corrupted-archive.sh new file mode 100755 index 00000000000..e66aeeaa2cc --- /dev/null +++ b/tests/cli-tests/decompression/corrupted-archive.sh @@ -0,0 +1,28 @@ +#!/bin/sh + +set -e + +. "$COMMON/platform.sh" + +echo "some data" > frame1 +echo "more data" > frame2 +cat frame1 frame2 > both +zstd -q frame1 frame2 +cat frame1.zst frame2.zst > both.zst +SIZE=$(wc -c < both.zst) + +println "+ a frame that is cut short reports how much was decoded" + +# the last frame loses most of its checksum +head -c $((SIZE - 3)) both.zst > cut.zst +zstd -dc cut.zst > cut.out && die "should fail" +zstd -t cut.zst && die "should fail" +$DIFF both cut.out + +println "+ a corrupted frame reports how much was decoded" + +# the last byte of the content of the last frame is replaced +{ head -c $((SIZE - 5)) both.zst; printf 'X'; tail -c 4 both.zst; } > corrupt.zst +zstd -dc corrupt.zst > corrupt.out && die "should fail" +zstd -t corrupt.zst && die "should fail" +$DIFF frame1 corrupt.out diff --git a/tests/cli-tests/decompression/corrupted-archive.sh.stderr.exact b/tests/cli-tests/decompression/corrupted-archive.sh.stderr.exact new file mode 100644 index 00000000000..e632bda0158 --- /dev/null +++ b/tests/cli-tests/decompression/corrupted-archive.sh.stderr.exact @@ -0,0 +1,8 @@ +cut.zst : Read error (39) : premature end +zstd: cut.zst: corrupted or incomplete archive; decoded 20 bytes +cut.zst : Read error (39) : premature end +zstd: cut.zst: corrupted or incomplete archive; decoded 20 bytes +corrupt.zst : Decoding error (36) : Restored data doesn't match checksum +zstd: corrupt.zst: corrupted or incomplete archive; decoded 10 bytes +corrupt.zst : Decoding error (36) : Restored data doesn't match checksum +zstd: corrupt.zst: corrupted or incomplete archive; decoded 10 bytes diff --git a/tests/cli-tests/decompression/corrupted-archive.sh.stdout.exact b/tests/cli-tests/decompression/corrupted-archive.sh.stdout.exact new file mode 100644 index 00000000000..8ec5a5e1fa4 --- /dev/null +++ b/tests/cli-tests/decompression/corrupted-archive.sh.stdout.exact @@ -0,0 +1,2 @@ ++ a frame that is cut short reports how much was decoded ++ a corrupted frame reports how much was decoded diff --git a/tests/cli-tests/decompression/trailing-garbage.sh b/tests/cli-tests/decompression/trailing-garbage.sh new file mode 100755 index 00000000000..958727359a0 --- /dev/null +++ b/tests/cli-tests/decompression/trailing-garbage.sh @@ -0,0 +1,67 @@ +#!/bin/sh + +set -e + +. "$COMMON/platform.sh" + +echo "some data" > frame1 +echo "more data" > frame2 +zstd -q frame1 frame2 +cat frame1 frame2 > both + +println "+ input that never contained a frame keeps its errors" + +printf 'garbage' > notzstd +zstd -dc notzstd > /dev/null && die "should fail" +printf 'ab' > short +zstd -dc short > /dev/null && die "should fail" + +println "+ trailing data after the last frame is ignored with a warning" + +# 7 bytes: enough to check a magic number, which no known format matches +cp frame1.zst junk7.zst +printf 'garbage' >> junk7.zst +zstd -dc junk7.zst > junk7.out +$DIFF frame1 junk7.out +zstd -dc < junk7.zst > junk7-stdin.out +$DIFF frame1 junk7-stdin.out +zstd -t junk7.zst +zstd -d -f junk7.zst +$DIFF frame1 junk7 + +# 2 bytes: too short to check a magic number +cp frame1.zst junk2.zst +printf 'ab' >> junk2.zst +zstd -dc junk2.zst > junk2.out +$DIFF frame1 junk2.out + +# several frames, then trailing data +cat frame1.zst frame2.zst > two-frames.zst +printf 'garbage' >> two-frames.zst +zstd -dc two-frames.zst > two-frames.out +$DIFF both two-frames.out + +println "+ -qq suppresses the warning" + +zstd -qq -dc junk7.zst > junk7-quiet.out +$DIFF frame1 junk7-quiet.out + +println "+ pass-through still copies the trailing data" + +zstd -dcf junk7.zst > junk7-pass.out +cat frame1 > expected-pass +printf 'garbage' >> expected-pass +$DIFF expected-pass junk7-pass.out + +println "+ --rm keeps the source when trailing data was ignored" + +zstd -d -f --rm junk7.zst +test -f junk7.zst +$DIFF frame1 junk7 +# a clean archive is still removed +cp frame1.zst clean.zst +zstd -d -f --rm clean.zst +test ! -f clean.zst +# writing to stdout never removes the source +zstd -dc --rm frame2.zst > /dev/null +test -f frame2.zst diff --git a/tests/cli-tests/decompression/trailing-garbage.sh.stderr.exact b/tests/cli-tests/decompression/trailing-garbage.sh.stderr.exact new file mode 100644 index 00000000000..4fd7337979e --- /dev/null +++ b/tests/cli-tests/decompression/trailing-garbage.sh.stderr.exact @@ -0,0 +1,13 @@ +zstd: notzstd: unsupported format +zstd: short: unknown header +zstd: WARNING: junk7.zst: trailing data after the last frame is ignored +zstd: WARNING: /*stdin*\: trailing data after the last frame is ignored +zstd: WARNING: junk7.zst: trailing data after the last frame is ignored +junk7.zst : 10 bytes +zstd: WARNING: junk7.zst: trailing data after the last frame is ignored +junk7.zst : 10 bytes +zstd: WARNING: junk2.zst: trailing data after the last frame is ignored +zstd: WARNING: two-frames.zst: trailing data after the last frame is ignored +zstd: WARNING: junk7.zst: trailing data after the last frame is ignored +junk7.zst : 10 bytes +clean.zst : 10 bytes diff --git a/tests/cli-tests/decompression/trailing-garbage.sh.stdout.exact b/tests/cli-tests/decompression/trailing-garbage.sh.stdout.exact new file mode 100644 index 00000000000..c867cf029e5 --- /dev/null +++ b/tests/cli-tests/decompression/trailing-garbage.sh.stdout.exact @@ -0,0 +1,5 @@ ++ input that never contained a frame keeps its errors ++ trailing data after the last frame is ignored with a warning ++ -qq suppresses the warning ++ pass-through still copies the trailing data ++ --rm keeps the source when trailing data was ignored