From: Yann Collet Date: Mon, 19 Jun 2017 18:53:01 +0000 (-0700) Subject: new api : setting compression parameters is refused if a dictionary is already loaded X-Git-Tag: v1.3.0~1^2~17^2~27 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d7a3bffba909eb5c0ebe79d3f1e457c1ec7d2da9;p=thirdparty%2Fzstd.git new api : setting compression parameters is refused if a dictionary is already loaded --- diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index f848c088e..3620e13d8 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -253,11 +253,15 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, unsigned v case ZSTD_p_compressionLevel : if ((int)value > ZSTD_maxCLevel()) value = ZSTD_maxCLevel(); /* cap max compression level */ if (value == 0) return 0; /* special value : 0 means "don't change anything" */ + if (cctx->cdict) return ERROR(stage_wrong); cctx->compressionLevel = value; return 0; case ZSTD_p_windowLog : + DEBUGLOG(5, "setting ZSTD_p_windowLog = %u (cdict:%u)", + value, (cctx->cdict!=NULL)); if (value == 0) return 0; /* special value : 0 means "don't change anything" */ + if (cctx->cdict) return ERROR(stage_wrong); CLAMPCHECK(value, ZSTD_WINDOWLOG_MIN, ZSTD_WINDOWLOG_MAX); ZSTD_cLevelToCParams(cctx); cctx->requestedParams.cParams.windowLog = value; @@ -265,6 +269,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, unsigned v case ZSTD_p_hashLog : if (value == 0) return 0; /* special value : 0 means "don't change anything" */ + if (cctx->cdict) return ERROR(stage_wrong); CLAMPCHECK(value, ZSTD_HASHLOG_MIN, ZSTD_HASHLOG_MAX); ZSTD_cLevelToCParams(cctx); cctx->requestedParams.cParams.hashLog = value; @@ -272,6 +277,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, unsigned v case ZSTD_p_chainLog : if (value == 0) return 0; /* special value : 0 means "don't change anything" */ + if (cctx->cdict) return ERROR(stage_wrong); CLAMPCHECK(value, ZSTD_CHAINLOG_MIN, ZSTD_CHAINLOG_MAX); ZSTD_cLevelToCParams(cctx); cctx->requestedParams.cParams.chainLog = value; @@ -279,6 +285,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, unsigned v case ZSTD_p_searchLog : if (value == 0) return 0; /* special value : 0 means "don't change anything" */ + if (cctx->cdict) return ERROR(stage_wrong); CLAMPCHECK(value, ZSTD_SEARCHLOG_MIN, ZSTD_SEARCHLOG_MAX); ZSTD_cLevelToCParams(cctx); cctx->requestedParams.cParams.searchLog = value; @@ -286,6 +293,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, unsigned v case ZSTD_p_minMatch : if (value == 0) return 0; /* special value : 0 means "don't change anything" */ + if (cctx->cdict) return ERROR(stage_wrong); CLAMPCHECK(value, ZSTD_SEARCHLENGTH_MIN, ZSTD_SEARCHLENGTH_MAX); ZSTD_cLevelToCParams(cctx); cctx->requestedParams.cParams.searchLength = value; @@ -293,6 +301,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, unsigned v case ZSTD_p_targetLength : if (value == 0) return 0; /* special value : 0 means "don't change anything" */ + if (cctx->cdict) return ERROR(stage_wrong); CLAMPCHECK(value, ZSTD_TARGETLENGTH_MIN, ZSTD_TARGETLENGTH_MAX); ZSTD_cLevelToCParams(cctx); cctx->requestedParams.cParams.targetLength = value; @@ -300,6 +309,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, unsigned v case ZSTD_p_compressionStrategy : if (value == 0) return 0; /* special value : 0 means "don't change anything" */ + if (cctx->cdict) return ERROR(stage_wrong); CLAMPCHECK(value, (unsigned)ZSTD_fast, (unsigned)ZSTD_btultra); ZSTD_cLevelToCParams(cctx); cctx->requestedParams.cParams.strategy = (ZSTD_strategy)value; @@ -384,6 +394,7 @@ ZSTDLIB_API size_t ZSTD_CCtx_loadDictionary(ZSTD_CCtx* cctx, const void* dict, s { if (cctx->streamStage != zcss_init) return ERROR(stage_wrong); if (cctx->staticSize) return ERROR(memory_allocation); /* no malloc for static CCtx */ + DEBUGLOG(5, "load dictionary of size %u", (U32)dictSize); ZSTD_freeCDict(cctx->cdictLocal); /* in case one already exists */ if (dict==NULL || dictSize==0) { /* no dictionary mode */ cctx->cdictLocal = NULL; diff --git a/lib/zstd.h b/lib/zstd.h index 928abe240..b204b9a7b 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -528,8 +528,9 @@ ZSTDLIB_API ZSTD_CCtx* ZSTD_createCCtx_advanced(ZSTD_customMem customMem); * If it needs more memory than available, it will simply error out. * Note 2 : there is no corresponding "free" function. * Since workspace was allocated externally, it must be freed externally too. - * Limitation : currently not compatible with internal CDict creation, such as - * ZSTD_CCtx_loadDictionary() or ZSTD_initCStream_usingDict(). + * Limitation 1 : currently not compatible with internal CDict creation, such as + * ZSTD_CCtx_loadDictionary() or ZSTD_initCStream_usingDict(). + * Limitation 2 : currently not compatible with multi-threading */ ZSTDLIB_API ZSTD_CCtx* ZSTD_initStaticCCtx(void* workspace, size_t workspaceSize); @@ -746,7 +747,7 @@ ZSTDLIB_API size_t ZSTD_CCtx_setPledgedSrcSize(ZSTD_CCtx* cctx, unsigned long lo * It's also a CPU-heavy operation, with non-negligible impact on latency. * Note 3 : Dictionary will be used for all future compression jobs. * To return to "no-dictionary" situation, load a NULL dictionary */ -ZSTDLIB_API size_t ZSTD_CCtx_loadDictionary(ZSTD_CCtx* cctx, const void* dict, size_t dictSize); /* Not ready yet ! */ +ZSTDLIB_API size_t ZSTD_CCtx_loadDictionary(ZSTD_CCtx* cctx, const void* dict, size_t dictSize); /*! ZSTD_CCtx_refPrefix() : * Reference a prefix (content-only dictionary) to bootstrap next compression job. diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 09aaae54d..57993fcbb 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -1161,7 +1161,7 @@ _output_error: } -/* Tests for ZSTD_compressGeneric() API */ +/* Tests for ZSTD_compress_generic() API */ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double compressibility, int bigTests) { static const U32 maxSrcLog = 24; @@ -1181,7 +1181,7 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double ZSTD_DStream* zd = ZSTD_createDStream(); /* will be reset sometimes */ ZSTD_DStream* const zd_noise = ZSTD_createDStream(); clock_t const startClock = clock(); - const BYTE* dict=NULL; /* can keep same dict on 2 consecutive tests */ + const BYTE* dict = NULL; /* can keep same dict on 2 consecutive tests */ size_t dictSize = 0; U32 oldTestLog = 0; int const cLevelLimiter = bigTests ? 3 : 2; @@ -1278,11 +1278,13 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double dictSize = ((FUZ_rand(&lseed)&63)==1) ? FUZ_rLogLength(&lseed, dictLog) : 0; { size_t const dictStart = FUZ_rand(&lseed) % (srcBufferSize - dictSize); dict = srcBuffer + dictStart; + if (!dictSize) dict=NULL; } { U64 const pledgedSrcSize = (FUZ_rand(&lseed) & 3) ? ZSTD_CONTENTSIZE_UNKNOWN : maxTestSize; ZSTD_compressionParameters cParams = ZSTD_getCParams(cLevel, pledgedSrcSize, dictSize); /* mess with compression parameters */ + CHECK_Z( ZSTD_CCtx_loadDictionary(zc, NULL, 0) ); /* always cancel previous dict, to make user it's possible to pass compression parameters */ cParams.windowLog += (FUZ_rand(&lseed) & 3) - 1; cParams.hashLog += (FUZ_rand(&lseed) & 3) - 1; cParams.chainLog += (FUZ_rand(&lseed) & 3) - 1; @@ -1298,10 +1300,15 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double if (FUZ_rand(&lseed) & 1) CHECK_Z( ZSTD_CCtx_setParameter(zc, ZSTD_p_minMatch, cParams.searchLength) ); if (FUZ_rand(&lseed) & 1) CHECK_Z( ZSTD_CCtx_setParameter(zc, ZSTD_p_targetLength, cParams.targetLength) ); - if (FUZ_rand(&lseed) & 1) { dict=NULL; dictSize=0; } - CHECK_Z( ZSTD_CCtx_loadDictionary(zc, dict, dictSize) ); /* unconditionally set, to be sync with decoder (could be improved) */ + /* unconditionally set, to be sync with decoder */ + CHECK_Z( ZSTD_CCtx_loadDictionary(zc, dict, dictSize) ); - /* to do : check that cParams are blocked after loading non-NULL dictionary */ + if (dict && dictSize) { + /* test that compression parameters are correctly rejected after setting a dictionary */ + DISPLAYLEVEL(5, "setting windowLog while dict!=NULL \n"); + size_t const setError = ZSTD_CCtx_setParameter(zc, ZSTD_p_windowLog, cParams.windowLog-1) ; + CHECK(!ZSTD_isError(setError), "ZSTD_CCtx_setParameter should have failed"); + } /* mess with frame parameters */ if (FUZ_rand(&lseed) & 1) CHECK_Z( ZSTD_CCtx_setParameter(zc, ZSTD_p_checksumFlag, FUZ_rand(&lseed) & 1) ); @@ -1352,7 +1359,9 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double outBuff.size = outBuff.pos + adjustedDstSize; DISPLAYLEVEL(5, "End-flush into dst buffer of size %u \n", (U32)adjustedDstSize); remainingToFlush = ZSTD_compress_generic(zc, &outBuff, &inBuff, ZSTD_e_end); - CHECK(ZSTD_isError(remainingToFlush), "ZSTD_compress_generic w/ ZSTD_e_end error : %s", ZSTD_getErrorName(remainingToFlush)); + CHECK(ZSTD_isError(remainingToFlush), + "ZSTD_compress_generic w/ ZSTD_e_end error : %s", + ZSTD_getErrorName(remainingToFlush) ); } } crcOrig = XXH64_digest(&xxhState); cSize = outBuff.pos; @@ -1471,7 +1480,7 @@ int main(int argc, const char** argv) e_api selected_api = simple_api; const char* const programName = argv[0]; ZSTD_customMem const customMem = { allocFunction, freeFunction, NULL }; - ZSTD_customMem const customNULL = { NULL, NULL, NULL }; + ZSTD_customMem const customNULL = ZSTD_defaultCMem; /* Check command line */ for(argNb=1; argNb