cli: ignore trailing data after the last frame, and report corrupted archives

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.

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

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 

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

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

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 

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