From f0a7651fce53f5e85da6140f9d075b730ae6eac7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 28 Mar 2017 17:10:01 -0700 Subject: Safer LZ4_getFrameInfo() LZ4_getFrameInfo() is now guaranteed to keep dctx state clean, even in case of failure. --- lib/lz4frame.c | 112 +++++++++++++++++++++++++++++--------------------- lib/lz4frame.h | 27 +++++++----- lib/lz4frame_static.h | 35 +++++++++------- tests/frametest.c | 26 ++++++++---- 4 files changed, 120 insertions(+), 80 deletions(-) diff --git a/lib/lz4frame.c b/lib/lz4frame.c index 119dbee..518194d 100644 --- a/lib/lz4frame.c +++ b/lib/lz4frame.c @@ -779,7 +779,9 @@ LZ4F_errorCode_t LZ4F_freeDecompressionContext(LZ4F_dctx* const dctxPtr) /*==--- Streaming Decompression operations ---==*/ -typedef enum { dstage_getHeader=0, dstage_storeHeader, +typedef enum { + dstage_getHeader=0, dstage_storeHeader, + dstage_init, dstage_getCBlockSize, dstage_storeCBlockSize, dstage_copyDirect, dstage_getCBlock, dstage_storeCBlock, @@ -896,65 +898,57 @@ static size_t LZ4F_decodeHeader(LZ4F_dctx* dctxPtr, const void* src, size_t srcS if (contentSizeFlag) dctxPtr->frameRemainingSize = dctxPtr->frameInfo.contentSize = LZ4F_readLE64(srcPtr+6); - /* init */ - if (contentChecksumFlag) XXH32_reset(&(dctxPtr->xxh), 0); - - /* internal buffers allocation */ - { size_t const bufferNeeded = dctxPtr->maxBlockSize + ((dctxPtr->frameInfo.blockMode==LZ4F_blockLinked) * 128 KB); - if (bufferNeeded > dctxPtr->maxBufferSize) { /* tmp buffers too small */ - dctxPtr->maxBufferSize = 0; /* ensure allocation will be re-attempted on next entry*/ - FREEMEM(dctxPtr->tmpIn); - dctxPtr->tmpIn = (BYTE*)ALLOCATOR(dctxPtr->maxBlockSize); - if (dctxPtr->tmpIn == NULL) return err0r(LZ4F_ERROR_allocation_failed); - FREEMEM(dctxPtr->tmpOutBuffer); - dctxPtr->tmpOutBuffer= (BYTE*)ALLOCATOR(bufferNeeded); - if (dctxPtr->tmpOutBuffer== NULL) return err0r(LZ4F_ERROR_allocation_failed); - dctxPtr->maxBufferSize = bufferNeeded; - } } - dctxPtr->tmpInSize = 0; - dctxPtr->tmpInTarget = 0; - dctxPtr->dict = dctxPtr->tmpOutBuffer; - dctxPtr->dictSize = 0; - dctxPtr->tmpOut = dctxPtr->tmpOutBuffer; - dctxPtr->tmpOutStart = 0; - dctxPtr->tmpOutSize = 0; - - dctxPtr->dStage = dstage_getCBlockSize; + dctxPtr->dStage = dstage_init; return frameHeaderSize; } /*! LZ4F_getFrameInfo() : -* Decodes frame header information, such as blockSize. Usage is optional. -* The objective is to extract header information before receiving decompressed data, typically for allocation purposes. -* LZ4F_getFrameInfo() can also be used *after* starting decompression, on a valid LZ4F_decompressionContext_t. -* The number of bytes consumed from srcBuffer will be provided within *srcSizePtr (necessarily <= original value). -* Decompression must resume from where it stopped (srcBuffer + *srcSizePtr) -* @return : hint of the better `srcSize` to use for next call to LZ4F_decompress, -* or an error code which can be tested using LZ4F_isError(). -*/ + * This function extracts frame parameters (such as max blockSize, frame checksum, etc.). + * Its usage is optional. The objective is to provide relevant information for allocation purposes. + * This function works in 2 situations : + * - At the beginning of a new frame, in which case it will decode this information from `srcBuffer`, and start the decoding process. + * Amount of input data provided must be large enough to successfully decode the frame header. + * A header size is variable, but is guaranteed to be <= LZ4F_HEADER_SIZE_MAX bytes. It's possible to provide more input data than this minimum. + * - After decoding has been started. In which case, no input is read, frame parameters are extracted from dctx. + * The number of bytes consumed from srcBuffer will be updated within *srcSizePtr (necessarily <= original value). + * Decompression must resume from (srcBuffer + *srcSizePtr). + * @return : an hint about how many srcSize bytes LZ4F_decompress() expects for next call, + * or an error code which can be tested using LZ4F_isError() + * note 1 : in case of error, dctx is not modified. Decoding operations can resume from where they stopped. + * note 2 : frame parameters are *copied into* an already allocated LZ4F_frameInfo_t structure. + */ LZ4F_errorCode_t LZ4F_getFrameInfo(LZ4F_dctx* dctxPtr, LZ4F_frameInfo_t* frameInfoPtr, const void* srcBuffer, size_t* srcSizePtr) { - if (dctxPtr->dStage > dstage_storeHeader) { /* note : requires dstage_* header related to be at beginning of enum */ + if (dctxPtr->dStage > dstage_storeHeader) { /* assumption : dstage_* header enum at beginning of range */ /* frameInfo already decoded */ size_t o=0, i=0; *srcSizePtr = 0; *frameInfoPtr = dctxPtr->frameInfo; return LZ4F_decompress(dctxPtr, NULL, &o, NULL, &i, NULL); /* returns : recommended nb of bytes for LZ4F_decompress() */ } else { - size_t nextSrcSize, o=0; - size_t const hSize = LZ4F_headerSize(srcBuffer, *srcSizePtr); - if (LZ4F_isError(hSize)) { *srcSizePtr=0; return hSize; } - if (*srcSizePtr < hSize) { *srcSizePtr=0; return err0r(LZ4F_ERROR_frameHeader_incomplete); } - - *srcSizePtr = hSize; - nextSrcSize = LZ4F_decompress(dctxPtr, NULL, &o, srcBuffer, srcSizePtr, NULL); - if (dctxPtr->dStage <= dstage_storeHeader) return err0r(LZ4F_ERROR_frameHeader_incomplete); /* should not happen, already checked */ - *frameInfoPtr = dctxPtr->frameInfo; - return nextSrcSize; - } + if (dctxPtr->dStage == dstage_storeHeader) { + /* frame decoding already started, in the middle of header => automatic fail */ + *srcSizePtr = 0; + return err0r(LZ4F_ERROR_frameDecoding_alreadyStarted); + } else { + size_t decodeResult; + size_t const hSize = LZ4F_headerSize(srcBuffer, *srcSizePtr); + if (LZ4F_isError(hSize)) { *srcSizePtr=0; return hSize; } + if (*srcSizePtr < hSize) { *srcSizePtr=0; return err0r(LZ4F_ERROR_frameHeader_incomplete); } + + decodeResult = LZ4F_decodeHeader(dctxPtr, srcBuffer, hSize); + if (LZ4F_isError(decodeResult)) { + *srcSizePtr = 0; + } else { + *srcSizePtr = decodeResult; + decodeResult = BHSize; /* block header size */ + } + *frameInfoPtr = dctxPtr->frameInfo; + return decodeResult; + } } } @@ -1064,7 +1058,7 @@ size_t LZ4F_decompress(LZ4F_dctx* dctxPtr, *srcSizePtr = 0; *dstSizePtr = 0; - /* programmed as a state machine */ + /* behaves like a state machine */ while (doAnotherStage) { @@ -1079,6 +1073,7 @@ size_t LZ4F_decompress(LZ4F_dctx* dctxPtr, break; } dctxPtr->tmpInSize = 0; + if (srcEnd-srcPtr == 0) return minFHSize; /* 0-size input */ dctxPtr->tmpInTarget = minFHSize; /* minimum to attempt decode */ dctxPtr->dStage = dstage_storeHeader; /* pass-through */ @@ -1100,6 +1095,31 @@ size_t LZ4F_decompress(LZ4F_dctx* dctxPtr, break; } + case dstage_init: + if (dctxPtr->frameInfo.contentChecksumFlag) XXH32_reset(&(dctxPtr->xxh), 0); + /* internal buffers allocation */ + { size_t const bufferNeeded = dctxPtr->maxBlockSize + ((dctxPtr->frameInfo.blockMode==LZ4F_blockLinked) * 128 KB); + if (bufferNeeded > dctxPtr->maxBufferSize) { /* tmp buffers too small */ + dctxPtr->maxBufferSize = 0; /* ensure allocation will be re-attempted on next entry*/ + FREEMEM(dctxPtr->tmpIn); + dctxPtr->tmpIn = (BYTE*)ALLOCATOR(dctxPtr->maxBlockSize); + if (dctxPtr->tmpIn == NULL) return err0r(LZ4F_ERROR_allocation_failed); + FREEMEM(dctxPtr->tmpOutBuffer); + dctxPtr->tmpOutBuffer= (BYTE*)ALLOCATOR(bufferNeeded); + if (dctxPtr->tmpOutBuffer== NULL) return err0r(LZ4F_ERROR_allocation_failed); + dctxPtr->maxBufferSize = bufferNeeded; + } } + dctxPtr->tmpInSize = 0; + dctxPtr->tmpInTarget = 0; + dctxPtr->dict = dctxPtr->tmpOutBuffer; + dctxPtr->dictSize = 0; + dctxPtr->tmpOut = dctxPtr->tmpOutBuffer; + dctxPtr->tmpOutStart = 0; + dctxPtr->tmpOutSize = 0; + + dctxPtr->dStage = dstage_getCBlockSize; + /* pass-through */ + case dstage_getCBlockSize: if ((size_t)(srcEnd - srcPtr) >= BHSize) { selectedIn = srcPtr; diff --git a/lib/lz4frame.h b/lib/lz4frame.h index a0cf0ab..7c33464 100644 --- a/lib/lz4frame.h +++ b/lib/lz4frame.h @@ -303,25 +303,30 @@ LZ4FLIB_API LZ4F_errorCode_t LZ4F_createDecompressionContext(LZ4F_dctx** dctxPtr LZ4FLIB_API LZ4F_errorCode_t LZ4F_freeDecompressionContext(LZ4F_dctx* const dctx); -/* ====== Decompression ======*/ +/*-*********************************** +* Streaming decompression functions +*************************************/ -/*!LZ4F_getFrameInfo() : - * This function decodes frame header information (such as max blockSize, frame checksum, etc.). - * Its usage is optional. The objective is to extract frame header information, typically for allocation purposes. - * A header size is variable and can length from 7 to 15 bytes. It's possible to provide more input bytes than that. +/*! LZ4F_getFrameInfo() : + * This function extracts frame parameters (such as max blockSize, frame checksum, etc.). + * Its usage is optional. The objective is to provide relevant information for allocation purposes. + * This function works in 2 situations : + * - At the beginning of a new frame, in which case it will decode this information from `srcBuffer`, and start the decoding process. + * Amount of input data provided must be large enough to successfully decode the frame header. + * A header size is variable, but is guaranteed to be <= LZ4F_HEADER_SIZE_MAX bytes. It's possible to provide more input data than this minimum. + * - After decoding has been started. In which case, no input is read, frame parameters are extracted from dctx. * The number of bytes consumed from srcBuffer will be updated within *srcSizePtr (necessarily <= original value). - * Decompression must resume from this point (srcBuffer + *srcSizePtr). - * Note that LZ4F_getFrameInfo() can also be used anytime *after* decompression is started, in which case 0 input byte can be enough. - * Frame header info is *copied into* an already allocated LZ4F_frameInfo_t structure. + * Decompression must resume from (srcBuffer + *srcSizePtr). * @return : an hint about how many srcSize bytes LZ4F_decompress() expects for next call, * or an error code which can be tested using LZ4F_isError() - * (typically, when there is not enough src bytes to fully decode the frame header) + * note 1 : in case of error, dctx is not modified. Decoding operations can resume from where they stopped. + * note 2 : frame parameters are *copied into* an already allocated LZ4F_frameInfo_t structure. */ LZ4FLIB_API size_t LZ4F_getFrameInfo(LZ4F_dctx* dctx, LZ4F_frameInfo_t* frameInfoPtr, const void* srcBuffer, size_t* srcSizePtr); -/*!LZ4F_decompress() : +/*! LZ4F_decompress() : * Call this function repetitively to regenerate data compressed within `srcBuffer`. * The function will attempt to decode up to *srcSizePtr bytes from srcBuffer, into dstBuffer of capacity *dstSizePtr. * @@ -337,7 +342,7 @@ LZ4FLIB_API size_t LZ4F_getFrameInfo(LZ4F_dctx* dctx, * * @return is an hint of how many `srcSize` bytes LZ4F_decompress() expects for next call. * Schematically, it's the size of the current (or remaining) compressed block + header of next block. - * Respecting the hint provides some boost to performance, since it does skip intermediate buffers. + * Respecting the hint provides some small speed benefit, because it skips intermediate buffers. * This is just a hint though, it's always possible to provide any srcSize. * When a frame is fully decoded, @return will be 0 (no more data expected). * If decompression failed, @return is an error code, which can be tested using LZ4F_isError(). diff --git a/lib/lz4frame_static.h b/lib/lz4frame_static.h index f2228a5..d3bae82 100644 --- a/lib/lz4frame_static.h +++ b/lib/lz4frame_static.h @@ -43,7 +43,7 @@ extern "C" { /* lz4frame_static.h should be used solely in the context of static linking. * It contains definitions which are not stable and may change in the future. * Never use it in the context of DLL linking. - * */ + */ /* --- Dependency --- */ @@ -52,25 +52,32 @@ extern "C" { /* --- Error List --- */ #define LZ4F_LIST_ERRORS(ITEM) \ - ITEM(OK_NoError) ITEM(ERROR_GENERIC) \ - ITEM(ERROR_maxBlockSize_invalid) ITEM(ERROR_blockMode_invalid) ITEM(ERROR_contentChecksumFlag_invalid) \ + ITEM(OK_NoError) \ + ITEM(ERROR_GENERIC) \ + ITEM(ERROR_maxBlockSize_invalid) \ + ITEM(ERROR_blockMode_invalid) \ + ITEM(ERROR_contentChecksumFlag_invalid) \ ITEM(ERROR_compressionLevel_invalid) \ - ITEM(ERROR_headerVersion_wrong) ITEM(ERROR_blockChecksum_unsupported) ITEM(ERROR_reservedFlag_set) \ + ITEM(ERROR_headerVersion_wrong) \ + ITEM(ERROR_blockChecksum_unsupported) \ + ITEM(ERROR_reservedFlag_set) \ ITEM(ERROR_allocation_failed) \ - ITEM(ERROR_srcSize_tooLarge) ITEM(ERROR_dstMaxSize_tooSmall) \ - ITEM(ERROR_frameHeader_incomplete) ITEM(ERROR_frameType_unknown) ITEM(ERROR_frameSize_wrong) \ + ITEM(ERROR_srcSize_tooLarge) \ + ITEM(ERROR_dstMaxSize_tooSmall) \ + ITEM(ERROR_frameHeader_incomplete) \ + ITEM(ERROR_frameType_unknown) \ + ITEM(ERROR_frameSize_wrong) \ ITEM(ERROR_srcPtr_wrong) \ ITEM(ERROR_decompressionFailed) \ - ITEM(ERROR_headerChecksum_invalid) ITEM(ERROR_contentChecksum_invalid) \ + ITEM(ERROR_headerChecksum_invalid) \ + ITEM(ERROR_contentChecksum_invalid) \ + ITEM(ERROR_frameDecoding_alreadyStarted) \ ITEM(ERROR_maxCode) -#define LZ4F_DISABLE_OLD_ENUMS /* comment to enable deprecated enums */ -#ifndef LZ4F_DISABLE_OLD_ENUMS -# define LZ4F_GENERATE_ENUM(ENUM) LZ4F_##ENUM, ENUM = LZ4F_##ENUM, -#else -# define LZ4F_GENERATE_ENUM(ENUM) LZ4F_##ENUM, -#endif -typedef enum { LZ4F_LIST_ERRORS(LZ4F_GENERATE_ENUM) } LZ4F_errorCodes; /* enum is exposed, to handle specific errors; compare function result to -enum value */ +#define LZ4F_GENERATE_ENUM(ENUM) LZ4F_##ENUM, + +/* enum list is exposed, to handle specific errors */ +typedef enum { LZ4F_LIST_ERRORS(LZ4F_GENERATE_ENUM) } LZ4F_errorCodes; LZ4F_errorCodes LZ4F_getErrorCode(size_t functionResult); diff --git a/tests/frametest.c b/tests/frametest.c index e2e0f86..f7e3abf 100644 --- a/tests/frametest.c +++ b/tests/frametest.c @@ -276,17 +276,25 @@ int basicTests(U32 seed, double compressibility) if (LZ4F_isError(errorCode)) goto _output_error; DISPLAYLEVEL(3, " %u \n", (unsigned)errorCode); - DISPLAYLEVEL(3, "get FrameInfo on null input : "); - errorCode = LZ4F_getFrameInfo(dCtx, &fi, ip, &iSize); - if (errorCode != (size_t)-LZ4F_ERROR_frameHeader_incomplete) goto _output_error; - DISPLAYLEVEL(3, " correctly failed : %s \n", LZ4F_getErrorName(errorCode)); + DISPLAYLEVEL(3, "LZ4F_getFrameInfo on zero-size input : "); + { size_t nullSize = 0; + size_t const fiError = LZ4F_getFrameInfo(dCtx, &fi, ip, &nullSize); + if (LZ4F_getErrorCode(fiError) != LZ4F_ERROR_frameHeader_incomplete) { + DISPLAYLEVEL(3, "incorrect error : %s != ERROR_frameHeader_incomplete \n", LZ4F_getErrorName(fiError)); + goto _output_error; + } + DISPLAYLEVEL(3, " correctly failed : %s \n", LZ4F_getErrorName(fiError)); + } DISPLAYLEVEL(3, "get FrameInfo on not enough input : "); - iSize = 6; - errorCode = LZ4F_getFrameInfo(dCtx, &fi, ip, &iSize); - if (errorCode != (size_t)-LZ4F_ERROR_frameHeader_incomplete) goto _output_error; - DISPLAYLEVEL(3, " correctly failed : %s \n", LZ4F_getErrorName(errorCode)); - ip += iSize; + { size_t inputSize = 6; + size_t const fiError = LZ4F_getFrameInfo(dCtx, &fi, ip, &inputSize); + if (LZ4F_getErrorCode(fiError) != LZ4F_ERROR_frameHeader_incomplete) { + DISPLAYLEVEL(3, "incorrect error : %s != ERROR_frameHeader_incomplete \n", LZ4F_getErrorName(fiError)); + goto _output_error; + } + DISPLAYLEVEL(3, " correctly failed : %s \n", LZ4F_getErrorName(fiError)); + } DISPLAYLEVEL(3, "get FrameInfo on enough input : "); iSize = 15 - iSize; -- cgit v0.12