From 44da314adb2b63f96b1d571f822e91a0e412e984 Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 27 Dec 2022 20:05:08 +0700 Subject: [PATCH] Add compress-level range checking for each compress-type. The prior range checking was done based on the valid values for gz. While this worked it was a subset of what is available for lz4 and zst. Allow the range to be specified for each compress-type. Adding this functionality to the parse module would be a better solution but that is a bigger project than this fix deserves, at least for now. --- doc/lib/pgBackRestDoc/Custom/DocConfigData.pm | 5 +- doc/xml/release.xml | 25 ++++++++++ src/build/config/config.yaml | 1 - src/common/compress/bz2/compress.c | 2 +- src/common/compress/bz2/compress.h | 7 +++ src/common/compress/gz/compress.c | 2 +- src/common/compress/gz/compress.h | 7 +++ src/common/compress/helper.c | 48 +++++++++++++++++-- src/common/compress/helper.intern.h | 6 +++ src/common/compress/lz4/compress.c | 2 +- src/common/compress/lz4/compress.h | 7 +++ src/common/compress/zst/compress.c | 2 +- src/common/compress/zst/compress.h | 7 +++ src/config/load.c | 24 ++++++++-- src/config/parse.auto.c.inc | 12 ----- test/src/module/common/compressTest.c | 34 ++++++++++--- test/src/module/config/loadTest.c | 34 +++++++++++++ 17 files changed, 189 insertions(+), 36 deletions(-) mode change 100644 => 100755 src/config/parse.auto.c.inc diff --git a/doc/lib/pgBackRestDoc/Custom/DocConfigData.pm b/doc/lib/pgBackRestDoc/Custom/DocConfigData.pm index 1046102e5..e2bb49d94 100644 --- a/doc/lib/pgBackRestDoc/Custom/DocConfigData.pm +++ b/doc/lib/pgBackRestDoc/Custom/DocConfigData.pm @@ -426,9 +426,8 @@ foreach my $strKey (sort(keys(%{$rhConfigDefine}))) $rhConfigDefine->{$strKey}{&CFGDEF_SECURE} = false; } - # All int, size and time options must have an allow range - if (($rhConfigDefine->{$strKey}{&CFGDEF_TYPE} eq CFGDEF_TYPE_INTEGER || - $rhConfigDefine->{$strKey}{&CFGDEF_TYPE} eq CFGDEF_TYPE_TIME || + # All size and time options must have an allow range + if (($rhConfigDefine->{$strKey}{&CFGDEF_TYPE} eq CFGDEF_TYPE_TIME || $rhConfigDefine->{$strKey}{&CFGDEF_TYPE} eq CFGDEF_TYPE_SIZE) && !(defined($rhConfigDefine->{$strKey}{&CFGDEF_ALLOW_RANGE}) || defined($rhConfigDefine->{$strKey}{&CFGDEF_ALLOW_LIST}))) { diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 617ce988e..695575fed 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -27,6 +27,21 @@

Remove support for PostgreSQL 9.0/9.1/9.2.

+ + + + + + + + + + + + + +

Add compress-level range checking for each compress-type.

+ @@ -12046,6 +12061,11 @@ geirra + + gkleen + gkleen + + Greg Sabino Mullane turnstep @@ -12606,6 +12626,11 @@ Viorel Tabara + + ViperRu + ViperRu + + Vitaliy Kukharik vitabaks diff --git a/src/build/config/config.yaml b/src/build/config/config.yaml index b83ea3a3f..76dff620b 100644 --- a/src/build/config/config.yaml +++ b/src/build/config/config.yaml @@ -692,7 +692,6 @@ option: section: global type: integer required: false - allow-range: [0, 9] command: compress command-role: async: {} diff --git a/src/common/compress/bz2/compress.c b/src/common/compress/bz2/compress.c index 192691795..596da8dfb 100644 --- a/src/common/compress/bz2/compress.c +++ b/src/common/compress/bz2/compress.c @@ -163,7 +163,7 @@ bz2CompressNew(int level) FUNCTION_LOG_PARAM(INT, level); FUNCTION_LOG_END(); - ASSERT(level > 0); + ASSERT(level >= BZ2_COMPRESS_LEVEL_MIN && level <= BZ2_COMPRESS_LEVEL_MAX); IoFilter *this = NULL; diff --git a/src/common/compress/bz2/compress.h b/src/common/compress/bz2/compress.h index c152c598b..8c3b15d12 100644 --- a/src/common/compress/bz2/compress.h +++ b/src/common/compress/bz2/compress.h @@ -13,6 +13,13 @@ Filter type constant ***********************************************************************************************************************************/ #define BZ2_COMPRESS_FILTER_TYPE STRID5("bz2-cmp", 0x41a3df3420) +/*********************************************************************************************************************************** +Level constants +***********************************************************************************************************************************/ +#define BZ2_COMPRESS_LEVEL_DEFAULT 9 +#define BZ2_COMPRESS_LEVEL_MIN 1 +#define BZ2_COMPRESS_LEVEL_MAX 9 + /*********************************************************************************************************************************** Constructors ***********************************************************************************************************************************/ diff --git a/src/common/compress/gz/compress.c b/src/common/compress/gz/compress.c index 1cd01e994..e2303e972 100644 --- a/src/common/compress/gz/compress.c +++ b/src/common/compress/gz/compress.c @@ -168,7 +168,7 @@ gzCompressNew(int level) FUNCTION_LOG_PARAM(INT, level); FUNCTION_LOG_END(); - ASSERT(level >= -1 && level <= 9); + ASSERT(level >= GZ_COMPRESS_LEVEL_MIN && level <= GZ_COMPRESS_LEVEL_MAX); IoFilter *this = NULL; diff --git a/src/common/compress/gz/compress.h b/src/common/compress/gz/compress.h index a31f307dc..a71f04623 100644 --- a/src/common/compress/gz/compress.h +++ b/src/common/compress/gz/compress.h @@ -13,6 +13,13 @@ Filter type constant ***********************************************************************************************************************************/ #define GZ_COMPRESS_FILTER_TYPE STRID5("gz-cmp", 0x20d1ef470) +/*********************************************************************************************************************************** +Level constants +***********************************************************************************************************************************/ +#define GZ_COMPRESS_LEVEL_DEFAULT 6 +#define GZ_COMPRESS_LEVEL_MIN -1 +#define GZ_COMPRESS_LEVEL_MAX 9 + /*********************************************************************************************************************************** Constructors ***********************************************************************************************************************************/ diff --git a/src/common/compress/helper.c b/src/common/compress/helper.c index 68808b4a9..6b5f4ccd4 100644 --- a/src/common/compress/helper.c +++ b/src/common/compress/helper.c @@ -42,7 +42,9 @@ static const struct CompressHelperLocal IoFilter *(*compressNew)(int); // Function to create new compression filter StringId decompressType; // Type of the decompression filter IoFilter *(*decompressNew)(void); // Function to create new decompression filter - int levelDefault; // Default compression level + int levelDefault:8; // Default compression level + int levelMin:8; // Minimum compression level + int levelMax:8; // Maximum compression level } compressHelperLocal[] = { { @@ -58,7 +60,9 @@ static const struct CompressHelperLocal .compressNew = bz2CompressNew, .decompressType = BZ2_DECOMPRESS_FILTER_TYPE, .decompressNew = bz2DecompressNew, - .levelDefault = 9, + .levelDefault = BZ2_COMPRESS_LEVEL_DEFAULT, + .levelMin = BZ2_COMPRESS_LEVEL_MIN, + .levelMax = BZ2_COMPRESS_LEVEL_MAX, }, { .typeId = STRID5("gz", 0x3470), @@ -68,7 +72,9 @@ static const struct CompressHelperLocal .compressNew = gzCompressNew, .decompressType = GZ_DECOMPRESS_FILTER_TYPE, .decompressNew = gzDecompressNew, - .levelDefault = 6, + .levelDefault = GZ_COMPRESS_LEVEL_DEFAULT, + .levelMin = GZ_COMPRESS_LEVEL_MIN, + .levelMax = GZ_COMPRESS_LEVEL_MAX, }, { .typeId = STRID6("lz4", 0x2068c1), @@ -79,7 +85,9 @@ static const struct CompressHelperLocal .compressNew = lz4CompressNew, .decompressType = LZ4_DECOMPRESS_FILTER_TYPE, .decompressNew = lz4DecompressNew, - .levelDefault = 1, + .levelDefault = LZ4_COMPRESS_LEVEL_DEFAULT, + .levelMin = LZ4_COMPRESS_LEVEL_MIN, + .levelMax = LZ4_COMPRESS_LEVEL_MAX, #endif }, { @@ -91,7 +99,9 @@ static const struct CompressHelperLocal .compressNew = zstCompressNew, .decompressType = ZST_DECOMPRESS_FILTER_TYPE, .decompressNew = zstDecompressNew, - .levelDefault = 3, + .levelDefault = ZST_COMPRESS_LEVEL_DEFAULT, + .levelMin = ZST_COMPRESS_LEVEL_MIN, + .levelMax = ZST_COMPRESS_LEVEL_MAX, #endif }, { @@ -190,6 +200,34 @@ compressLevelDefault(CompressType type) FUNCTION_TEST_RETURN(INT, compressHelperLocal[type].levelDefault); } +/**********************************************************************************************************************************/ +int +compressLevelMin(CompressType type) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(ENUM, type); + FUNCTION_TEST_END(); + + ASSERT(type < LENGTH_OF(compressHelperLocal)); + compressTypePresent(type); + + FUNCTION_TEST_RETURN(INT, compressHelperLocal[type].levelMin); +} + +/**********************************************************************************************************************************/ +int +compressLevelMax(CompressType type) +{ + FUNCTION_TEST_BEGIN(); + FUNCTION_TEST_PARAM(ENUM, type); + FUNCTION_TEST_END(); + + ASSERT(type < LENGTH_OF(compressHelperLocal)); + compressTypePresent(type); + + FUNCTION_TEST_RETURN(INT, compressHelperLocal[type].levelMax); +} + /**********************************************************************************************************************************/ IoFilter * compressFilter(CompressType type, int level) diff --git a/src/common/compress/helper.intern.h b/src/common/compress/helper.intern.h index 900644f8b..fb7efd84f 100644 --- a/src/common/compress/helper.intern.h +++ b/src/common/compress/helper.intern.h @@ -12,4 +12,10 @@ Functions // Default compression level for a compression type, used while loading the configuration int compressLevelDefault(CompressType type); +// Minimum compression level for a compression type, used while loading the configuration +int compressLevelMin(CompressType type); + +// Maximum compression level for a compression type, used while loading the configuration +int compressLevelMax(CompressType type); + #endif diff --git a/src/common/compress/lz4/compress.c b/src/common/compress/lz4/compress.c index 84f63b83b..e3d47284f 100644 --- a/src/common/compress/lz4/compress.c +++ b/src/common/compress/lz4/compress.c @@ -249,7 +249,7 @@ lz4CompressNew(int level) FUNCTION_LOG_PARAM(INT, level); FUNCTION_LOG_END(); - ASSERT(level >= 0); + ASSERT(level >= LZ4_COMPRESS_LEVEL_MIN && level <= LZ4_COMPRESS_LEVEL_MAX); IoFilter *this = NULL; diff --git a/src/common/compress/lz4/compress.h b/src/common/compress/lz4/compress.h index 345b14387..3422b0fcf 100644 --- a/src/common/compress/lz4/compress.h +++ b/src/common/compress/lz4/compress.h @@ -15,6 +15,13 @@ Filter type constant ***********************************************************************************************************************************/ #define LZ4_COMPRESS_FILTER_TYPE STRID6("lz4-cmp", 0x103436e068c1) +/*********************************************************************************************************************************** +Level constants +***********************************************************************************************************************************/ +#define LZ4_COMPRESS_LEVEL_DEFAULT 1 +#define LZ4_COMPRESS_LEVEL_MIN -5 +#define LZ4_COMPRESS_LEVEL_MAX 12 + /*********************************************************************************************************************************** Constructors ***********************************************************************************************************************************/ diff --git a/src/common/compress/zst/compress.c b/src/common/compress/zst/compress.c index 5779c2b0c..a3c263104 100644 --- a/src/common/compress/zst/compress.c +++ b/src/common/compress/zst/compress.c @@ -170,7 +170,7 @@ zstCompressNew(int level) FUNCTION_LOG_PARAM(INT, level); FUNCTION_LOG_END(); - ASSERT(level >= 0); + ASSERT(level >= ZST_COMPRESS_LEVEL_MIN && level <= ZST_COMPRESS_LEVEL_MAX); IoFilter *this = NULL; diff --git a/src/common/compress/zst/compress.h b/src/common/compress/zst/compress.h index 0021fadf2..6e52f8d41 100644 --- a/src/common/compress/zst/compress.h +++ b/src/common/compress/zst/compress.h @@ -15,6 +15,13 @@ Filter type constant ***********************************************************************************************************************************/ #define ZST_COMPRESS_FILTER_TYPE STRID5("zst-cmp", 0x41a3dd27a0) +/*********************************************************************************************************************************** +Level constants +***********************************************************************************************************************************/ +#define ZST_COMPRESS_LEVEL_DEFAULT 3 +#define ZST_COMPRESS_LEVEL_MIN -7 +#define ZST_COMPRESS_LEVEL_MAX 22 + /*********************************************************************************************************************************** Constructors ***********************************************************************************************************************************/ diff --git a/src/config/load.c b/src/config/load.c index deea2dfee..91832efa5 100644 --- a/src/config/load.c +++ b/src/config/load.c @@ -357,12 +357,26 @@ cfgLoadUpdateOption(void) if (cfgOptionValid(cfgOptCompressType)) compressTypePresent(compressTypeEnum(cfgOptionStrId(cfgOptCompressType))); - // Update compress-level default based on the compression type - if (cfgOptionValid(cfgOptCompressLevel) && cfgOptionSource(cfgOptCompressLevel) == cfgSourceDefault) + // Update compress-level default based on the compression type. Also check that level range is valid per compression type. + if (cfgOptionValid(cfgOptCompressLevel)) { - cfgOptionSet( - cfgOptCompressLevel, cfgSourceDefault, - VARINT64(compressLevelDefault(compressTypeEnum(cfgOptionStrId(cfgOptCompressType))))); + const CompressType compressType = compressTypeEnum(cfgOptionStrId(cfgOptCompressType)); + + if (cfgOptionSource(cfgOptCompressLevel) == cfgSourceDefault) + { + cfgOptionSet(cfgOptCompressLevel, cfgSourceDefault, VARINT64(compressLevelDefault(compressType))); + } + else if (compressType != compressTypeNone) + { + if (cfgOptionInt(cfgOptCompressLevel) < compressLevelMin(compressType) || + cfgOptionInt(cfgOptCompressLevel) > compressLevelMax(compressType)) + { + THROW_FMT( + OptionInvalidValueError, + "'%d' is out of range for '" CFGOPT_COMPRESS_LEVEL "' option when '" CFGOPT_COMPRESS_TYPE "' option = '%s'", + cfgOptionInt(cfgOptCompressLevel), strZ(strIdToStr(cfgOptionStrId(cfgOptCompressType)))); + } + } } FUNCTION_LOG_RETURN_VOID(); diff --git a/src/config/parse.auto.c.inc b/src/config/parse.auto.c.inc old mode 100644 new mode 100755 index 64d428569..2ee3597ba --- a/src/config/parse.auto.c.inc +++ b/src/config/parse.auto.c.inc @@ -1374,18 +1374,6 @@ static const ParseRuleOption parseRuleOption[CFG_OPTION_TOTAL] = PARSE_RULE_OPTION_COMMAND_ROLE_ASYNC_VALID_LIST // opt/compress-level ( // opt/compress-level PARSE_RULE_OPTION_COMMAND(cfgCmdArchivePush) // opt/compress-level - ), // opt/compress-level - // opt/compress-level - PARSE_RULE_OPTIONAL // opt/compress-level - ( // opt/compress-level - PARSE_RULE_OPTIONAL_GROUP // opt/compress-level - ( // opt/compress-level - PARSE_RULE_OPTIONAL_ALLOW_RANGE // opt/compress-level - ( // opt/compress-level - PARSE_RULE_VAL_INT(parseRuleValInt0), // opt/compress-level - PARSE_RULE_VAL_INT(parseRuleValInt9), // opt/compress-level - ), // opt/compress-level - ), // opt/compress-level ), // opt/compress-level ), // opt/compress-level // ----------------------------------------------------------------------------------------------------------------------------- diff --git a/test/src/module/common/compressTest.c b/test/src/module/common/compressTest.c index 59290c8d9..c3d9aa456 100644 --- a/test/src/module/common/compressTest.c +++ b/test/src/module/common/compressTest.c @@ -196,6 +196,13 @@ testRun(void) TEST_ERROR(gzError(Z_VERSION_ERROR), FormatError, "zlib threw error: [-6] incompatible version"); TEST_ERROR(gzError(999), AssertError, "zlib threw error: [999] unknown error"); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("compressLevelDefault(), compressLevelMin(), and compressLevelMax()"); + + TEST_RESULT_INT(compressLevelDefault(compressTypeGz), 6, "level default"); + TEST_RESULT_INT(compressLevelMin(compressTypeGz), -1, "level default"); + TEST_RESULT_INT(compressLevelMax(compressTypeGz), 9, "level default"); + // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("gzDecompressToLog() and gzCompressToLog()"); @@ -234,6 +241,13 @@ testRun(void) TEST_ERROR(bz2Error(BZ_CONFIG_ERROR), AssertError, "bz2 error: [-9] config error"); TEST_ERROR(bz2Error(-999), AssertError, "bz2 error: [-999] unknown error"); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("compressLevelDefault(), compressLevelMin(), and compressLevelMax()"); + + TEST_RESULT_INT(compressLevelDefault(compressTypeBz2), 9, "level default"); + TEST_RESULT_INT(compressLevelMin(compressTypeBz2), 1, "level default"); + TEST_RESULT_INT(compressLevelMax(compressTypeBz2), 9, "level default"); + // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("bz2DecompressToLog() and bz2CompressToLog()"); @@ -265,6 +279,13 @@ testRun(void) TEST_RESULT_UINT(lz4Error(0), 0, "check success"); TEST_ERROR(lz4Error((size_t)-2), FormatError, "lz4 error: [-2] ERROR_maxBlockSize_invalid"); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("compressLevelDefault(), compressLevelMin(), and compressLevelMax()"); + + TEST_RESULT_INT(compressLevelDefault(compressTypeLz4), 1, "level default"); + TEST_RESULT_INT(compressLevelMin(compressTypeLz4), -5, "level default"); + TEST_RESULT_INT(compressLevelMax(compressTypeLz4), 12, "level default"); + // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("lz4DecompressToLog() and lz4CompressToLog()"); @@ -303,6 +324,13 @@ testRun(void) TEST_RESULT_UINT(zstError(0), 0, "check success"); TEST_ERROR(zstError((size_t)-12), FormatError, "zst error: [-12] Version not supported"); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("compressLevelDefault(), compressLevelMin(), and compressLevelMax()"); + + TEST_RESULT_INT(compressLevelDefault(compressTypeZst), 3, "level default"); + TEST_RESULT_INT(compressLevelMin(compressTypeZst), -7, "level default"); + TEST_RESULT_INT(compressLevelMax(compressTypeZst), 22, "level default"); + // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("zstDecompressToLog() and zstCompressToLog()"); @@ -380,12 +408,6 @@ testRun(void) TEST_ERROR(compressExtStrip(STRDEF("file"), compressTypeGz), FormatError, "'file' must have '.gz' extension"); TEST_RESULT_STR_Z(compressExtStrip(STRDEF("file"), compressTypeNone), "file", "nothing to strip"); TEST_RESULT_STR_Z(compressExtStrip(STRDEF("file.gz"), compressTypeGz), "file", "strip gz"); - - // ------------------------------------------------------------------------------------------------------------------------- - TEST_TITLE("compressLevelDefault()"); - - TEST_RESULT_INT(compressLevelDefault(compressTypeNone), 0, "none level=0"); - TEST_RESULT_INT(compressLevelDefault(compressTypeGz), 6, "gz level=6"); } FUNCTION_HARNESS_RETURN_VOID(); diff --git a/test/src/module/config/loadTest.c b/test/src/module/config/loadTest.c index a27ec8ed0..e5d28a7ae 100644 --- a/test/src/module/config/loadTest.c +++ b/test/src/module/config/loadTest.c @@ -516,6 +516,40 @@ testRun(void) TEST_RESULT_INT(cfgOptionInt(cfgOptCompressLevel), 9, "compress-level=9"); TEST_RESULT_BOOL(cfgOptionValid(cfgOptCompress), false, "compress is not valid"); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("error on invalid compress level"); + + argList = strLstNew(); + hrnCfgArgRawZ(argList, cfgOptStanza, "db"); + hrnCfgArgRawZ(argList, cfgOptCompressType, "gz"); + hrnCfgArgRawZ(argList, cfgOptCompressLevel, "-2"); + + TEST_ERROR( + hrnCfgLoadP(cfgCmdArchivePush, argList), OptionInvalidValueError, + "'-2' is out of range for 'compress-level' option when 'compress-type' option = 'gz'"); + + argList = strLstNew(); + hrnCfgArgRawZ(argList, cfgOptStanza, "db"); + hrnCfgArgRawZ(argList, cfgOptCompressType, "gz"); + hrnCfgArgRawZ(argList, cfgOptCompressLevel, "10"); + + TEST_ERROR( + hrnCfgLoadP(cfgCmdArchivePush, argList), OptionInvalidValueError, + "'10' is out of range for 'compress-level' option when 'compress-type' option = 'gz'"); + + // In practice level should not be used here but preserve the prior behavior in case something depends on it + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("do not check range when compress-type = none"); + + argList = strLstNew(); + hrnCfgArgRawZ(argList, cfgOptStanza, "db"); + hrnCfgArgRawZ(argList, cfgOptCompressType, "none"); + hrnCfgArgRawZ(argList, cfgOptCompressLevel, "3"); + + HRN_CFG_LOAD(cfgCmdArchivePush, argList); + TEST_RESULT_UINT(cfgOptionStrId(cfgOptCompressType), CFGOPTVAL_COMPRESS_TYPE_NONE, "compress-type=none"); + TEST_RESULT_INT(cfgOptionInt(cfgOptCompressLevel), 3, "compress-level=3"); + // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("warn when compress-type and compress both set");