From a957091eb481039bf597333ba4c82311d7432b0a Mon Sep 17 00:00:00 2001 From: Liam Merino Date: Thu, 23 Apr 2026 23:49:59 -0700 Subject: [PATCH 1/3] Fix #2081: Apply zstd compression level reliably across the documented range --- .../LightPcapNg/src/light_zstd_compression.c | 9 +++++-- Tests/Pcap++Test/Common/PcapFileNamesDef.h | 1 + Tests/Pcap++Test/TestDefinition.h | 1 + Tests/Pcap++Test/Tests/FileTests.cpp | 25 +++++++++++++++++++ Tests/Pcap++Test/main.cpp | 1 + 5 files changed, 35 insertions(+), 2 deletions(-) diff --git a/3rdParty/LightPcapNg/LightPcapNg/src/light_zstd_compression.c b/3rdParty/LightPcapNg/LightPcapNg/src/light_zstd_compression.c index 2212fe300f..c912b63a18 100644 --- a/3rdParty/LightPcapNg/LightPcapNg/src/light_zstd_compression.c +++ b/3rdParty/LightPcapNg/LightPcapNg/src/light_zstd_compression.c @@ -57,8 +57,13 @@ _compression_t * get_zstd_compression_context(int compression_level) context->buffer_out_max_size = max(ZSTD_CStreamOutSize(), COMPRESSION_BUFFER_IN_MAX_SIZE); context->buffer_in = malloc(context->buffer_in_max_size); context->buffer_out = malloc(context->buffer_out_max_size); - context->compression_level = compression_level * 2; //Input is scale 0-10 but zstd goes 0 - 20! - assert(!ZSTD_isError(ZSTD_CCtx_setParameter(context->cctx, ZSTD_c_compressionLevel, compression_level))); + // Map 0-10 input scale onto zstd's 1-22 range; preserve 0 as the "default + // compression" sentinel. setParameter must run outside assert() so its + // side effect is not stripped under NDEBUG. + context->compression_level = compression_level > 0 ? (compression_level * 2) + 1 : 0; + size_t const set_param_result = ZSTD_CCtx_setParameter(context->cctx, ZSTD_c_compressionLevel, context->compression_level); + assert(!ZSTD_isError(set_param_result)); + (void)set_param_result; return context; } diff --git a/Tests/Pcap++Test/Common/PcapFileNamesDef.h b/Tests/Pcap++Test/Common/PcapFileNamesDef.h index 734481fc32..50eeab3ae2 100644 --- a/Tests/Pcap++Test/Common/PcapFileNamesDef.h +++ b/Tests/Pcap++Test/Common/PcapFileNamesDef.h @@ -20,6 +20,7 @@ #define EXAMPLE_PCAPNG_WRITE_PATH "PcapExamples/many_interfaces_copy.pcapng" #define EXAMPLE2_PCAPNG_WRITE_PATH "PcapExamples/pcapng-example-write.pcapng" #define EXAMPLE_PCAPNG_ZSTD_WRITE_PATH "PcapExamples/many_interfaces_copy.pcapng.zstd" +#define EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH "PcapExamples/many_interfaces_levels.pcapng.zstd" #define EXAMPLE2_PCAPNG_ZSTD_WRITE_PATH "PcapExamples/pcapng-example-write.pcapng.zstd" #define EXAMPLE2_PCAPNG_ZST_WRITE_PATH "PcapExamples/pcapng-example-write.pcapng.zst" #define EXAMPLE_PCAPNG_INTERFACES_PATH "PcapExamples/too_many_interfaces.pcapng" diff --git a/Tests/Pcap++Test/TestDefinition.h b/Tests/Pcap++Test/TestDefinition.h index 40e427d3a9..9d07a4c274 100644 --- a/Tests/Pcap++Test/TestDefinition.h +++ b/Tests/Pcap++Test/TestDefinition.h @@ -30,6 +30,7 @@ PTF_TEST_CASE(TestPcapFileReadAdv); PTF_TEST_CASE(TestPcapFileWriteAdv); PTF_TEST_CASE(TestPcapFileAppend); PTF_TEST_CASE(TestPcapNgFileReadWrite); +PTF_TEST_CASE(TestPcapNgZstdCompressionLevels); PTF_TEST_CASE(TestPcapNgFileReadWriteAdv); PTF_TEST_CASE(TestPcapNgFileTooManyInterfaces); PTF_TEST_CASE(TestPcapFileReadLinkTypeIPv6); diff --git a/Tests/Pcap++Test/Tests/FileTests.cpp b/Tests/Pcap++Test/Tests/FileTests.cpp index 5818fd9ef6..552d5b8e6b 100644 --- a/Tests/Pcap++Test/Tests/FileTests.cpp +++ b/Tests/Pcap++Test/Tests/FileTests.cpp @@ -1314,6 +1314,31 @@ PTF_TEST_CASE(TestPcapNgFileReadWrite) } // TestPcapNgFileReadWrite +PTF_TEST_CASE(TestPcapNgZstdCompressionLevels) +{ + // If the compression level is silently ignored, low and high levels + // produce identical output sizes. + int sizes[2] = { 0, 0 }; + const int levels[2] = { 1, 10 }; + for (int i = 0; i < 2; i++) + { + pcpp::PcapNgFileReaderDevice readerDev(EXAMPLE_PCAPNG_PATH); + PTF_ASSERT_TRUE(readerDev.open()); + + pcpp::PcapNgFileWriterDevice writerDev(EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH, levels[i]); + PTF_ASSERT_TRUE(writerDev.open()); + + pcpp::RawPacket rawPacket; + while (readerDev.getNextPacket(rawPacket)) + PTF_ASSERT_TRUE(writerDev.writePacket(rawPacket)); + readerDev.close(); + writerDev.close(); + + sizes[i] = getFileLength(EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH); + } + PTF_ASSERT_GREATER_THAN(sizes[0], sizes[1]); +} // TestPcapNgZstdCompressionLevels + PTF_TEST_CASE(TestPcapNgFileReadWriteAdv) { pcpp::PcapNgFileReaderDevice readerDev(EXAMPLE2_PCAPNG_PATH); diff --git a/Tests/Pcap++Test/main.cpp b/Tests/Pcap++Test/main.cpp index 879da2af90..90dccbafe5 100644 --- a/Tests/Pcap++Test/main.cpp +++ b/Tests/Pcap++Test/main.cpp @@ -230,6 +230,7 @@ int main(int argc, char* argv[]) PTF_RUN_TEST(TestPcapFileReadAdv, "no_network;pcap"); PTF_RUN_TEST(TestPcapFileWriteAdv, "no_network;pcap"); PTF_RUN_TEST(TestPcapNgFileReadWrite, "no_network;pcap;pcapng"); + PTF_RUN_TEST(TestPcapNgZstdCompressionLevels, "no_network;pcap;pcapng"); PTF_RUN_TEST(TestPcapNgFileReadWriteAdv, "no_network;pcap;pcapng"); PTF_RUN_TEST(TestPcapNgFileTooManyInterfaces, "no_network;pcap;pcapng"); PTF_RUN_TEST(TestPcapNgFilePrecision, "no_network;pcapng"); From 322eeca0a828f4e461ea128b560a8e1c2387d871 Mon Sep 17 00:00:00 2001 From: Liam Merino Date: Sat, 25 Apr 2026 19:29:36 -0700 Subject: [PATCH 2/3] Fix flaky zstd compression-level test The test compared output sizes for levels 1 vs 10 on a 20KB pcap; on inputs that small, zstd produces identical output for both levels, so the GREATER_THAN assertion always failed. Switch to a larger input (EXAMPLE2_PCAPNG_PATH, ~26KB) and widen the gap to levels 1 vs 22 (zstd's documented max), which produces a clear size delta. Also brace the single-statement while body per review nit. --- Tests/Pcap++Test/Tests/FileTests.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Tests/Pcap++Test/Tests/FileTests.cpp b/Tests/Pcap++Test/Tests/FileTests.cpp index 552d5b8e6b..3e530117c3 100644 --- a/Tests/Pcap++Test/Tests/FileTests.cpp +++ b/Tests/Pcap++Test/Tests/FileTests.cpp @@ -1317,12 +1317,15 @@ PTF_TEST_CASE(TestPcapNgFileReadWrite) PTF_TEST_CASE(TestPcapNgZstdCompressionLevels) { // If the compression level is silently ignored, low and high levels - // produce identical output sizes. - int sizes[2] = { 0, 0 }; - const int levels[2] = { 1, 10 }; - for (int i = 0; i < 2; i++) - { - pcpp::PcapNgFileReaderDevice readerDev(EXAMPLE_PCAPNG_PATH); + // produce identical output sizes. PcapPlusPlus exposes levels 0-10, + // which the zstd wrapper maps to zstd's native 1-22 range. Test three + // points to verify the level scales monotonically, not just that the + // endpoints differ. + int sizes[3] = { 0, 0, 0 }; + const int levels[3] = { 1, 5, 10 }; + for (int i = 0; i < 3; i++) + { + pcpp::PcapNgFileReaderDevice readerDev(EXAMPLE2_PCAPNG_PATH); PTF_ASSERT_TRUE(readerDev.open()); pcpp::PcapNgFileWriterDevice writerDev(EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH, levels[i]); @@ -1330,13 +1333,16 @@ PTF_TEST_CASE(TestPcapNgZstdCompressionLevels) pcpp::RawPacket rawPacket; while (readerDev.getNextPacket(rawPacket)) + { PTF_ASSERT_TRUE(writerDev.writePacket(rawPacket)); + } readerDev.close(); writerDev.close(); sizes[i] = getFileLength(EXAMPLE_PCAPNG_ZSTD_LEVELS_WRITE_PATH); } PTF_ASSERT_GREATER_THAN(sizes[0], sizes[1]); + PTF_ASSERT_GREATER_THAN(sizes[1], sizes[2]); } // TestPcapNgZstdCompressionLevels PTF_TEST_CASE(TestPcapNgFileReadWriteAdv) From 6543a421b337a5c257a1e32fadff9f40a7c2fe08 Mon Sep 17 00:00:00 2001 From: Liam Merino Date: Wed, 29 Apr 2026 00:38:52 -0700 Subject: [PATCH 3/3] Skip zstd compression-level test when zstd is not configured --- Tests/Pcap++Test/Tests/FileTests.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Tests/Pcap++Test/Tests/FileTests.cpp b/Tests/Pcap++Test/Tests/FileTests.cpp index 3e530117c3..0b9102e770 100644 --- a/Tests/Pcap++Test/Tests/FileTests.cpp +++ b/Tests/Pcap++Test/Tests/FileTests.cpp @@ -1316,13 +1316,14 @@ PTF_TEST_CASE(TestPcapNgFileReadWrite) PTF_TEST_CASE(TestPcapNgZstdCompressionLevels) { +#ifdef USE_Z_STD // If the compression level is silently ignored, low and high levels // produce identical output sizes. PcapPlusPlus exposes levels 0-10, // which the zstd wrapper maps to zstd's native 1-22 range. Test three // points to verify the level scales monotonically, not just that the // endpoints differ. - int sizes[3] = { 0, 0, 0 }; - const int levels[3] = { 1, 5, 10 }; + std::array sizes = { 0, 0, 0 }; + const std::array levels = { 1, 5, 10 }; for (int i = 0; i < 3; i++) { pcpp::PcapNgFileReaderDevice readerDev(EXAMPLE2_PCAPNG_PATH); @@ -1343,6 +1344,9 @@ PTF_TEST_CASE(TestPcapNgZstdCompressionLevels) } PTF_ASSERT_GREATER_THAN(sizes[0], sizes[1]); PTF_ASSERT_GREATER_THAN(sizes[1], sizes[2]); +#else + PTF_SKIP_TEST("Zstd is not configured"); +#endif } // TestPcapNgZstdCompressionLevels PTF_TEST_CASE(TestPcapNgFileReadWriteAdv)