From 20aaab47f9dab40b37b85a1044ac43909ab50443 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 28 Mar 2023 11:28:04 -0400 Subject: [PATCH] LibCompress: Use a bit stream for the entire GZIP decompression process We currently mix normal and bit streams during GZIP decompression, where the latter is a wrapper around the former. This isn't causing issues now as the underlying bit stream buffer is a byte, so the normal stream can pick up where the bit stream left off. In order to increase the size of that buffer though, the normal stream will not be able to assume it can resume reading after the bit stream. The buffer can easily contain more bits than it was meant to read, so when the normal stream resumes, there may be N bits leftover in the bit stream that the normal stream was meant to read. To avoid weird behavior when mixing streams, this changes the GZIP decompressor to always read from a bit stream. --- Userland/Libraries/LibCompress/Deflate.cpp | 8 ++++---- Userland/Libraries/LibCompress/Deflate.h | 4 ++-- Userland/Libraries/LibCompress/Gzip.cpp | 7 ++++--- Userland/Libraries/LibCompress/Gzip.h | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Userland/Libraries/LibCompress/Deflate.cpp b/Userland/Libraries/LibCompress/Deflate.cpp index a5ac0d1c1d..817b36e1be 100644 --- a/Userland/Libraries/LibCompress/Deflate.cpp +++ b/Userland/Libraries/LibCompress/Deflate.cpp @@ -189,14 +189,14 @@ ErrorOr DeflateDecompressor::UncompressedBlock::try_read_more() return true; } -ErrorOr> DeflateDecompressor::construct(MaybeOwned stream) +ErrorOr> DeflateDecompressor::construct(MaybeOwned stream) { auto output_buffer = TRY(CircularBuffer::create_empty(32 * KiB)); return TRY(adopt_nonnull_own_or_enomem(new (nothrow) DeflateDecompressor(move(stream), move(output_buffer)))); } -DeflateDecompressor::DeflateDecompressor(MaybeOwned stream, CircularBuffer output_buffer) - : m_input_stream(make(move(stream))) +DeflateDecompressor::DeflateDecompressor(MaybeOwned stream, CircularBuffer output_buffer) + : m_input_stream(move(stream)) , m_output_buffer(move(output_buffer)) { } @@ -317,7 +317,7 @@ void DeflateDecompressor::close() ErrorOr DeflateDecompressor::decompress_all(ReadonlyBytes bytes) { auto memory_stream = TRY(try_make(bytes)); - auto deflate_stream = TRY(DeflateDecompressor::construct(move(memory_stream))); + auto deflate_stream = TRY(DeflateDecompressor::construct(make(move(memory_stream)))); AllocatingMemoryStream output_stream; auto buffer = TRY(ByteBuffer::create_uninitialized(4096)); diff --git a/Userland/Libraries/LibCompress/Deflate.h b/Userland/Libraries/LibCompress/Deflate.h index c62f1bd34e..feb6e088ec 100644 --- a/Userland/Libraries/LibCompress/Deflate.h +++ b/Userland/Libraries/LibCompress/Deflate.h @@ -76,7 +76,7 @@ public: friend CompressedBlock; friend UncompressedBlock; - static ErrorOr> construct(MaybeOwned stream); + static ErrorOr> construct(MaybeOwned stream); ~DeflateDecompressor(); virtual ErrorOr read_some(Bytes) override; @@ -88,7 +88,7 @@ public: static ErrorOr decompress_all(ReadonlyBytes); private: - DeflateDecompressor(MaybeOwned stream, CircularBuffer buffer); + DeflateDecompressor(MaybeOwned stream, CircularBuffer buffer); ErrorOr decode_length(u32); ErrorOr decode_distance(u32); diff --git a/Userland/Libraries/LibCompress/Gzip.cpp b/Userland/Libraries/LibCompress/Gzip.cpp index fce7a5fc09..10b6788b6b 100644 --- a/Userland/Libraries/LibCompress/Gzip.cpp +++ b/Userland/Libraries/LibCompress/Gzip.cpp @@ -7,6 +7,7 @@ #include +#include #include #include #include @@ -38,9 +39,9 @@ bool BlockHeader::supported_by_implementation() const return true; } -ErrorOr> GzipDecompressor::Member::construct(BlockHeader header, Stream& stream) +ErrorOr> GzipDecompressor::Member::construct(BlockHeader header, LittleEndianInputBitStream& stream) { - auto deflate_stream = TRY(DeflateDecompressor::construct(MaybeOwned(stream))); + auto deflate_stream = TRY(DeflateDecompressor::construct(MaybeOwned(stream))); return TRY(adopt_nonnull_own_or_enomem(new (nothrow) Member(header, move(deflate_stream)))); } @@ -51,7 +52,7 @@ GzipDecompressor::Member::Member(BlockHeader header, NonnullOwnPtr stream) - : m_input_stream(move(stream)) + : m_input_stream(make(move(stream))) { } diff --git a/Userland/Libraries/LibCompress/Gzip.h b/Userland/Libraries/LibCompress/Gzip.h index 85a33eae90..de5d103a9c 100644 --- a/Userland/Libraries/LibCompress/Gzip.h +++ b/Userland/Libraries/LibCompress/Gzip.h @@ -58,7 +58,7 @@ public: private: class Member { public: - static ErrorOr> construct(BlockHeader header, Stream&); + static ErrorOr> construct(BlockHeader header, LittleEndianInputBitStream&); BlockHeader m_header; NonnullOwnPtr m_stream; @@ -72,7 +72,7 @@ private: Member const& current_member() const { return *m_current_member; } Member& current_member() { return *m_current_member; } - NonnullOwnPtr m_input_stream; + NonnullOwnPtr m_input_stream; u8 m_partial_header[sizeof(BlockHeader)]; size_t m_partial_header_offset { 0 }; OwnPtr m_current_member {};