Merge pull request #2408 from senhuang42/seekable_hang_fix

Remove possibility of hanging when using seekable decompression

Makefile

@@ -121,6 +121,7 @@ man:
 contrib: lib
 	$(MAKE) -C contrib/pzstd all
 	$(MAKE) -C contrib/seekable_format/examples all
+	$(MAKE) -C contrib/seekable_format/tests test
 	$(MAKE) -C contrib/largeNbDicts all
 	cd contrib/single_file_libs/ ; ./build_decoder_test.sh
 	cd contrib/single_file_libs/ ; ./build_library_test.sh
@@ -139,6 +140,7 @@ clean:
 	$(Q)$(MAKE) -C contrib/gen_html $@ > $(VOID)
 	$(Q)$(MAKE) -C contrib/pzstd $@ > $(VOID)
 	$(Q)$(MAKE) -C contrib/seekable_format/examples $@ > $(VOID)
+	$(Q)$(MAKE) -C contrib/seekable_format/tests $@ > $(VOID)
 	$(Q)$(MAKE) -C contrib/largeNbDicts $@ > $(VOID)
 	$(Q)$(RM) zstd$(EXT) zstdmt$(EXT) tmp*
 	$(Q)$(RM) -r lz4

contrib/seekable_format/tests/.gitignore

@@ -0,0 +1 @@
+seekable_tests

contrib/seekable_format/tests/Makefile

@@ -0,0 +1,38 @@
+# ################################################################
+# Copyright (c) 2017-present, Facebook, Inc.
+# All rights reserved.
+#
+# This source code is licensed under both the BSD-style license (found in the
+# LICENSE file in the root directory of this source tree) and the GPLv2 (found
+# in the COPYING file in the root directory of this source tree).
+# ################################################################
+
+# This Makefile presumes libzstd is built, using `make` in / or /lib/
+
+ZSTDLIB_PATH = ../../../lib
+ZSTDLIB_NAME = libzstd.a
+ZSTDLIB = $(ZSTDLIB_PATH)/$(ZSTDLIB_NAME)
+
+CPPFLAGS += -I../ -I$(ZSTDLIB_PATH) -I$(ZSTDLIB_PATH)/common
+
+CFLAGS ?= -O3
+CFLAGS += -g
+
+SEEKABLE_OBJS = ../zstdseek_compress.c ../zstdseek_decompress.c $(ZSTDLIB)
+
+.PHONY: default clean test
+
+default: seekable_tests
+
+test: seekable_tests
+	./seekable_tests
+
+$(ZSTDLIB):
+	$(MAKE) -C $(ZSTDLIB_PATH) $(ZSTDLIB_NAME)
+
+seekable_tests : seekable_tests.c $(SEEKABLE_OBJS)
+
+clean:
+	@rm -f core *.o tmp* result* *.zst \
+		seekable_tests
+	@echo Cleaning completed

contrib/seekable_format/tests/seekable_tests.c

@@ -0,0 +1,63 @@
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+
+#include "zstd_seekable.h"
+
+/* Basic unit tests for zstd seekable format */
+int main(int argc, const char** argv)
+{
+    unsigned testNb = 1;
+    printf("Beginning zstd seekable format tests...\n");
+    printf("Test %u - check that seekable decompress does not hang: ", testNb++);
+    {   /* Github issue #2335 */
+        const size_t compressed_size = 17;
+        const uint8_t compressed_data[17] = {
+            '^',
+            '*',
+            'M',
+            '\x18',
+            '\t',
+            '\x00',
+            '\x00',
+            '\x00',
+            '\x00',
+            '\x00',
+            '\x00',
+            '\x00',
+            ';',
+            (uint8_t)('\xb1'),
+            (uint8_t)('\xea'),
+            (uint8_t)('\x92'),
+            (uint8_t)('\x8f'),
+        };
+        const size_t uncompressed_size = 32;
+        uint8_t uncompressed_data[32];
+
+        ZSTD_seekable* stream = ZSTD_seekable_create();
+        size_t status = ZSTD_seekable_initBuff(stream, compressed_data, compressed_size);
+        if (ZSTD_isError(status)) {
+            ZSTD_seekable_free(stream);
+            goto _test_error;
+        }
+
+        const size_t offset = 2;
+        /* Should return an error, but not hang */
+        status = ZSTD_seekable_decompress(stream, uncompressed_data, uncompressed_size, offset);
+        if (!ZSTD_isError(status)) {
+            ZSTD_seekable_free(stream);
+            goto _test_error;
+        }
+
+        ZSTD_seekable_free(stream);
+    }
+    printf("Success!\n");
+
+    /* TODO: Add more tests */
+    printf("Finished tests\n");
+    return 0;
+
+_test_error:
+    printf("test failed! Exiting..\n");
+    return 1;
+}

contrib/seekable_format/zstdseek_decompress.c

@@ -79,6 +79,8 @@
 #define MIN(a, b) ((a) < (b) ? (a) : (b))
 #define MAX(a, b) ((a) > (b) ? (a) : (b))
 
+#define ZSTD_SEEKABLE_NO_OUTPUT_PROGRESS_MAX 16
+
 /* Special-case callbacks for FILE* and in-memory modes, so that we can treat
  * them the same way as the advanced API */
 static int ZSTD_seekable_read_FILE(void* opaque, void* buffer, size_t n)
@@ -380,6 +382,7 @@ size_t ZSTD_seekable_initAdvanced(ZSTD_seekable* zs, ZSTD_seekable_customFile sr
 size_t ZSTD_seekable_decompress(ZSTD_seekable* zs, void* dst, size_t len, unsigned long long offset)
 {
     U32 targetFrame = ZSTD_seekable_offsetToFrameIndex(zs, offset);
+    U32 noOutputProgressCount = 0;
     do {
         /* check if we can continue from a previous decompress job */
         if (targetFrame != zs->curFrame || offset != zs->decompressedOffset) {
@@ -398,6 +401,7 @@ size_t ZSTD_seekable_decompress(ZSTD_seekable* zs, void* dst, size_t len, unsign
             size_t toRead;
             ZSTD_outBuffer outTmp;
             size_t prevOutPos;
+            size_t forwardProgress;
             if (zs->decompressedOffset < offset) {
                 /* dummy decompressions until we get to the target offset */
                 outTmp = (ZSTD_outBuffer){zs->outBuff, MIN(SEEKABLE_BUFF_SIZE, offset - zs->decompressedOffset), 0};
@@ -415,7 +419,15 @@ size_t ZSTD_seekable_decompress(ZSTD_seekable* zs, void* dst, size_t len, unsign
                 XXH64_update(&zs->xxhState, (BYTE*)outTmp.dst + prevOutPos,
                              outTmp.pos - prevOutPos);
             }
-            zs->decompressedOffset += outTmp.pos - prevOutPos;
+            forwardProgress = outTmp.pos - prevOutPos;
+            if (forwardProgress == 0) {
+                if (noOutputProgressCount++ > ZSTD_SEEKABLE_NO_OUTPUT_PROGRESS_MAX) {
+                    return ERROR(seekableIO);
+                }
+            } else {
+                noOutputProgressCount = 0;
+            }
+            zs->decompressedOffset += forwardProgress;
 
             if (toRead == 0) {
                 /* frame complete */