LibArchive+Utilities: Stop using DeprecatedString

This also slightly improves error propagation in tar, unzip and zip.

Meta/Lagom/Fuzzers/FuzzZip.cpp

@@ -14,7 +14,7 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
     if (!zip_file.has_value())
         return 0;
 
-    zip_file->for_each_member([](auto&) {
+    (void)zip_file->for_each_member([](auto&) {
         return IterationDecision::Continue;
     });
 

Userland/Libraries/LibArchive/Tar.cpp

@@ -24,10 +24,12 @@ unsigned TarFileHeader::expected_checksum() const
     return checksum;
 }
 
-void TarFileHeader::calculate_checksum()
+ErrorOr<void> TarFileHeader::calculate_checksum()
 {
     memset(m_checksum, ' ', sizeof(m_checksum));
-    VERIFY(DeprecatedString::formatted("{:06o}", expected_checksum()).copy_characters_to_buffer(m_checksum, sizeof(m_checksum)));
+    bool copy_successful = TRY(String::formatted("{:06o}", expected_checksum())).bytes_as_string_view().copy_characters_to_buffer(m_checksum, sizeof(m_checksum));
+    VERIFY(copy_successful);
+    return {};
 }
 
 bool TarFileHeader::is_zero_block() const

Userland/Libraries/LibArchive/Tar.h

@@ -8,7 +8,7 @@
 
 #pragma once
 
-#include <AK/DeprecatedString.h>
+#include <AK/String.h>
 #include <AK/StringView.h>
 #include <string.h>
 #include <sys/types.h>
@@ -82,9 +82,10 @@ static void set_field(char (&field)[N], TSource&& source)
 }
 
 template<class TSource, size_t N>
-static void set_octal_field(char (&field)[N], TSource&& source)
+static ErrorOr<void> set_octal_field(char (&field)[N], TSource&& source)
 {
-    set_field(field, DeprecatedString::formatted("{:o}", forward<TSource>(source)));
+    set_field(field, TRY(String::formatted("{:o}", forward<TSource>(source))).bytes_as_string_view());
+    return {};
 }
 
 class [[gnu::packed]] TarFileHeader {
@@ -109,11 +110,11 @@ public:
     StringView prefix() const { return get_field_as_string_view(m_prefix); }
 
     void set_filename(StringView filename) { set_field(m_filename, filename); }
-    void set_mode(mode_t mode) { set_octal_field(m_mode, mode); }
-    void set_uid(uid_t uid) { set_octal_field(m_uid, uid); }
-    void set_gid(gid_t gid) { set_octal_field(m_gid, gid); }
-    void set_size(size_t size) { set_octal_field(m_size, size); }
-    void set_timestamp(time_t timestamp) { set_octal_field(m_timestamp, timestamp); }
+    ErrorOr<void> set_mode(mode_t mode) { return set_octal_field(m_mode, mode); }
+    ErrorOr<void> set_uid(uid_t uid) { return set_octal_field(m_uid, uid); }
+    ErrorOr<void> set_gid(gid_t gid) { return set_octal_field(m_gid, gid); }
+    ErrorOr<void> set_size(size_t size) { return set_octal_field(m_size, size); }
+    ErrorOr<void> set_timestamp(time_t timestamp) { return set_octal_field(m_timestamp, timestamp); }
     void set_type_flag(TarFileType type) { m_type_flag = to_underlying(type); }
     void set_link_name(StringView link_name) { set_field(m_link_name, link_name); }
     // magic doesn't necessarily include a null byte
@@ -122,12 +123,12 @@ public:
     void set_version(StringView version) { set_field(m_version, version); }
     void set_owner_name(StringView owner_name) { set_field(m_owner_name, owner_name); }
     void set_group_name(StringView group_name) { set_field(m_group_name, group_name); }
-    void set_major(int major) { set_octal_field(m_major, major); }
-    void set_minor(int minor) { set_octal_field(m_minor, minor); }
+    ErrorOr<void> set_major(int major) { return set_octal_field(m_major, major); }
+    ErrorOr<void> set_minor(int minor) { return set_octal_field(m_minor, minor); }
     void set_prefix(StringView prefix) { set_field(m_prefix, prefix); }
 
     unsigned expected_checksum() const;
-    void calculate_checksum();
+    ErrorOr<void> calculate_checksum();
 
     bool is_zero_block() const;
     bool content_is_like_extended_header() const;

Userland/Libraries/LibArchive/TarStream.cpp

@@ -142,34 +142,34 @@ TarOutputStream::TarOutputStream(Core::Stream::Handle<Core::Stream::Stream> stre
 {
 }
 
-ErrorOr<void> TarOutputStream::add_directory(DeprecatedString const& path, mode_t mode)
+ErrorOr<void> TarOutputStream::add_directory(StringView path, mode_t mode)
 {
     VERIFY(!m_finished);
     TarFileHeader header {};
-    header.set_size(0);
-    header.set_filename_and_prefix(DeprecatedString::formatted("{}/", path)); // Old tar implementations assume directory names end with a /
+    TRY(header.set_size(0));
+    header.set_filename_and_prefix(TRY(String::formatted("{}/", path))); // Old tar implementations assume directory names end with a /
     header.set_type_flag(TarFileType::Directory);
-    header.set_mode(mode);
+    TRY(header.set_mode(mode));
     header.set_magic(gnu_magic);
     header.set_version(gnu_version);
-    header.calculate_checksum();
+    TRY(header.calculate_checksum());
     TRY(m_stream->write_entire_buffer(Bytes { &header, sizeof(header) }));
     u8 padding[block_size] = { 0 };
     TRY(m_stream->write_entire_buffer(Bytes { &padding, block_size - sizeof(header) }));
     return {};
 }
 
-ErrorOr<void> TarOutputStream::add_file(DeprecatedString const& path, mode_t mode, ReadonlyBytes bytes)
+ErrorOr<void> TarOutputStream::add_file(StringView path, mode_t mode, ReadonlyBytes bytes)
 {
     VERIFY(!m_finished);
     TarFileHeader header {};
-    header.set_size(bytes.size());
+    TRY(header.set_size(bytes.size()));
     header.set_filename_and_prefix(path);
     header.set_type_flag(TarFileType::NormalFile);
-    header.set_mode(mode);
+    TRY(header.set_mode(mode));
     header.set_magic(gnu_magic);
     header.set_version(gnu_version);
-    header.calculate_checksum();
+    TRY(header.calculate_checksum());
     TRY(m_stream->write_entire_buffer(ReadonlyBytes { &header, sizeof(header) }));
     constexpr Array<u8, block_size> padding { 0 };
     TRY(m_stream->write_entire_buffer(ReadonlyBytes { &padding, block_size - sizeof(header) }));
@@ -181,18 +181,18 @@ ErrorOr<void> TarOutputStream::add_file(DeprecatedString const& path, mode_t mod
     return {};
 }
 
-ErrorOr<void> TarOutputStream::add_link(DeprecatedString const& path, mode_t mode, StringView link_name)
+ErrorOr<void> TarOutputStream::add_link(StringView path, mode_t mode, StringView link_name)
 {
     VERIFY(!m_finished);
     TarFileHeader header {};
-    header.set_size(0);
+    TRY(header.set_size(0));
     header.set_filename_and_prefix(path);
     header.set_type_flag(TarFileType::SymLink);
-    header.set_mode(mode);
+    TRY(header.set_mode(mode));
     header.set_magic(gnu_magic);
     header.set_version(gnu_version);
     header.set_link_name(link_name);
-    header.calculate_checksum();
+    TRY(header.calculate_checksum());
     TRY(m_stream->write_entire_buffer(Bytes { &header, sizeof(header) }));
     u8 padding[block_size] = { 0 };
     TRY(m_stream->write_entire_buffer(Bytes { &padding, block_size - sizeof(header) }));

Userland/Libraries/LibArchive/TarStream.h

@@ -59,9 +59,9 @@ private:
 class TarOutputStream {
 public:
     TarOutputStream(Core::Stream::Handle<Core::Stream::Stream>);
-    ErrorOr<void> add_file(DeprecatedString const& path, mode_t, ReadonlyBytes);
-    ErrorOr<void> add_link(DeprecatedString const& path, mode_t, StringView);
-    ErrorOr<void> add_directory(DeprecatedString const& path, mode_t);
+    ErrorOr<void> add_file(StringView path, mode_t, ReadonlyBytes);
+    ErrorOr<void> add_link(StringView path, mode_t, StringView);
+    ErrorOr<void> add_directory(StringView path, mode_t);
     ErrorOr<void> finish();
 
 private:

Userland/Libraries/LibArchive/Zip.cpp

@@ -75,7 +75,7 @@ Optional<Zip> Zip::try_create(ReadonlyBytes buffer)
     };
 }
 
-bool Zip::for_each_member(Function<IterationDecision(ZipMember const&)> callback)
+ErrorOr<bool> Zip::for_each_member(Function<IterationDecision(ZipMember const&)> callback)
 {
     size_t member_offset = m_members_start_offset;
     for (size_t i = 0; i < m_member_count; i++) {
@@ -85,15 +85,12 @@ bool Zip::for_each_member(Function<IterationDecision(ZipMember const&)> callback
         VERIFY(local_file_header.read(m_input_data.slice(central_directory_record.local_file_header_offset)));
 
         ZipMember member;
-        char null_terminated_name[central_directory_record.name_length + 1];
-        memcpy(null_terminated_name, central_directory_record.name, central_directory_record.name_length);
-        null_terminated_name[central_directory_record.name_length] = 0;
-        member.name = DeprecatedString { null_terminated_name };
+        member.name = TRY(String::from_utf8({ central_directory_record.name, central_directory_record.name_length }));
         member.compressed_data = { local_file_header.compressed_data, central_directory_record.compressed_size };
         member.compression_method = central_directory_record.compression_method;
         member.uncompressed_size = central_directory_record.uncompressed_size;
         member.crc32 = central_directory_record.crc32;
-        member.is_directory = central_directory_record.external_attributes & zip_directory_external_attribute || member.name.ends_with('/'); // FIXME: better directory detection
+        member.is_directory = central_directory_record.external_attributes & zip_directory_external_attribute || member.name.bytes_as_string_view().ends_with('/'); // FIXME: better directory detection
 
         if (callback(member) == IterationDecision::Break)
             return false;
@@ -117,7 +114,7 @@ static u16 minimum_version_needed(ZipCompressionMethod method)
 ErrorOr<void> ZipOutputStream::add_member(ZipMember const& member)
 {
     VERIFY(!m_finished);
-    VERIFY(member.name.length() <= UINT16_MAX);
+    VERIFY(member.name.bytes_as_string_view().length() <= UINT16_MAX);
     VERIFY(member.compressed_data.size() <= UINT32_MAX);
     TRY(m_members.try_append(member));
 
@@ -130,9 +127,9 @@ ErrorOr<void> ZipOutputStream::add_member(ZipMember const& member)
         .crc32 = member.crc32,
         .compressed_size = static_cast<u32>(member.compressed_data.size()),
         .uncompressed_size = member.uncompressed_size,
-        .name_length = static_cast<u16>(member.name.length()),
+        .name_length = static_cast<u16>(member.name.bytes_as_string_view().length()),
         .extra_data_length = 0,
-        .name = reinterpret_cast<u8 const*>(member.name.characters()),
+        .name = reinterpret_cast<u8 const*>(member.name.bytes_as_string_view().characters_without_null_termination()),
         .extra_data = nullptr,
         .compressed_data = member.compressed_data.data(),
     };
@@ -158,18 +155,18 @@ ErrorOr<void> ZipOutputStream::finish()
             .crc32 = member.crc32,
             .compressed_size = static_cast<u32>(member.compressed_data.size()),
             .uncompressed_size = member.uncompressed_size,
-            .name_length = static_cast<u16>(member.name.length()),
+            .name_length = static_cast<u16>(member.name.bytes_as_string_view().length()),
             .extra_data_length = 0,
             .comment_length = 0,
             .start_disk = 0,
             .internal_attributes = 0,
             .external_attributes = member.is_directory ? zip_directory_external_attribute : 0,
             .local_file_header_offset = file_header_offset, // FIXME: we assume the wrapped output stream was never written to before us
-            .name = reinterpret_cast<u8 const*>(member.name.characters()),
+            .name = reinterpret_cast<u8 const*>(member.name.bytes_as_string_view().characters_without_null_termination()),
             .extra_data = nullptr,
             .comment = nullptr,
         };
-        file_header_offset += sizeof(LocalFileHeader::signature) + (sizeof(LocalFileHeader) - (sizeof(u8*) * 3)) + member.name.length() + member.compressed_data.size();
+        file_header_offset += sizeof(LocalFileHeader::signature) + (sizeof(LocalFileHeader) - (sizeof(u8*) * 3)) + member.name.bytes_as_string_view().length() + member.compressed_data.size();
         TRY(central_directory_record.write(*m_stream));
         central_directory_size += central_directory_record.size();
     }

Userland/Libraries/LibArchive/Zip.h

@@ -8,9 +8,9 @@
 #pragma once
 
 #include <AK/Array.h>
-#include <AK/DeprecatedString.h>
 #include <AK/Function.h>
 #include <AK/IterationDecision.h>
+#include <AK/String.h>
 #include <AK/Vector.h>
 #include <LibCore/Stream.h>
 #include <string.h>
@@ -238,7 +238,7 @@ struct [[gnu::packed]] LocalFileHeader {
 };
 
 struct ZipMember {
-    DeprecatedString name;
+    String name;
     ReadonlyBytes compressed_data; // TODO: maybe the decompression/compression should be handled by LibArchive instead of the user?
     ZipCompressionMethod compression_method;
     u32 uncompressed_size;
@@ -249,7 +249,7 @@ struct ZipMember {
 class Zip {
 public:
     static Optional<Zip> try_create(ReadonlyBytes buffer);
-    bool for_each_member(Function<IterationDecision(ZipMember const&)>);
+    ErrorOr<bool> for_each_member(Function<IterationDecision(ZipMember const&)>);
 
 private:
     static bool find_end_of_central_directory_offset(ReadonlyBytes, size_t& offset);

Userland/Utilities/tar.cpp

@@ -228,7 +228,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
             }
 
             auto statbuf = TRY(Core::System::lstat(path));
-            auto canonicalized_path = LexicalPath::canonicalized_path(path);
+            auto canonicalized_path = TRY(String::from_deprecated_string(LexicalPath::canonicalized_path(path)));
             TRY(tar_stream.add_file(canonicalized_path, statbuf.st_mode, file->read_all()));
             if (verbose)
                 outln("{}", canonicalized_path);
@@ -239,7 +239,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         auto add_link = [&](DeprecatedString path) -> ErrorOr<void> {
             auto statbuf = TRY(Core::System::lstat(path));
 
-            auto canonicalized_path = LexicalPath::canonicalized_path(path);
+            auto canonicalized_path = TRY(String::from_deprecated_string(LexicalPath::canonicalized_path(path)));
             TRY(tar_stream.add_link(canonicalized_path, statbuf.st_mode, TRY(Core::System::readlink(path))));
             if (verbose)
                 outln("{}", canonicalized_path);
@@ -250,7 +250,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         auto add_directory = [&](DeprecatedString path, auto handle_directory) -> ErrorOr<void> {
             auto statbuf = TRY(Core::System::lstat(path));
 
-            auto canonicalized_path = LexicalPath::canonicalized_path(path);
+            auto canonicalized_path = TRY(String::from_deprecated_string(LexicalPath::canonicalized_path(path)));
             TRY(tar_stream.add_directory(canonicalized_path, statbuf.st_mode));
             if (verbose)
                 outln("{}", canonicalized_path);

Userland/Utilities/unzip.cpp

@@ -20,16 +20,16 @@
 static bool unpack_zip_member(Archive::ZipMember zip_member, bool quiet)
 {
     if (zip_member.is_directory) {
-        if (mkdir(zip_member.name.characters(), 0755) < 0) {
-            perror("mkdir");
+        if (auto maybe_error = Core::System::mkdir(zip_member.name, 0755); maybe_error.is_error()) {
+            warnln("Failed to create directory '{}': {}", zip_member.name, maybe_error.error());
             return false;
         }
         if (!quiet)
             outln(" extracting: {}", zip_member.name);
         return true;
     }
-    MUST(Core::Directory::create(LexicalPath(zip_member.name).parent(), Core::Directory::CreateDirectories::Yes));
-    auto new_file = Core::File::construct(zip_member.name);
+    MUST(Core::Directory::create(LexicalPath(zip_member.name.to_deprecated_string()).parent(), Core::Directory::CreateDirectories::Yes));
+    auto new_file = Core::File::construct(zip_member.name.to_deprecated_string());
     if (!new_file->open(Core::OpenMode::WriteOnly)) {
         warnln("Can't write file {}: {}", zip_member.name, new_file->error_string());
         return false;
@@ -123,14 +123,14 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         TRY(Core::System::chdir(output_directory_path));
     }
 
-    auto success = zip_file->for_each_member([&](auto zip_member) {
+    auto success = TRY(zip_file->for_each_member([&](auto zip_member) {
         bool keep_file = false;
 
         if (!file_filters.is_empty()) {
             for (auto& filter : file_filters) {
                 // Convert underscore wildcards (usual unzip convention) to question marks (as used by StringUtils)
                 auto string_filter = filter.replace("_"sv, "?"sv, ReplaceMode::All);
-                if (zip_member.name.matches(string_filter, CaseSensitivity::CaseSensitive)) {
+                if (zip_member.name.bytes_as_string_view().matches(string_filter, CaseSensitivity::CaseSensitive)) {
                     keep_file = true;
                     break;
                 }
@@ -145,7 +145,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         }
 
         return IterationDecision::Continue;
-    });
+    }));
 
     return success ? 0 : 1;
 }

Userland/Utilities/zip.cpp

@@ -56,7 +56,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         auto file = TRY(Core::Stream::File::open(path, Core::Stream::OpenMode::Read));
         auto file_buffer = TRY(file->read_until_eof());
         Archive::ZipMember member {};
-        member.name = canonicalized_path;
+        member.name = TRY(String::from_deprecated_string(canonicalized_path));
 
         auto deflate_buffer = Compress::DeflateCompressor::compress_all(file_buffer);
         if (deflate_buffer.has_value() && deflate_buffer.value().size() < file_buffer.size()) {
@@ -79,7 +79,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     auto add_directory = [&](DeprecatedString path, auto handle_directory) -> ErrorOr<void> {
         auto canonicalized_path = DeprecatedString::formatted("{}/", LexicalPath::canonicalized_path(path));
         Archive::ZipMember member {};
-        member.name = canonicalized_path;
+        member.name = TRY(String::from_deprecated_string(canonicalized_path));
         member.compressed_data = {};
         member.compression_method = Archive::ZipCompressionMethod::Store;
         member.uncompressed_size = 0;