FileManager: Use a Core::File for the FileOperation pipes

Instead of popen()/pclose(), we now open the pipes manually and wrap
them in a friendly Core::File object.

3 files changed, 46 insertions, 14 deletions

diff --git a/Userland/Applications/FileManager/DirectoryView.cpp b/Userland/Applications/FileManager/DirectoryView.cpp
index a13e268847..5909032c84 100644
--- a/Userland/Applications/FileManager/DirectoryView.cpp
+++ b/Userland/Applications/FileManager/DirectoryView.cpp
@@ -54,15 +54,47 @@ static HashTable<RefPtr<GUI::Window>> file_operation_windows;
 
 static void run_file_operation([[maybe_unused]] FileOperation operation, const String& source, const String& destination, GUI::Window* parent_window)
 {
-    // FIXME: Don't use popen() like this, very string injection friendly..
-    FILE* helper_pipe = popen(String::formatted("/bin/FileOperation Copy {} {}", source, LexicalPath(destination).dirname()).characters(), "r");
-    VERIFY(helper_pipe);
+    int pipe_fds[2];
+    if (pipe(pipe_fds) < 0) {
+        perror("pipe");
+        VERIFY_NOT_REACHED();
+    }
+
+    pid_t child_pid = fork();
+    if (child_pid < 0) {
+        perror("fork");
+        VERIFY_NOT_REACHED();
+    }
+
+    if (!child_pid) {
+        if (close(pipe_fds[0]) < 0) {
+            perror("close");
+            _exit(1);
+        }
+        if (dup2(pipe_fds[1], STDOUT_FILENO) < 0) {
+            perror("dup2");
+            _exit(1);
+        }
+        if (execlp("/bin/FileOperation", "/bin/FileOperation", "Copy", source.characters(), LexicalPath(destination).dirname().characters(), nullptr) < 0) {
+            perror("execlp");
+            _exit(1);
+        }
+        VERIFY_NOT_REACHED();
+    } else {
+        if (close(pipe_fds[1]) < 0) {
+            perror("close");
+            _exit(1);
+        }
+    }
 
     auto window = GUI::Window::construct();
     file_operation_windows.set(window);
 
+    auto pipe_input_file = Core::File::construct();
+    pipe_input_file->open(pipe_fds[0], Core::IODevice::ReadOnly, Core::File::ShouldCloseFileDescriptor::Yes);
+
     window->set_title("Copying Files...");
-    window->set_main_widget<FileOperationProgressWidget>(helper_pipe);
+    window->set_main_widget<FileOperationProgressWidget>(pipe_input_file);
     window->resize(320, 200);
     if (parent_window)
         window->center_within(*parent_window);
diff --git a/Userland/Applications/FileManager/FileOperationProgressWidget.cpp b/Userland/Applications/FileManager/FileOperationProgressWidget.cpp
index 25b758a88c..cd4897f775 100644
--- a/Userland/Applications/FileManager/FileOperationProgressWidget.cpp
+++ b/Userland/Applications/FileManager/FileOperationProgressWidget.cpp
@@ -26,6 +26,7 @@
 
 #include "FileOperationProgressWidget.h"
 #include <Applications/FileManager/FileOperationProgressGML.h>
+#include <LibCore/File.h>
 #include <LibCore/Notifier.h>
 #include <LibGUI/Button.h>
 #include <LibGUI/Label.h>
@@ -35,8 +36,8 @@
 
 namespace FileManager {
 
-FileOperationProgressWidget::FileOperationProgressWidget(FILE* helper_pipe)
-    : m_helper_pipe(helper_pipe)
+FileOperationProgressWidget::FileOperationProgressWidget(NonnullRefPtr<Core::File> helper_pipe)
+    : m_helper_pipe(move(helper_pipe))
 {
     load_from_gml(file_operation_progress_gml);
 
@@ -47,17 +48,17 @@ FileOperationProgressWidget::FileOperationProgressWidget(FILE* helper_pipe)
         window()->close();
     };
 
-    m_notifier = Core::Notifier::construct(fileno(m_helper_pipe), Core::Notifier::Read);
+    m_notifier = Core::Notifier::construct(m_helper_pipe->fd(), Core::Notifier::Read);
     m_notifier->on_ready_to_read = [this] {
-        char buffer[8192];
-        if (!fgets(buffer, sizeof(buffer), m_helper_pipe)) {
+        auto line = m_helper_pipe->read_line();
+        if (line.is_null()) {
             did_error();
             return;
         }
-        auto parts = StringView(buffer).split_view(' ');
+        auto parts = line.split_view(' ');
         VERIFY(!parts.is_empty());
 
-        if (parts[0] == "FINISH\n"sv) {
+        if (parts[0] == "FINISH"sv) {
             did_finish();
             return;
         }
@@ -113,7 +114,6 @@ void FileOperationProgressWidget::close_pipe()
 {
     if (!m_helper_pipe)
         return;
-    pclose(m_helper_pipe);
     m_helper_pipe = nullptr;
     if (m_notifier)
         m_notifier->on_ready_to_read = nullptr;
diff --git a/Userland/Applications/FileManager/FileOperationProgressWidget.h b/Userland/Applications/FileManager/FileOperationProgressWidget.h
index 1bd9b5d2c1..4efbbb19d7 100644
--- a/Userland/Applications/FileManager/FileOperationProgressWidget.h
+++ b/Userland/Applications/FileManager/FileOperationProgressWidget.h
@@ -37,7 +37,7 @@ public:
     virtual ~FileOperationProgressWidget() override;
 
 private:
-    explicit FileOperationProgressWidget(FILE* helper_pipe);
+    explicit FileOperationProgressWidget(NonnullRefPtr<Core::File> helper_pipe);
 
     void did_finish();
     void did_error();
@@ -46,6 +46,6 @@ private:
     void close_pipe();
 
     RefPtr<Core::Notifier> m_notifier;
-    FILE* m_helper_pipe { nullptr };
+    RefPtr<Core::File> m_helper_pipe;
 };
 }