Prechádzať zdrojové kódy

LibCompress: Move XZ header validation into the read function

The constructor is now only concerned with creating the required
streams, which means that it no longer fails for XZ streams with
invalid headers. Instead, everything is parsed and validated during the
first read, preparing us for files with multiple streams.
Tim Schumacher 2 rokov pred
rodič
commit
00332c9b7d

+ 6 - 4
Tests/LibCompress/TestXz.cpp

@@ -118,8 +118,9 @@ TEST_CASE(xz_utils_bad_0_header_magic)
     };
     };
 
 
     auto stream = MUST(try_make<FixedMemoryStream>(compressed));
     auto stream = MUST(try_make<FixedMemoryStream>(compressed));
-    auto decompressor_or_error = Compress::XzDecompressor::create(move(stream));
-    EXPECT(decompressor_or_error.is_error());
+    auto decompressor = MUST(Compress::XzDecompressor::create(move(stream)));
+    auto buffer_or_error = decompressor->read_until_eof(PAGE_SIZE);
+    EXPECT(buffer_or_error.is_error());
 }
 }
 
 
 TEST_CASE(xz_utils_bad_0_nonempty_index)
 TEST_CASE(xz_utils_bad_0_nonempty_index)
@@ -695,8 +696,9 @@ TEST_CASE(xz_utils_bad_1_stream_flags_2)
     };
     };
 
 
     auto stream = MUST(try_make<FixedMemoryStream>(compressed));
     auto stream = MUST(try_make<FixedMemoryStream>(compressed));
-    auto decompressor_or_error = Compress::XzDecompressor::create(move(stream));
-    EXPECT(decompressor_or_error.is_error());
+    auto decompressor = MUST(Compress::XzDecompressor::create(move(stream)));
+    auto buffer_or_error = decompressor->read_until_eof(PAGE_SIZE);
+    EXPECT(buffer_or_error.is_error());
 }
 }
 
 
 TEST_CASE(xz_utils_bad_1_stream_flags_3)
 TEST_CASE(xz_utils_bad_1_stream_flags_3)

+ 11 - 8
Userland/Libraries/LibCompress/Xz.cpp

@@ -165,17 +165,13 @@ ErrorOr<NonnullOwnPtr<XzDecompressor>> XzDecompressor::create(MaybeOwned<Stream>
 {
 {
     auto counting_stream = TRY(try_make<CountingStream>(move(stream)));
     auto counting_stream = TRY(try_make<CountingStream>(move(stream)));
 
 
-    auto stream_header = TRY(counting_stream->read_value<XzStreamHeader>());
-    TRY(stream_header.validate());
-
-    auto decompressor = TRY(adopt_nonnull_own_or_enomem(new (nothrow) XzDecompressor(move(counting_stream), stream_header.flags)));
+    auto decompressor = TRY(adopt_nonnull_own_or_enomem(new (nothrow) XzDecompressor(move(counting_stream))));
 
 
     return decompressor;
     return decompressor;
 }
 }
 
 
-XzDecompressor::XzDecompressor(NonnullOwnPtr<CountingStream> stream, XzStreamFlags stream_flags)
+XzDecompressor::XzDecompressor(NonnullOwnPtr<CountingStream> stream)
     : m_stream(move(stream))
     : m_stream(move(stream))
-    , m_stream_flags(stream_flags)
 {
 {
 }
 }
 
 
@@ -184,6 +180,13 @@ ErrorOr<Bytes> XzDecompressor::read_some(Bytes bytes)
     if (m_found_stream_footer)
     if (m_found_stream_footer)
         return bytes.trim(0);
         return bytes.trim(0);
 
 
+    if (!m_stream_flags.has_value()) {
+        auto stream_header = TRY(m_stream->read_value<XzStreamHeader>());
+        TRY(stream_header.validate());
+
+        m_stream_flags = stream_header.flags;
+    }
+
     if (!m_current_block_stream.has_value() || (*m_current_block_stream)->is_eof()) {
     if (!m_current_block_stream.has_value() || (*m_current_block_stream)->is_eof()) {
         if (m_current_block_stream.has_value()) {
         if (m_current_block_stream.has_value()) {
             // We have already processed a block, so we weed to clean up trailing data before the next block starts.
             // We have already processed a block, so we weed to clean up trailing data before the next block starts.
@@ -212,7 +215,7 @@ ErrorOr<Bytes> XzDecompressor::read_some(Bytes bytes)
             //  indicate a warning or error."
             //  indicate a warning or error."
             // TODO: Block content checks are currently unimplemented as a whole, independent of the check type.
             // TODO: Block content checks are currently unimplemented as a whole, independent of the check type.
             //       For now, we only make sure to remove the correct amount of bytes from the stream.
             //       For now, we only make sure to remove the correct amount of bytes from the stream.
-            switch (m_stream_flags.check_type) {
+            switch (m_stream_flags->check_type) {
             case XzStreamCheckType::None:
             case XzStreamCheckType::None:
                 break;
                 break;
             case XzStreamCheckType::CRC32:
             case XzStreamCheckType::CRC32:
@@ -329,7 +332,7 @@ ErrorOr<Bytes> XzDecompressor::read_some(Bytes bytes)
             //  when parsing the Stream backwards. The decoder MUST compare
             //  when parsing the Stream backwards. The decoder MUST compare
             //  the Stream Flags fields in both Stream Header and Stream
             //  the Stream Flags fields in both Stream Header and Stream
             //  Footer, and indicate an error if they are not identical."
             //  Footer, and indicate an error if they are not identical."
-            if (Bytes { &m_stream_flags, sizeof(m_stream_flags) } != Bytes { &stream_footer.flags, sizeof(stream_footer.flags) })
+            if (Bytes { &*m_stream_flags, sizeof(XzStreamFlags) } != Bytes { &stream_footer.flags, sizeof(stream_footer.flags) })
                 return Error::from_string_literal("XZ stream header flags don't match the stream footer");
                 return Error::from_string_literal("XZ stream header flags don't match the stream footer");
 
 
             m_found_stream_footer = true;
             m_found_stream_footer = true;

+ 2 - 2
Userland/Libraries/LibCompress/Xz.h

@@ -108,10 +108,10 @@ public:
     virtual void close() override;
     virtual void close() override;
 
 
 private:
 private:
-    XzDecompressor(NonnullOwnPtr<CountingStream>, XzStreamFlags);
+    XzDecompressor(NonnullOwnPtr<CountingStream>);
 
 
     NonnullOwnPtr<CountingStream> m_stream;
     NonnullOwnPtr<CountingStream> m_stream;
-    XzStreamFlags m_stream_flags;
+    Optional<XzStreamFlags> m_stream_flags;
     bool m_found_stream_footer { false };
     bool m_found_stream_footer { false };
 
 
     Optional<MaybeOwned<Stream>> m_current_block_stream {};
     Optional<MaybeOwned<Stream>> m_current_block_stream {};