From 90c364d935b060d5413624a3c2dc413883047634 Mon Sep 17 00:00:00 2001 From: Jeremi Maciejewski Date: Thu, 24 Jul 2025 11:29:31 +0200 Subject: [PATCH 1/2] Throw out overload of uncompressString which used QByteArray; update documentation; update tests; add exceptions on unsuccessful decompression. --- .../include/OpenMS/FORMAT/ZlibCompression.h | 15 ++-- src/openms/source/FORMAT/ZlibCompression.cpp | 31 ++++---- .../openms/source/ZlibCompression_test.cpp | 71 +++++++++---------- 3 files changed, 54 insertions(+), 63 deletions(-) diff --git a/src/openms/include/OpenMS/FORMAT/ZlibCompression.h b/src/openms/include/OpenMS/FORMAT/ZlibCompression.h index 6e5819a562b..79aec918f42 100644 --- a/src/openms/include/OpenMS/FORMAT/ZlibCompression.h +++ b/src/openms/include/OpenMS/FORMAT/ZlibCompression.h @@ -64,16 +64,21 @@ namespace OpenMS /** * @brief Uncompresses data using zlib + * Compared to variant of this function, which does not use output_size parameter, this is faster and preferred. + * Be aware that this function preallocates amount of memory corresponding to ouput_size. * * @param compressed_data Compressed data * @param nr_bytes Number of bytes in compressed data * @param raw_data Uncompressed result data + * @param output_size Size of uncompressed data * */ static void uncompressString(const void * compressed_data, size_t nr_bytes, std::string& raw_data, size_t output_size); /** * @brief Uncompresses data using zlib + * When size of data after decompression is known, it is recommended to use another variant of this function, which allows specifying output size. + * Does not support gzip format decompression. * * @param compressed_data Compressed data * @param nr_bytes Number of bytes in compressed data @@ -81,16 +86,6 @@ namespace OpenMS * */ static void uncompressString(const void * compressed_data, size_t nr_bytes, std::string& raw_data); - - /** - * @brief Uncompresses data using zlib - * - * @param compressed_data Compressed data - * @param raw_data Uncompressed result data - * - */ - static void uncompressString(const QByteArray& compressed_data, QByteArray& raw_data); - }; } // namespace OpenMS diff --git a/src/openms/source/FORMAT/ZlibCompression.cpp b/src/openms/source/FORMAT/ZlibCompression.cpp index 3131979bfe3..9a71d9cbcef 100644 --- a/src/openms/source/FORMAT/ZlibCompression.cpp +++ b/src/openms/source/FORMAT/ZlibCompression.cpp @@ -69,13 +69,17 @@ namespace OpenMS if (ret == Z_OK) { if (uncompressedSize != raw_data.size()) - { - OPENMS_LOG_INFO << "zlib::uncompress: data was smaller than anticipated.\n"; + { + OPENMS_LOG_INFO << "ZlibCompression::uncompressString: Warning: decompressed data was smaller (" << std::to_string(uncompressedSize) << ") than anticipated: " << std::to_string(output_size) << std::endl; raw_data.resize(output_size); } } - else { - std::cerr << "Zlib::uncompress() failed with code: " << ret << " and expected output size: " << output_size << std::endl; + else if (ret == Z_BUF_ERROR) + { + throw Exception::InvalidValue(__FILE__, __LINE__, OPENMS_PRETTY_FUNCTION, "Decompression failed because specified output_size was too small. Size of data after decompression is larger than anticipated: " + std::to_string(uncompressedSize), std::to_string(output_size)); + } else + { + throw Exception::InternalToolError(__FILE__, __LINE__, OPENMS_PRETTY_FUNCTION, "zlib::inflate failed with code " + std::to_string(ret) + " ."); } } @@ -90,12 +94,17 @@ namespace OpenMS strm.next_in = (Bytef*)(compressed_data); strm.avail_in = nr_bytes; + int ret; + // Initialize zlib (use inflateInit2 for gzip or raw deflate) - if (inflateInit(&strm) != Z_OK) { throw std::runtime_error("inflateInit failed"); } + ret = inflateInit(&strm); + if (ret != Z_OK) + { + throw Exception::InternalToolError(__FILE__, __LINE__, OPENMS_PRETTY_FUNCTION, "zlib::inflateInit failed with code " + std::to_string(ret) + " ."); + } // Decompress loop std::array buffer; - int ret; do { @@ -106,7 +115,7 @@ namespace OpenMS if (ret == Z_STREAM_ERROR || ret == Z_DATA_ERROR || ret == Z_MEM_ERROR) { inflateEnd(&strm); - throw std::runtime_error("inflate failed"); + throw Exception::InternalToolError(__FILE__, __LINE__, OPENMS_PRETTY_FUNCTION, "zlib::inflate failed with code " + std::to_string(ret) + " ."); } size_t bytesDecompressed = CHUNK_SIZE - strm.avail_out; @@ -116,13 +125,5 @@ namespace OpenMS inflateEnd(&strm); } - - void ZlibCompression::uncompressString(const QByteArray& compressed_data, QByteArray& raw_data) - { - std::string uncompressed; - uncompressString(compressed_data.constData(), compressed_data.size(), uncompressed); - raw_data = QByteArray::fromRawData(uncompressed.data(), static_cast(uncompressed.size())); - } - } diff --git a/src/tests/class_tests/openms/source/ZlibCompression_test.cpp b/src/tests/class_tests/openms/source/ZlibCompression_test.cpp index 3de5597fc17..27c31b4c855 100644 --- a/src/tests/class_tests/openms/source/ZlibCompression_test.cpp +++ b/src/tests/class_tests/openms/source/ZlibCompression_test.cpp @@ -106,78 +106,73 @@ START_SECTION((static void compressString(const QByteArray& raw_data, QByteArray } END_SECTION -START_SECTION((static void uncompressString(const void * compressed_data, size_t nr_bytes, std::string& raw_data))) +START_SECTION((static void uncompressString(const void* compressed_data, size_t nr_bytes, std::string& raw_data, size_t output_size))) { std::string compressed_data; std::string uncompressed_data; ZlibCompression::compressString(raw_data, compressed_data); - ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data); + ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data, raw_data.size()); TEST_EQUAL(raw_data.size(), 58) - TEST_TRUE(compressed_data.size() < raw_data.size()) + TEST_TRUE(compressed_data.size() < raw_data.size()) TEST_EQUAL(uncompressed_data.size(), 58) TEST_TRUE(uncompressed_data == raw_data) ZlibCompression::compressString(raw_data2, compressed_data); - ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data); + ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data, raw_data2.size()); TEST_EQUAL(raw_data2.size(), 64) - TEST_TRUE(compressed_data.size() >= raw_data2.size()) // "ABCD..." string is difficult to compress + TEST_TRUE(compressed_data.size() >= raw_data2.size()) // difficult to compress... TEST_EQUAL(uncompressed_data.size(), 64) TEST_TRUE(uncompressed_data == raw_data2) ZlibCompression::compressString(raw_data3, compressed_data); - ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data); + ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data, raw_data3.size()); TEST_EQUAL(raw_data3.size(), 105) - TEST_TRUE(compressed_data.size() < raw_data3.size()) + TEST_TRUE(compressed_data.size() < raw_data3.size()) TEST_EQUAL(uncompressed_data.size(), 105) TEST_TRUE(uncompressed_data == raw_data3) ZlibCompression::compressString(raw_data4, compressed_data); - ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data); + ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data, raw_data4.size()); TEST_EQUAL(raw_data4.size(), 1052) - TEST_TRUE(compressed_data.size() < raw_data4.size()) + TEST_TRUE(compressed_data.size() < raw_data4.size()) TEST_EQUAL(uncompressed_data.size(), 1052) TEST_TRUE(uncompressed_data == raw_data4) } END_SECTION - -START_SECTION((static void uncompressString(const QByteArray& compressed_data, QByteArray& raw_data))) -{ - QByteArray raw_data_q = QByteArray::fromRawData(&raw_data[0], raw_data.size()); - QByteArray raw_data_q2 = QByteArray::fromRawData(&raw_data2[0], raw_data2.size()); - QByteArray raw_data_q3 = QByteArray::fromRawData(&raw_data3[0], raw_data3.size()); - QByteArray raw_data_q4 = QByteArray::fromRawData(&raw_data4[0], raw_data4.size()); - QByteArray compressed_data; - QByteArray uncompressed_data; +START_SECTION((static void uncompressString(const void * compressed_data, size_t nr_bytes, std::string& raw_data))) +{ + std::string compressed_data; + std::string uncompressed_data; - ZlibCompression::compressString(raw_data_q, compressed_data); - ZlibCompression::uncompressString(compressed_data, uncompressed_data); - TEST_EQUAL(raw_data_q.size(), 58) - TEST_TRUE(compressed_data.size() < raw_data_q.size()) + ZlibCompression::compressString(raw_data, compressed_data); + ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data); + TEST_EQUAL(raw_data.size(), 58) + TEST_TRUE(compressed_data.size() < raw_data.size()) TEST_EQUAL(uncompressed_data.size(), 58) - TEST_TRUE(uncompressed_data == raw_data_q) + TEST_TRUE(uncompressed_data == raw_data) - ZlibCompression::compressString(raw_data_q2, compressed_data); - ZlibCompression::uncompressString(compressed_data, uncompressed_data); - TEST_EQUAL(raw_data_q2.size(), 64) - TEST_TRUE(compressed_data.size() >= raw_data_q2.size()) // difficult to compress... + ZlibCompression::compressString(raw_data2, compressed_data); + ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data); + TEST_EQUAL(raw_data2.size(), 64) + TEST_TRUE(compressed_data.size() >= raw_data2.size()) // "ABCD..." string is difficult to compress TEST_EQUAL(uncompressed_data.size(), 64) - TEST_TRUE(uncompressed_data == raw_data_q2) + TEST_TRUE(uncompressed_data == raw_data2) - ZlibCompression::compressString(raw_data_q3, compressed_data); - ZlibCompression::uncompressString(compressed_data, uncompressed_data); - TEST_EQUAL(raw_data_q3.size(), 105) - TEST_TRUE(compressed_data.size() < raw_data_q3.size()) + ZlibCompression::compressString(raw_data3, compressed_data); + ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data); + TEST_EQUAL(raw_data3.size(), 105) + TEST_TRUE(compressed_data.size() < raw_data3.size()) TEST_EQUAL(uncompressed_data.size(), 105) - TEST_TRUE(uncompressed_data == raw_data_q3) + TEST_TRUE(uncompressed_data == raw_data3) - ZlibCompression::compressString(raw_data_q4, compressed_data); - ZlibCompression::uncompressString(compressed_data, uncompressed_data); - TEST_EQUAL(raw_data_q4.size(), 1052) - TEST_TRUE(compressed_data.size() < raw_data_q4.size()) + ZlibCompression::compressString(raw_data4, compressed_data); + ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data); + TEST_EQUAL(raw_data4.size(), 1052) + TEST_TRUE(compressed_data.size() < raw_data4.size()) TEST_EQUAL(uncompressed_data.size(), 1052) - TEST_TRUE(uncompressed_data == raw_data_q4) + TEST_TRUE(uncompressed_data == raw_data4) } END_SECTION From c2ce9fcdc1b0f74371352d826971100313efb798 Mon Sep 17 00:00:00 2001 From: Jeremi Maciejewski Date: Thu, 24 Jul 2025 15:59:21 +0200 Subject: [PATCH 2/2] Update documentation; add tests --- src/openms/include/OpenMS/FORMAT/ZlibCompression.h | 3 +++ .../class_tests/openms/source/ZlibCompression_test.cpp | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/src/openms/include/OpenMS/FORMAT/ZlibCompression.h b/src/openms/include/OpenMS/FORMAT/ZlibCompression.h index 79aec918f42..f3402a73ed3 100644 --- a/src/openms/include/OpenMS/FORMAT/ZlibCompression.h +++ b/src/openms/include/OpenMS/FORMAT/ZlibCompression.h @@ -71,6 +71,8 @@ namespace OpenMS * @param nr_bytes Number of bytes in compressed data * @param raw_data Uncompressed result data * @param output_size Size of uncompressed data + * @throws Exception::InvalidValue if output_size specified turns out to be smaller than actual size of uncompressed data. + * @throws Exception::InternalToolError if zlib tools used by function fail for unknown reason. * */ static void uncompressString(const void * compressed_data, size_t nr_bytes, std::string& raw_data, size_t output_size); @@ -83,6 +85,7 @@ namespace OpenMS * @param compressed_data Compressed data * @param nr_bytes Number of bytes in compressed data * @param raw_data Uncompressed result data + * @throws Exception::InternalToolError if zlib tools used by function fail for unknown reason. This might happen if decompression from unsupported gzip format is attempted. * */ static void uncompressString(const void * compressed_data, size_t nr_bytes, std::string& raw_data); diff --git a/src/tests/class_tests/openms/source/ZlibCompression_test.cpp b/src/tests/class_tests/openms/source/ZlibCompression_test.cpp index 27c31b4c855..684d97bff7d 100644 --- a/src/tests/class_tests/openms/source/ZlibCompression_test.cpp +++ b/src/tests/class_tests/openms/source/ZlibCompression_test.cpp @@ -138,6 +138,15 @@ START_SECTION((static void uncompressString(const void* compressed_data, size_t TEST_TRUE(compressed_data.size() < raw_data4.size()) TEST_EQUAL(uncompressed_data.size(), 1052) TEST_TRUE(uncompressed_data == raw_data4) + + //////////////// + // Exceptions // + //////////////// + ZlibCompression::compressString(raw_data, compressed_data); + // Invalid output_size + TEST_EXCEPTION(Exception::InvalidValue, ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size(), uncompressed_data, 10);) + // Truncated data + TEST_EXCEPTION(Exception::InternalToolError, ZlibCompression::uncompressString(&compressed_data[0], compressed_data.size()-10, uncompressed_data, raw_data.size());) } END_SECTION