From 3aa2b27a890e9aa4c64b9e52632a6ef4c1418041 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Dec 2017 02:52:50 -0500 Subject: [PATCH 1/4] fix #942 : streaming interface does not compress after ZSTD_initCStream() While the final result is still, technically, a frame, the resulting frame expands initial data instead of compressing it. This is because the streaming API creates a tiny 1-byte buffer for input, because it believes input is empty (0-bytes), because in the past, 0 used to mean "unknown" instead. This patch fixes the issue. Todo : add a test which traps the issue. --- lib/compress/zstd_compress.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 8d493852f..69b0d6402 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2466,6 +2466,7 @@ size_t ZSTD_compress_usingCDict(ZSTD_CCtx* cctx, ZSTD_CStream* ZSTD_createCStream(void) { + DEBUGLOG(3, "ZSTD_createCStream"); return ZSTD_createCStream_advanced(ZSTD_defaultCMem); } @@ -2633,7 +2634,8 @@ size_t ZSTD_initCStream_srcSize(ZSTD_CStream* zcs, int compressionLevel, unsigne size_t ZSTD_initCStream(ZSTD_CStream* zcs, int compressionLevel) { - return ZSTD_initCStream_srcSize(zcs, compressionLevel, 0); + DEBUGLOG(4, "ZSTD_initCStream"); + return ZSTD_initCStream_srcSize(zcs, compressionLevel, ZSTD_CONTENTSIZE_UNKNOWN); } /*====== Compression ======*/ From 21f7672d43549e1f87b879fb8a14ac1a4fabc266 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Dec 2017 03:06:01 -0500 Subject: [PATCH 2/4] zstreamtest : add test which detects when ZSTD_initCStream() inflate data as in #942, when ZSTD_initCStream() believes data is empty, instead of unknown. --- tests/zstreamtest.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 920e988b0..e08a96d8f 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -245,13 +245,28 @@ static int basicUnitTests(U32 seed, double compressibility) } dictID = ZDICT_getDictID(dictionary.start, dictionary.filled); + /* Basic compression test */ + DISPLAYLEVEL(3, "test%3i : compress %u bytes : ", testNb++, COMPRESSIBLE_NOISE_LENGTH); + CHECK_Z( ZSTD_initCStream(zc, 1 /* cLevel */) ); + outBuff.dst = (char*)(compressedBuffer); + outBuff.size = compressedBufferSize; + outBuff.pos = 0; + inBuff.src = CNBuffer; + inBuff.size = CNBufferSize; + inBuff.pos = 0; + CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) ); + if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */ + { size_t const r = ZSTD_endStream(zc, &outBuff); + if (r != 0) goto _output_error; } /* error, or some data not flushed */ + DISPLAYLEVEL(3, "OK (%u bytes)\n", (U32)outBuff.pos); + /* generate skippable frame */ MEM_writeLE32(compressedBuffer, ZSTD_MAGIC_SKIPPABLE_START); MEM_writeLE32(((char*)compressedBuffer)+4, (U32)skippableFrameSize); cSize = skippableFrameSize + 8; - /* Basic compression test */ - DISPLAYLEVEL(3, "test%3i : compress %u bytes : ", testNb++, COMPRESSIBLE_NOISE_LENGTH); + /* Basic compression test using dict */ + DISPLAYLEVEL(3, "test%3i : skipframe + compress %u bytes : ", testNb++, COMPRESSIBLE_NOISE_LENGTH); CHECK_Z( ZSTD_initCStream_usingDict(zc, CNBuffer, dictSize, 1 /* cLevel */) ); outBuff.dst = (char*)(compressedBuffer)+cSize; assert(compressedBufferSize > cSize); From c029ee1f0bc464754c899deb2f5f0a2f16f895e4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Dec 2017 17:13:10 -0500 Subject: [PATCH 3/4] ZSTD_initCStream_srcSize() considers "0" to mean "unknown" to not break existing programs relying on this behavior. Might be changed to mean "empty" in the future. --- lib/compress/zstd_compress.c | 7 +++---- lib/zstd.h | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 69b0d6402..20b095998 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2623,12 +2623,11 @@ size_t ZSTD_initCStream_usingDict(ZSTD_CStream* zcs, const void* dict, size_t di return ZSTD_initCStream_internal(zcs, dict, dictSize, NULL, cctxParams, ZSTD_CONTENTSIZE_UNKNOWN); } -size_t ZSTD_initCStream_srcSize(ZSTD_CStream* zcs, int compressionLevel, unsigned long long pledgedSrcSize) +size_t ZSTD_initCStream_srcSize(ZSTD_CStream* zcs, int compressionLevel, unsigned long long pss) { - ZSTD_CCtx_params cctxParams; + U64 const pledgedSrcSize = (pss==0) ? ZSTD_CONTENTSIZE_UNKNOWN : pss; /* temporary : 0 interpreted as "unknown" during transition period. Users willing to specify "unknown" **must** use ZSTD_CONTENTSIZE_UNKNOWN. `0` will be interpreted as "empty" in the future */ ZSTD_parameters const params = ZSTD_getParams(compressionLevel, pledgedSrcSize, 0); - cctxParams = ZSTD_assignParamsToCCtxParams(zcs->requestedParams, params); - cctxParams.fParams.contentSizeFlag = (pledgedSrcSize>0); + ZSTD_CCtx_params const cctxParams = ZSTD_assignParamsToCCtxParams(zcs->requestedParams, params); return ZSTD_initCStream_internal(zcs, NULL, 0, NULL, cctxParams, pledgedSrcSize); } diff --git a/lib/zstd.h b/lib/zstd.h index fe37cec8b..76d75e2a0 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -734,7 +734,7 @@ ZSTDLIB_API unsigned ZSTD_getDictID_fromFrame(const void* src, size_t srcSize); /*===== Advanced Streaming compression functions =====*/ ZSTDLIB_API ZSTD_CStream* ZSTD_createCStream_advanced(ZSTD_customMem customMem); ZSTDLIB_API ZSTD_CStream* ZSTD_initStaticCStream(void* workspace, size_t workspaceSize); /**< same as ZSTD_initStaticCCtx() */ -ZSTDLIB_API size_t ZSTD_initCStream_srcSize(ZSTD_CStream* zcs, int compressionLevel, unsigned long long pledgedSrcSize); /**< pledgedSrcSize must be correct. If it is not known at init time, use ZSTD_CONTENTSIZE_UNKNOWN. Note that, for compatibility with older programs, a size of 0 is interepreted as "unknown". But it may change in some future version to mean "empty". */ +ZSTDLIB_API size_t ZSTD_initCStream_srcSize(ZSTD_CStream* zcs, int compressionLevel, unsigned long long pledgedSrcSize); /**< pledgedSrcSize must be correct. If it is not known at init time, use ZSTD_CONTENTSIZE_UNKNOWN. Note that, for compatibility with older programs, "0" also disables frame content size field. It may be enabled in the future. */ ZSTDLIB_API size_t ZSTD_initCStream_usingDict(ZSTD_CStream* zcs, const void* dict, size_t dictSize, int compressionLevel); /**< creates of an internal CDict (incompatible with static CCtx), except if dict == NULL or dictSize < 8, in which case no dict is used. Note: dict is loaded with ZSTD_dm_auto (treated as a full zstd dictionary if it begins with ZSTD_MAGIC_DICTIONARY, else as raw content) and ZSTD_dlm_byCopy.*/ ZSTDLIB_API size_t ZSTD_initCStream_advanced(ZSTD_CStream* zcs, const void* dict, size_t dictSize, ZSTD_parameters params, unsigned long long pledgedSrcSize); /**< pledgedSrcSize : if srcSize is not known at init time, use value ZSTD_CONTENTSIZE_UNKNOWN. dict is loaded with ZSTD_dm_auto and ZSTD_dlm_byCopy. */ From dfc697e967ad736060dc9ec695a7db0d9a805fe3 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 8 Dec 2017 12:16:49 -0500 Subject: [PATCH 4/4] comment clarification --- lib/compress/zstd_compress.c | 8 ++++---- lib/zstd.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 20b095998..3f39875f5 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2599,7 +2599,8 @@ size_t ZSTD_initCStream_usingCDict(ZSTD_CStream* zcs, const ZSTD_CDict* cdict) } /* ZSTD_initCStream_advanced() : - * pledgedSrcSize : if srcSize is not known at init time, use value ZSTD_CONTENTSIZE_UNKNOWN. + * pledgedSrcSize must be correct. + * if srcSize is not known at init time, use value ZSTD_CONTENTSIZE_UNKNOWN. * dict is loaded with default parameters ZSTD_dm_auto and ZSTD_dlm_byCopy. */ size_t ZSTD_initCStream_advanced(ZSTD_CStream* zcs, const void* dict, size_t dictSize, @@ -2610,9 +2611,8 @@ size_t ZSTD_initCStream_advanced(ZSTD_CStream* zcs, DEBUGLOG(4, "ZSTD_initCStream_advanced: pledgedSrcSize=%u, flag=%u", (U32)pledgedSrcSize, params.fParams.contentSizeFlag); CHECK_F( ZSTD_checkCParams(params.cParams) ); - if ((pledgedSrcSize==0) && (params.fParams.contentSizeFlag==0)) - pledgedSrcSize = ZSTD_CONTENTSIZE_UNKNOWN; - return ZSTD_initCStream_internal(zcs, dict, dictSize, NULL, cctxParams, pledgedSrcSize); + if ((pledgedSrcSize==0) && (params.fParams.contentSizeFlag==0)) pledgedSrcSize = ZSTD_CONTENTSIZE_UNKNOWN; /* for compatibility with older programs relying on this behavior. Users should now specify ZSTD_CONTENTSIZE_UNKNOWN. This line will be removed in the future. */ + return ZSTD_initCStream_internal(zcs, dict, dictSize, NULL /*cdict*/, cctxParams, pledgedSrcSize); } size_t ZSTD_initCStream_usingDict(ZSTD_CStream* zcs, const void* dict, size_t dictSize, int compressionLevel) diff --git a/lib/zstd.h b/lib/zstd.h index 76d75e2a0..15c467535 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -737,9 +737,9 @@ ZSTDLIB_API ZSTD_CStream* ZSTD_initStaticCStream(void* workspace, size_t workspa ZSTDLIB_API size_t ZSTD_initCStream_srcSize(ZSTD_CStream* zcs, int compressionLevel, unsigned long long pledgedSrcSize); /**< pledgedSrcSize must be correct. If it is not known at init time, use ZSTD_CONTENTSIZE_UNKNOWN. Note that, for compatibility with older programs, "0" also disables frame content size field. It may be enabled in the future. */ ZSTDLIB_API size_t ZSTD_initCStream_usingDict(ZSTD_CStream* zcs, const void* dict, size_t dictSize, int compressionLevel); /**< creates of an internal CDict (incompatible with static CCtx), except if dict == NULL or dictSize < 8, in which case no dict is used. Note: dict is loaded with ZSTD_dm_auto (treated as a full zstd dictionary if it begins with ZSTD_MAGIC_DICTIONARY, else as raw content) and ZSTD_dlm_byCopy.*/ ZSTDLIB_API size_t ZSTD_initCStream_advanced(ZSTD_CStream* zcs, const void* dict, size_t dictSize, - ZSTD_parameters params, unsigned long long pledgedSrcSize); /**< pledgedSrcSize : if srcSize is not known at init time, use value ZSTD_CONTENTSIZE_UNKNOWN. dict is loaded with ZSTD_dm_auto and ZSTD_dlm_byCopy. */ + ZSTD_parameters params, unsigned long long pledgedSrcSize); /**< pledgedSrcSize must be correct. If srcSize is not known at init time, use value ZSTD_CONTENTSIZE_UNKNOWN. dict is loaded with ZSTD_dm_auto and ZSTD_dlm_byCopy. */ ZSTDLIB_API size_t ZSTD_initCStream_usingCDict(ZSTD_CStream* zcs, const ZSTD_CDict* cdict); /**< note : cdict will just be referenced, and must outlive compression session */ -ZSTDLIB_API size_t ZSTD_initCStream_usingCDict_advanced(ZSTD_CStream* zcs, const ZSTD_CDict* cdict, ZSTD_frameParameters fParams, unsigned long long pledgedSrcSize); /**< same as ZSTD_initCStream_usingCDict(), with control over frame parameters */ +ZSTDLIB_API size_t ZSTD_initCStream_usingCDict_advanced(ZSTD_CStream* zcs, const ZSTD_CDict* cdict, ZSTD_frameParameters fParams, unsigned long long pledgedSrcSize); /**< same as ZSTD_initCStream_usingCDict(), with control over frame parameters. pledgedSrcSize must be correct. If srcSize is not known at init time, use value ZSTD_CONTENTSIZE_UNKNOWN. */ /*! ZSTD_resetCStream() : * start a new compression job, using same parameters from previous job.