From 135daeb8bb2a1e113f684a0796ca4712711756b9 Mon Sep 17 00:00:00 2001 From: Valtteri Koskivuori Date: Mon, 18 Nov 2024 20:16:55 +0200 Subject: [PATCH] LibCompress: Don't assume zlib header is available right away Instead of checking the header in ZlibDecompressor::create(), we now check it in read_some() when it is called for the first time. This resolves a FIXME in the new DecompressionStream implementation. --- Libraries/LibCompress/Zlib.cpp | 38 ++++++++++--------- Libraries/LibCompress/Zlib.h | 6 +-- .../Compression/DecompressionStream.cpp | 2 - Tests/LibCompress/TestZlib.cpp | 19 ++++++++++ .../Compression/DecompressionStream.txt | 1 + .../Compression/DecompressionStream.html | 3 +- 6 files changed, 45 insertions(+), 24 deletions(-) diff --git a/Libraries/LibCompress/Zlib.cpp b/Libraries/LibCompress/Zlib.cpp index b6857da8adf..5edc542b1cb 100644 --- a/Libraries/LibCompress/Zlib.cpp +++ b/Libraries/LibCompress/Zlib.cpp @@ -17,31 +17,35 @@ namespace Compress { ErrorOr> ZlibDecompressor::create(MaybeOwned stream) { - auto header = TRY(stream->read_value()); - - if (header.compression_method != ZlibCompressionMethod::Deflate || header.compression_info > 7) - return Error::from_string_literal("Non-DEFLATE compression inside Zlib is not supported"); - - if (header.present_dictionary) - return Error::from_string_literal("Zlib compression with a pre-defined dictionary is currently not supported"); - - if (header.as_u16 % 31 != 0) - return Error::from_string_literal("Zlib error correction code does not match"); - - auto bit_stream = make(move(stream)); - auto deflate_stream = TRY(Compress::DeflateDecompressor::construct(move(bit_stream))); - - return adopt_nonnull_own_or_enomem(new (nothrow) ZlibDecompressor(header, move(deflate_stream))); + return adopt_nonnull_own_or_enomem(new (nothrow) ZlibDecompressor(move(stream))); } -ZlibDecompressor::ZlibDecompressor(ZlibHeader header, NonnullOwnPtr stream) - : m_header(header) +ZlibDecompressor::ZlibDecompressor(MaybeOwned stream) + : m_has_seen_header(false) , m_stream(move(stream)) { } ErrorOr ZlibDecompressor::read_some(Bytes bytes) { + if (!m_has_seen_header) { + auto header = TRY(m_stream->read_value()); + + if (header.compression_method != ZlibCompressionMethod::Deflate || header.compression_info > 7) + return Error::from_string_literal("Non-DEFLATE compression inside Zlib is not supported"); + + if (header.present_dictionary) + return Error::from_string_literal("Zlib compression with a pre-defined dictionary is currently not supported"); + + if (header.as_u16 % 31 != 0) + return Error::from_string_literal("Zlib error correction code does not match"); + + auto bit_stream = make(move(m_stream)); + auto deflate_stream = TRY(Compress::DeflateDecompressor::construct(move(bit_stream))); + + m_stream = move(deflate_stream); + m_has_seen_header = true; + } return m_stream->read_some(bytes); } diff --git a/Libraries/LibCompress/Zlib.h b/Libraries/LibCompress/Zlib.h index 478ead7f24e..13d2e24be9c 100644 --- a/Libraries/LibCompress/Zlib.h +++ b/Libraries/LibCompress/Zlib.h @@ -55,10 +55,10 @@ public: virtual void close() override; private: - ZlibDecompressor(ZlibHeader, NonnullOwnPtr); + ZlibDecompressor(MaybeOwned); - ZlibHeader m_header; - NonnullOwnPtr m_stream; + bool m_has_seen_header { false }; + MaybeOwned m_stream; }; class ZlibCompressor : public Stream { diff --git a/Libraries/LibWeb/Compression/DecompressionStream.cpp b/Libraries/LibWeb/Compression/DecompressionStream.cpp index 64169442cee..a48b7bf3750 100644 --- a/Libraries/LibWeb/Compression/DecompressionStream.cpp +++ b/Libraries/LibWeb/Compression/DecompressionStream.cpp @@ -32,8 +32,6 @@ WebIDL::ExceptionOr> DecompressionStream::construct auto decompressor = [&, input_stream = MaybeOwned { *input_stream }]() mutable -> ErrorOr { switch (format) { case Bindings::CompressionFormat::Deflate: - // FIXME: Our zlib decompressor assumes the initial data contains the zlib header. We don't have any data yet, - // so this will always fail. return TRY(Compress::ZlibDecompressor::create(move(input_stream))); case Bindings::CompressionFormat::DeflateRaw: return TRY(Compress::DeflateDecompressor::construct(make(move(input_stream)))); diff --git a/Tests/LibCompress/TestZlib.cpp b/Tests/LibCompress/TestZlib.cpp index a98aaf57bae..5d58ae0c90b 100644 --- a/Tests/LibCompress/TestZlib.cpp +++ b/Tests/LibCompress/TestZlib.cpp @@ -27,6 +27,25 @@ TEST_CASE(zlib_decompress_simple) EXPECT(decompressed.bytes() == (ReadonlyBytes { uncompressed, sizeof(uncompressed) - 1 })); } +TEST_CASE(zlib_decompress_stream) +{ + Array const compressed { + 0x78, 0x01, 0x01, 0x1D, 0x00, 0xE2, 0xFF, 0x54, 0x68, 0x69, 0x73, 0x20, + 0x69, 0x73, 0x20, 0x61, 0x20, 0x73, 0x69, 0x6D, 0x70, 0x6C, 0x65, 0x20, + 0x74, 0x65, 0x78, 0x74, 0x20, 0x66, 0x69, 0x6C, 0x65, 0x20, 0x3A, 0x29, + 0x99, 0x5E, 0x09, 0xE8 + }; + + u8 const uncompressed[] = "This is a simple text file :)"; + + auto stream = make(); + auto input = MaybeOwned { *stream }; + auto decompressor = TRY_OR_FAIL(Compress::ZlibDecompressor::create(move(input))); + TRY_OR_FAIL(stream->write_until_depleted(compressed)); + auto decompressed = TRY_OR_FAIL(decompressor->read_until_eof()); + EXPECT(decompressed.bytes() == (ReadonlyBytes { uncompressed, sizeof(uncompressed) - 1 })); +} + TEST_CASE(zlib_compress_simple) { // Note: This is just the output of our compression function from an arbitrary point in time. diff --git a/Tests/LibWeb/Text/expected/Compression/DecompressionStream.txt b/Tests/LibWeb/Text/expected/Compression/DecompressionStream.txt index c6c19723d88..1a24f6aa35a 100644 --- a/Tests/LibWeb/Text/expected/Compression/DecompressionStream.txt +++ b/Tests/LibWeb/Text/expected/Compression/DecompressionStream.txt @@ -1,2 +1,3 @@ +format=deflate: Well hello friends! format=deflate-raw: Well hello friends! format=gzip: Well hello friends! diff --git a/Tests/LibWeb/Text/input/Compression/DecompressionStream.html b/Tests/LibWeb/Text/input/Compression/DecompressionStream.html index fa01e15842c..116682e7d14 100644 --- a/Tests/LibWeb/Text/input/Compression/DecompressionStream.html +++ b/Tests/LibWeb/Text/input/Compression/DecompressionStream.html @@ -2,8 +2,7 @@