Spreadsheet: Generate file previews in memory and save directly to file

When we were asked to make a new preview text, we first generated the
whole file in the /tmp directory and then read the first 8 lines to put
them in a text box. This was doing a bit of unnecessary work at first,
but more importantly, didn't work on a given file descriptor from
FileSystemAccessServer as we were copying a file to a filename instead,
meaning that we also had to unveil the whole home directory.

Anyway, previews will be written now to a MemoryStream by the generator,
which will limit to 8 lines. File saves will omit /tmp entirely,
allowing us to tighten program unveil list a little. :^)

4 files changed, 58 insertions, 111 deletions

diff --git a/Userland/Applications/Spreadsheet/ExportDialog.cpp b/Userland/Applications/Spreadsheet/ExportDialog.cpp
index ae934d10e4..b415ebf372 100644
--- a/Userland/Applications/Spreadsheet/ExportDialog.cpp
+++ b/Userland/Applications/Spreadsheet/ExportDialog.cpp
@@ -10,6 +10,7 @@
 #include <AK/DeprecatedString.h>
 #include <AK/JsonArray.h>
 #include <AK/LexicalPath.h>
+#include <AK/MemoryStream.h>
 #include <Applications/Spreadsheet/CSVExportGML.h>
 #include <LibCore/File.h>
 #include <LibCore/FileStream.h>
@@ -35,21 +36,6 @@ CSVExportDialogPage::CSVExportDialogPage(Sheet const& sheet)
 {
     m_headers.extend(m_data.take_first());
 
-    auto temp_template = DeprecatedString::formatted("{}/spreadsheet-csv-export.{}.XXXXXX", Core::StandardPaths::tempfile_directory(), getpid());
-    auto temp_path = ByteBuffer::create_uninitialized(temp_template.length() + 1).release_value_but_fixme_should_propagate_errors();
-    auto buf = reinterpret_cast<char*>(temp_path.data());
-    auto copy_ok = temp_template.copy_characters_to_buffer(buf, temp_path.size());
-    VERIFY(copy_ok);
-
-    int fd = mkstemp(buf);
-    if (fd < 0) {
-        perror("mkstemp");
-        // Just let the operation fail cleanly later.
-    } else {
-        unlink(buf);
-        m_temp_output_file_path = temp_path;
-    }
-
     m_page = GUI::WizardPage::construct(
         "CSV Export Options",
         "Please select the options for the csv file you wish to export to");
@@ -104,7 +90,7 @@ CSVExportDialogPage::CSVExportDialogPage(Sheet const& sheet)
     update_preview();
 }
 
-auto CSVExportDialogPage::make_writer() -> Optional<XSV>
+auto CSVExportDialogPage::make_writer(OutputStream& stream) -> Optional<XSV>
 {
     DeprecatedString delimiter;
     DeprecatedString quote;
@@ -167,80 +153,24 @@ auto CSVExportDialogPage::make_writer() -> Optional<XSV>
     if (should_quote_all_fields)
         behaviors = behaviors | Writer::WriterBehavior::QuoteAll;
 
-    // Note that the stream is used only by the ctor.
-    auto stream = Core::OutputFileStream::open(m_temp_output_file_path);
-    if (stream.is_error()) {
-        dbgln("Cannot open {} for writing: {}", m_temp_output_file_path, stream.error());
-        return {};
-    }
-    XSV writer(stream.value(), m_data, traits, *headers, behaviors);
-
-    if (stream.value().has_any_error()) {
-        dbgln("Write error when making preview");
-        return {};
-    }
-
-    return writer;
+    return XSV(stream, m_data, move(traits), *headers, behaviors);
 }
 
 void CSVExportDialogPage::update_preview()
-
 {
-    m_previously_made_writer = make_writer();
-    if (!m_previously_made_writer.has_value()) {
-    fail:;
+    DuplexMemoryStream memory_stream;
+    auto maybe_writer = make_writer(memory_stream);
+    if (!maybe_writer.has_value()) {
         m_data_preview_text_editor->set_text({});
         return;
     }
 
-    auto file_or_error = Core::File::open(
-        m_temp_output_file_path,
-        Core::OpenMode::ReadOnly);
-    if (file_or_error.is_error())
-        goto fail;
-
-    auto& file = *file_or_error.value();
-    StringBuilder builder;
-    size_t line = 0;
-    while (file.can_read_line()) {
-        if (++line == 8)
-            break;
-
-        builder.append(file.read_line());
-        builder.append('\n');
-    }
-    m_data_preview_text_editor->set_text(builder.string_view());
+    maybe_writer->generate_preview();
+    auto buffer = memory_stream.copy_into_contiguous_buffer();
+    m_data_preview_text_editor->set_text(StringView(buffer));
     m_data_preview_text_editor->update();
 }
 
-Result<void, DeprecatedString> CSVExportDialogPage::move_into(DeprecatedString const& target)
-{
-    auto& source = m_temp_output_file_path;
-
-    // First, try rename().
-    auto rc = rename(source.characters(), target.characters());
-    if (rc == 0)
-        return {};
-
-    auto saved_errno = errno;
-    if (saved_errno == EXDEV) {
-        // Can't do that, copy it instead.
-        auto result = Core::File::copy_file_or_directory(
-            target, source,
-            Core::File::RecursionMode::Disallowed,
-            Core::File::LinkMode::Disallowed,
-            Core::File::AddDuplicateFileMarker::No);
-
-        if (result.is_error())
-            return DeprecatedString::formatted("{}", static_cast<Error const&>(result.error()));
-
-        return {};
-    }
-
-    perror("rename");
-    return DeprecatedString { strerror(saved_errno) };
-}
-
 Result<void, DeprecatedString> ExportDialog::make_and_run_for(StringView mime, Core::File& file, Workbook& workbook)
 {
     auto wizard = GUI::WizardDialog::construct(GUI::Application::the()->active_window());
@@ -255,20 +185,17 @@ Result<void, DeprecatedString> ExportDialog::make_and_run_for(StringView mime, C
 
         CSVExportDialogPage page { workbook.sheets().first() };
         wizard->replace_page(page.page());
-        auto result = wizard->exec();
-
-        if (result == GUI::Dialog::ExecResult::OK) {
-            auto& writer = page.writer();
-            if (!writer.has_value())
-                return DeprecatedString { "CSV Export failed" };
-            if (writer->has_error())
-                return DeprecatedString::formatted("CSV Export failed: {}", writer->error_string());
-
-            // No error, move the temp file to the expected location
-            return page.move_into(file.filename());
-        } else {
+        if (wizard->exec() != GUI::Dialog::ExecResult::OK)
             return DeprecatedString { "CSV Export was cancelled" };
-        }
+
+        auto file_stream = Core::OutputFileStream(file);
+        auto writer = page.make_writer(file_stream);
+        if (!writer.has_value())
+            return DeprecatedString::formatted("CSV Export failed");
+        writer->generate();
+        if (writer->has_error())
+            return DeprecatedString::formatted("CSV Export failed: {}", writer->error_string());
+        return {};
     };
 
     auto export_worksheet = [&]() -> Result<void, DeprecatedString> {
diff --git a/Userland/Applications/Spreadsheet/ExportDialog.h b/Userland/Applications/Spreadsheet/ExportDialog.h
index dd473384ef..c249c065f5 100644
--- a/Userland/Applications/Spreadsheet/ExportDialog.h
+++ b/Userland/Applications/Spreadsheet/ExportDialog.h
@@ -23,17 +23,14 @@ struct CSVExportDialogPage {
     explicit CSVExportDialogPage(Sheet const&);
 
     NonnullRefPtr<GUI::WizardPage> page() { return *m_page; }
-    Optional<XSV>& writer() { return m_previously_made_writer; }
-    Result<void, DeprecatedString> move_into(DeprecatedString const& target);
+    Optional<XSV> make_writer(OutputStream&);
 
 protected:
     void update_preview();
-    Optional<XSV> make_writer();
 
 private:
     Vector<Vector<DeprecatedString>> m_data;
     Vector<DeprecatedString> m_headers;
-    Optional<XSV> m_previously_made_writer;
     RefPtr<GUI::WizardPage> m_page;
     RefPtr<GUI::RadioButton> m_delimiter_comma_radio;
     RefPtr<GUI::RadioButton> m_delimiter_semicolon_radio;
@@ -54,8 +51,6 @@ private:
         "Repeat",
         "Backslash",
     };
-
-    DeprecatedString m_temp_output_file_path;
 };
 
 struct ExportDialog {
diff --git a/Userland/Applications/Spreadsheet/Writers/XSV.h b/Userland/Applications/Spreadsheet/Writers/XSV.h
index 5c9a81bd5c..3a7323b28f 100644
--- a/Userland/Applications/Spreadsheet/Writers/XSV.h
+++ b/Userland/Applications/Spreadsheet/Writers/XSV.h
@@ -53,17 +53,15 @@ constexpr WriterBehavior default_behaviors()
 template<typename ContainerType, typename HeaderType = Vector<StringView>>
 class XSV {
 public:
-    XSV(OutputStream& output, ContainerType const& data, WriterTraits const& traits, HeaderType const& headers = {}, WriterBehavior behaviors = default_behaviors())
+    XSV(OutputStream& output, ContainerType const& data, WriterTraits traits, HeaderType const& headers = {}, WriterBehavior behaviors = default_behaviors())
         : m_data(data)
-        , m_traits(traits)
+        , m_traits(move(traits))
         , m_behaviors(behaviors)
         , m_names(headers)
         , m_output(output)
     {
         if (!headers.is_empty())
             m_behaviors = m_behaviors | WriterBehavior::WriteHeaders;
-
-        generate();
     }
 
     virtual ~XSV() = default;
@@ -83,20 +81,38 @@ public:
         VERIFY_NOT_REACHED();
     }
 
-private:
-    void set_error(WriteError error)
+    void generate()
     {
-        if (m_error == WriteError::None)
-            m_error = error;
+        auto with_headers = has_flag(m_behaviors, WriterBehavior::WriteHeaders);
+        if (with_headers) {
+            write_row(m_names);
+            if (m_output.write({ "\n", 1 }) != 1)
+                set_error(WriteError::InternalError);
+        }
+
+        for (auto&& row : m_data) {
+            if (with_headers) {
+                if (row.size() != m_names.size())
+                    set_error(WriteError::NonConformingColumnCount);
+            }
+
+            write_row(row);
+            if (m_output.write({ "\n", 1 }) != 1)
+                set_error(WriteError::InternalError);
+        }
     }
 
-    void generate()
+    void generate_preview()
     {
+        auto lines_written = 0;
+        constexpr auto max_preview_lines = 8;
+
         auto with_headers = has_flag(m_behaviors, WriterBehavior::WriteHeaders);
         if (with_headers) {
             write_row(m_names);
             if (m_output.write({ "\n", 1 }) != 1)
                 set_error(WriteError::InternalError);
+            ++lines_written;
         }
 
         for (auto&& row : m_data) {
@@ -108,9 +124,20 @@ private:
             write_row(row);
             if (m_output.write({ "\n", 1 }) != 1)
                 set_error(WriteError::InternalError);
+            ++lines_written;
+
+            if (lines_written >= max_preview_lines)
+                break;
         }
     }
 
+private:
+    void set_error(WriteError error)
+    {
+        if (m_error == WriteError::None)
+            m_error = error;
+    }
+
     template<typename T>
     void write_row(T&& row)
     {
@@ -180,7 +207,7 @@ private:
     }
 
     ContainerType const& m_data;
-    WriterTraits const& m_traits;
+    WriterTraits m_traits;
     WriterBehavior m_behaviors;
     HeaderType const& m_names;
     WriteError m_error { WriteError::None };
diff --git a/Userland/Applications/Spreadsheet/main.cpp b/Userland/Applications/Spreadsheet/main.cpp
index 106ee79fe8..7605622cf3 100644
--- a/Userland/Applications/Spreadsheet/main.cpp
+++ b/Userland/Applications/Spreadsheet/main.cpp
@@ -45,11 +45,9 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     }
 
     TRY(Core::System::unveil("/sys/kernel/processes", "r"));
+    TRY(Core::System::unveil("/tmp/session/%sid/portal/filesystemaccess", "rw"));
     TRY(Core::System::unveil("/tmp/session/%sid/portal/webcontent", "rw"));
-    // For writing temporary files when exporting.
-    TRY(Core::System::unveil("/tmp", "crw"));
     TRY(Core::System::unveil("/etc", "r"));
-    TRY(Core::System::unveil(Core::StandardPaths::home_directory(), "rwc"sv));
     TRY(Core::System::unveil("/res", "r"));
     TRY(Core::System::unveil(nullptr, nullptr));