LibArchive: Use Core::Stream inside TarInputStream

Meta/Lagom/Fuzzers/FuzzTar.cpp

@@ -4,23 +4,35 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <AK/MemoryStream.h>
+#include <AK/NonnullOwnPtr.h>
 #include <LibArchive/TarStream.h>
+#include <LibCore/MemoryStream.h>
+#include <LibCore/Stream.h>
 #include <stdio.h>
 
 extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
 {
-    InputMemoryStream input_stream(ReadonlyBytes { data, size });
+    // FIXME: Create a ReadonlyBytes variant of Core::Stream::MemoryStream.
-    Archive::TarInputStream tar_stream(input_stream);
+    auto input_stream_or_error = Core::Stream::MemoryStream::construct(Bytes { const_cast<uint8_t*>(data), size });
 
-    if (!tar_stream.valid())
+    if (input_stream_or_error.is_error())
         return 0;
 
-    while (!tar_stream.finished()) {
+    auto tar_stream_or_error = Archive::TarInputStream::construct(input_stream_or_error.release_value());
-        auto const& header = tar_stream.header();
+
+    if (tar_stream_or_error.is_error())
+        return 0;
+
+    auto tar_stream = tar_stream_or_error.release_value();
+
+    if (!tar_stream->valid())
+        return 0;
+
+    while (!tar_stream->finished()) {
+        auto const& header = tar_stream->header();
 
         if (!header.content_is_like_extended_header()) {
-            if (tar_stream.advance().is_error())
+            if (tar_stream->advance().is_error())
                 return 0;
             else
                 continue;
@@ -29,7 +41,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
         switch (header.type_flag()) {
         case Archive::TarFileType::GlobalExtendedHeader:
         case Archive::TarFileType::ExtendedHeader: {
-            auto result = tar_stream.for_each_extended_header([&](StringView, StringView) {});
+            auto result = tar_stream->for_each_extended_header([&](StringView, StringView) {});
             if (result.is_error())
                 return 0;
             break;
@@ -38,7 +50,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
             return 0;
         }
 
-        if (tar_stream.advance().is_error())
+        if (tar_stream->advance().is_error())
             return 0;
     }
 

Userland/Libraries/LibArchive/TarStream.cpp

@@ -26,10 +26,10 @@ ErrorOr<Bytes> TarFileStream::read(Bytes bytes)
 
     auto to_read = min(bytes.size(), header_size - m_tar_stream.m_file_offset);
 
-    auto nread = m_tar_stream.m_stream.read(bytes.trim(to_read));
+    auto slice = TRY(m_tar_stream.m_stream->read(bytes.trim(to_read)));
-    m_tar_stream.m_file_offset += nread;
+    m_tar_stream.m_file_offset += slice.size();
 
-    return bytes.slice(0, nread);
+    return slice;
 }
 
 bool TarFileStream::is_eof() const
@@ -42,7 +42,7 @@ bool TarFileStream::is_eof() const
         return true;
     auto header_size = header_size_or_error.release_value();
 
-    return m_tar_stream.m_stream.unreliable_eof()
+    return m_tar_stream.m_stream->is_eof()
         || m_tar_stream.m_file_offset >= header_size;
 }
 
@@ -52,14 +52,24 @@ ErrorOr<size_t> TarFileStream::write(ReadonlyBytes)
     VERIFY_NOT_REACHED();
 }
 
-TarInputStream::TarInputStream(InputStream& stream)
+ErrorOr<NonnullOwnPtr<TarInputStream>> TarInputStream::construct(NonnullOwnPtr<Core::Stream::Stream> stream)
-    : m_stream(stream)
+{
+    auto tar_stream = TRY(adopt_nonnull_own_or_enomem(new (nothrow) TarInputStream(move(stream))));
+
+    // Try and read the header.
+    auto header_span = TRY(tar_stream->m_stream->read(Bytes(&tar_stream->m_header, sizeof(m_header))));
+    if (header_span.size() != sizeof(m_header))
+        return Error::from_string_literal("Failed to read the entire header");
+
+    // Discard the rest of the block.
+    TRY(tar_stream->m_stream->discard(block_size - sizeof(TarFileHeader)));
+
+    return tar_stream;
+}
+
+TarInputStream::TarInputStream(NonnullOwnPtr<Core::Stream::Stream> stream)
+    : m_stream(move(stream))
 {
-    if (!m_stream.read_or_error(Bytes(&m_header, sizeof(m_header))) || !m_stream.discard_or_error(block_size - sizeof(TarFileHeader))) {
-        m_finished = true;
-        m_stream.handle_any_error(); // clear out errors so we don't assert
-        return;
-    }
 }
 
 static constexpr unsigned long block_ceiling(unsigned long offset)
@@ -69,26 +79,27 @@ static constexpr unsigned long block_ceiling(unsigned long offset)
 
 ErrorOr<void> TarInputStream::advance()
 {
-    if (m_finished)
+    if (finished())
-        return Error::from_string_literal("Attempted to read a finished stream");
+        return Error::from_string_literal("Attempted to advance a finished stream");
 
     m_generation++;
 
-    auto header_size = TRY(m_header.size());
+    // Discard the pending bytes of the current entry.
-    VERIFY(m_stream.discard_or_error(block_ceiling(header_size) - m_file_offset));
+    auto file_size = TRY(m_header.size());
+    TRY(m_stream->discard(block_ceiling(file_size) - m_file_offset));
     m_file_offset = 0;
 
-    if (!m_stream.read_or_error(Bytes(&m_header, sizeof(m_header)))) {
+    // FIXME: This is not unlike the initial initialization. Maybe we should merge those two.
-        m_finished = true;
+    auto header_span = TRY(m_stream->read(Bytes(&m_header, sizeof(m_header))));
-        m_stream.handle_any_error(); // clear out errors so we don't assert
+    if (header_span.size() != sizeof(m_header))
-        return Error::from_string_literal("Failed to read the header");
+        return Error::from_string_literal("Failed to read the entire header");
-    }
+
-    if (!valid()) {
+    if (!valid())
-        m_finished = true;
+        return Error::from_string_literal("Header is not valid");
-        return {};
+
-    }
+    // Discard the rest of the header block.
+    TRY(m_stream->discard(block_size - sizeof(TarFileHeader)));
 
-    VERIFY(m_stream.discard_or_error(block_size - sizeof(TarFileHeader)));
     return {};
 }
 
@@ -111,7 +122,7 @@ bool TarInputStream::valid() const
 
 TarFileStream TarInputStream::file_contents()
 {
-    VERIFY(!m_finished);
+    VERIFY(!finished());
     return TarFileStream(*this);
 }
 

Userland/Libraries/LibArchive/TarStream.h

@@ -33,9 +33,9 @@ private:
 
 class TarInputStream {
 public:
-    TarInputStream(InputStream&);
+    static ErrorOr<NonnullOwnPtr<TarInputStream>> construct(NonnullOwnPtr<Core::Stream::Stream>);
     ErrorOr<void> advance();
-    bool finished() const { return m_finished; }
+    bool finished() const { return m_stream->is_eof(); }
     bool valid() const;
     TarFileHeader const& header() const { return m_header; }
     TarFileStream file_contents();
@@ -44,11 +44,12 @@ public:
     ErrorOr<void> for_each_extended_header(F func);
 
 private:
+    TarInputStream(NonnullOwnPtr<Core::Stream::Stream>);
+
     TarFileHeader m_header;
-    InputStream& m_stream;
+    NonnullOwnPtr<Core::Stream::Stream> m_stream;
     unsigned long m_file_offset { 0 };
     int m_generation { 0 };
-    bool m_finished { false };
 
     friend class TarFileStream;
 };

Userland/Utilities/tar.cpp

@@ -15,6 +15,7 @@
 #include <LibCore/DirIterator.h>
 #include <LibCore/File.h>
 #include <LibCore/FileStream.h>
+#include <LibCore/Stream.h>
 #include <LibCore/System.h>
 #include <LibMain/Main.h>
 #include <fcntl.h>
@@ -61,22 +62,34 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     }
 
     if (list || extract) {
-        auto file = Core::File::standard_input();
-
-        if (!archive_file.is_empty())
-            file = TRY(Core::File::open(archive_file, Core::OpenMode::ReadOnly));
-
         if (!directory.is_empty())
             TRY(Core::System::chdir(directory));
 
-        Core::InputFileStream file_stream(file);
+        // FIXME: Remove these once we have smart pointers everywhere in LibArchive and LibCompress (or just ported the whole stack to Core::Stream).
-        Compress::GzipDecompressor gzip_stream(file_stream);
+        // Until then, we have to hold on to _some_ instance of the file AK::Stream.
+        // Note that this is only in use together with gzip.
+        OwnPtr<Core::InputFileStream> file_stream;
 
-        InputStream& file_input_stream = file_stream;
+        auto input_stream = TRY([&]() -> ErrorOr<NonnullOwnPtr<Core::Stream::Stream>> {
-        InputStream& gzip_input_stream = gzip_stream;
+            if (gzip) {
-        Archive::TarInputStream tar_stream((gzip) ? gzip_input_stream : file_input_stream);
+                // FIXME: Port gzip to Core::Stream.
+                auto file = Core::File::standard_input();
+
+                if (!archive_file.is_empty())
+                    file = TRY(Core::File::open(archive_file, Core::OpenMode::ReadOnly));
+
+                file_stream = adopt_own(*new Core::InputFileStream(file));
+                NonnullOwnPtr<InputStream> gzip_stream = make<Compress::GzipDecompressor>(*file_stream);
+
+                return make<Core::Stream::WrappedAKInputStream>(move(gzip_stream));
+            } else {
+                return TRY(Core::Stream::File::open_file_or_standard_stream(archive_file, Core::Stream::OpenMode::Read));
+            }
+        }());
+
+        auto tar_stream = TRY(Archive::TarInputStream::construct(move(input_stream)));
         // FIXME: implement ErrorOr<TarInputStream>?
-        if (!tar_stream.valid()) {
+        if (!tar_stream->valid()) {
             warnln("the provided file is not a well-formatted ustar file");
             return 1;
         }
@@ -98,14 +111,14 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
             return {};
         };
 
-        while (!tar_stream.finished()) {
+        while (!tar_stream->finished()) {
-            Archive::TarFileHeader const& header = tar_stream.header();
+            Archive::TarFileHeader const& header = tar_stream->header();
 
             // Handle meta-entries earlier to avoid consuming the file content stream.
             if (header.content_is_like_extended_header()) {
                 switch (header.type_flag()) {
                 case Archive::TarFileType::GlobalExtendedHeader: {
-                    TRY(tar_stream.for_each_extended_header([&](StringView key, StringView value) {
+                    TRY(tar_stream->for_each_extended_header([&](StringView key, StringView value) {
                         if (value.length() == 0)
                             global_overrides.remove(key);
                         else
@@ -114,7 +127,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
                     break;
                 }
                 case Archive::TarFileType::ExtendedHeader: {
-                    TRY(tar_stream.for_each_extended_header([&](StringView key, StringView value) {
+                    TRY(tar_stream->for_each_extended_header([&](StringView key, StringView value) {
                         local_overrides.set(key, value);
                     }));
                     break;
@@ -124,11 +137,11 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
                     VERIFY_NOT_REACHED();
                 }
 
-                TRY(tar_stream.advance());
+                TRY(tar_stream->advance());
                 continue;
             }
 
-            Archive::TarFileStream file_stream = tar_stream.file_contents();
+            Archive::TarFileStream file_stream = tar_stream->file_contents();
 
             // Handle other header types that don't just have an effect on extraction.
             switch (header.type_flag()) {
@@ -143,7 +156,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
                 }
 
                 local_overrides.set("path", long_name.to_string());
-                TRY(tar_stream.advance());
+                TRY(tar_stream->advance());
                 continue;
             }
             default:
@@ -204,9 +217,8 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
             // Non-global headers should be cleared after every file.
             local_overrides.clear();
 
-            TRY(tar_stream.advance());
+            TRY(tar_stream->advance());
         }
-        file_stream.close();
 
         return 0;
     }