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> 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(helper_pipe); + window->set_main_widget(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 +#include #include #include #include @@ -35,8 +36,8 @@ namespace FileManager { -FileOperationProgressWidget::FileOperationProgressWidget(FILE* helper_pipe) - : m_helper_pipe(helper_pipe) +FileOperationProgressWidget::FileOperationProgressWidget(NonnullRefPtr 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 helper_pipe); void did_finish(); void did_error(); @@ -46,6 +46,6 @@ private: void close_pipe(); RefPtr m_notifier; - FILE* m_helper_pipe { nullptr }; + RefPtr m_helper_pipe; }; }