feat: add CrashpadClient::Add/RemoveAttachment() for Windows and Linux (#124)

jpnurmi · web-flow · commit 2e8ce8b6832a · 2025-06-13T09:53:01.000+02:00
diff --git a/client/crashpad_client.h b/client/crashpad_client.h
@@ -840,6 +840,20 @@ class CrashpadClient {
   static void SetCrashLoopBefore(uint64_t crash_loop_before_time);
 #endif
 
+#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || DOXYGEN
+  //! \brief Adds a file to the list of files to be attached to the crash
+  //!     report.
+  //!
+  //! \param[in] attachment The path to the file to be added.
+  void AddAttachment(const base::FilePath& attachment);
+
+  //! \brief Removes a file from the list of files to be attached to the crash
+  //!     report.
+  //!
+  //! \param[in] attachment The path to the file to be removed.
+  void RemoveAttachment(const base::FilePath& attachment);
+#endif
+
  private:
 #if BUILDFLAG(IS_WIN)
   //!  \brief Registers process handlers for the client.
diff --git a/client/crashpad_client_linux.cc b/client/crashpad_client_linux.cc
@@ -436,6 +436,16 @@ class RequestCrashDumpHandler : public SignalHandler {
   }
 #endif
 
+  void AddAttachment(const base::FilePath& attachment) {
+    ExceptionHandlerClient client(sock_to_handler_.get(), true);
+    client.AddAttachment(attachment);
+  }
+
+  void RemoveAttachment(const base::FilePath& attachment) {
+    ExceptionHandlerClient client(sock_to_handler_.get(), true);
+    client.RemoveAttachment(attachment);
+  }
+
  private:
   RequestCrashDumpHandler() = default;
 
@@ -807,4 +817,14 @@ void CrashpadClient::SetCrashLoopBefore(uint64_t crash_loop_before_time) {
 }
 #endif
 
+void CrashpadClient::AddAttachment(const base::FilePath& attachment) {
+  auto signal_handler = RequestCrashDumpHandler::Get();
+  signal_handler->AddAttachment(attachment);
+}
+
+void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) {
+  auto signal_handler = RequestCrashDumpHandler::Get();
+  signal_handler->RemoveAttachment(attachment);
+}
+
 }  // namespace crashpad
diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc
@@ -1183,4 +1183,22 @@ void CrashpadClient::SetFirstChanceExceptionHandler(
   first_chance_handler_ = handler;
 }
 
+void CrashpadClient::AddAttachment(const base::FilePath& attachment) {
+  ClientToServerMessage message = {};
+  message.type = ClientToServerMessage::kAddAttachment;
+  swprintf_s(
+      message.attachment.path, MAX_PATH, L"%ls", attachment.value().c_str());
+  ServerToClientMessage response = {};
+  SendToCrashHandlerServer(ipc_pipe_, message, &response);
+}
+
+void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) {
+  ClientToServerMessage message = {};
+  message.type = ClientToServerMessage::kRemoveAttachment;
+  swprintf_s(
+      message.attachment.path, MAX_PATH, L"%ls", attachment.value().c_str());
+  ServerToClientMessage response = {};
+  SendToCrashHandlerServer(ipc_pipe_, message, &response);
+}
+
 }  // namespace crashpad
diff --git a/handler/linux/crash_report_exception_handler.cc b/handler/linux/crash_report_exception_handler.cc
@@ -111,7 +111,7 @@ CrashReportExceptionHandler::CrashReportExceptionHandler(
     : database_(database),
       upload_thread_(upload_thread),
       process_annotations_(process_annotations),
-      attachments_(attachments),
+      attachments_(*attachments),
       write_minidump_to_database_(write_minidump_to_database),
       write_minidump_to_log_(write_minidump_to_log),
       user_stream_data_sources_(user_stream_data_sources),
@@ -210,6 +210,26 @@ bool CrashReportExceptionHandler::HandleExceptionWithConnection(
   return result;
 }
 
+void CrashReportExceptionHandler::AddAttachment(
+    const base::FilePath& attachment) {
+  auto it = std::find(attachments_.begin(), attachments_.end(), attachment);
+  if (it != attachments_.end()) {
+    LOG(WARNING) << "ignoring duplicate attachment " << attachment;
+    return;
+  }
+  attachments_.push_back(attachment);
+}
+
+void CrashReportExceptionHandler::RemoveAttachment(
+    const base::FilePath& attachment) {
+  auto it = std::find(attachments_.begin(), attachments_.end(), attachment);
+  if (it == attachments_.end()) {
+    LOG(WARNING) << "ignoring non-existent attachment " << attachment;
+    return;
+  }
+  attachments_.erase(it);
+}
+
 bool CrashReportExceptionHandler::WriteMinidumpToDatabase(
     ProcessSnapshotLinux* process_snapshot,
     ProcessSnapshotSanitized* sanitized_snapshot,
@@ -252,7 +272,7 @@ bool CrashReportExceptionHandler::WriteMinidumpToDatabase(
     }
   }
 
-  for (const auto& attachment : (*attachments_)) {
+  for (const auto& attachment : attachments_) {
     FileReader file_reader;
     if (!file_reader.Open(attachment)) {
       LOG(ERROR) << "attachment " << attachment.value().c_str()
diff --git a/handler/linux/crash_report_exception_handler.h b/handler/linux/crash_report_exception_handler.h
@@ -18,6 +18,7 @@
 #include <map>
 #include <string>
 
+#include "base/files/file_path.h"
 #include "client/crash_report_database.h"
 #include "handler/crash_report_upload_thread.h"
 #include "handler/linux/exception_handler_server.h"
@@ -94,6 +95,9 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate {
       int broker_sock,
       UUID* local_report_id = nullptr) override;
 
+  void AddAttachment(const base::FilePath& attachment) override;
+  void RemoveAttachment(const base::FilePath& attachment) override;
+
  private:
   bool HandleExceptionWithConnection(
       PtraceConnection* connection,
@@ -119,7 +123,7 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate {
   CrashReportDatabase* database_;  // weak
   CrashReportUploadThread* upload_thread_;  // weak
   const std::map<std::string, std::string>* process_annotations_;  // weak
-  const std::vector<base::FilePath>* attachments_;  // weak
+  std::vector<base::FilePath> attachments_;
   bool write_minidump_to_database_;
   bool write_minidump_to_log_;
   const UserStreamDataSources* user_stream_data_sources_;  // weak
diff --git a/handler/linux/exception_handler_server.cc b/handler/linux/exception_handler_server.cc
@@ -431,6 +431,14 @@ bool ExceptionHandlerServer::ReceiveClientMessage(Event* event) {
           message.requesting_thread_stack_address,
           event->fd.get(),
           event->type == Event::Type::kSharedSocketMessage);
+
+    case ExceptionHandlerProtocol::ClientToServerMessage::kTypeAddAttachment:
+      delegate_->AddAttachment(base::FilePath(message.attachment_info.path));
+      return true;
+
+    case ExceptionHandlerProtocol::ClientToServerMessage::kTypeRemoveAttachment:
+      delegate_->RemoveAttachment(base::FilePath(message.attachment_info.path));
+      return true;
   }
 
   DCHECK(false);
diff --git a/handler/linux/exception_handler_server.h b/handler/linux/exception_handler_server.h
@@ -111,6 +111,12 @@ class ExceptionHandlerServer {
         int broker_sock,
         UUID* local_report_id = nullptr) = 0;
 
+    //! \brief Called to add an attachment to the crash report.
+    virtual void AddAttachment(const base::FilePath& attachment) = 0;
+
+    //! \brief Called to remove an attachment from the crash report.
+    virtual void RemoveAttachment(const base::FilePath& attachment) = 0;
+
     virtual ~Delegate() {}
   };
 
diff --git a/handler/win/crash_report_exception_handler.cc b/handler/win/crash_report_exception_handler.cc
@@ -45,7 +45,7 @@ CrashReportExceptionHandler::CrashReportExceptionHandler(
     : database_(database),
       upload_thread_(upload_thread),
       process_annotations_(process_annotations),
-      attachments_(attachments),
+      attachments_(*attachments),
       screenshot_(screenshot),
       wait_for_upload_(wait_for_upload),
       user_stream_data_sources_(user_stream_data_sources) {}
@@ -117,7 +117,7 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException(
       return termination_code;
     }
 
-    for (const auto& attachment : (*attachments_)) {
+    for (const auto& attachment : attachments_) {
       FileReader file_reader;
       if (!file_reader.Open(attachment)) {
         LOG(ERROR) << "attachment " << attachment
@@ -175,4 +175,24 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException(
   return termination_code;
 }
 
+void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentAdded(
+    const base::FilePath& attachment) {
+  auto it = std::find(attachments_.begin(), attachments_.end(), attachment);
+  if (it != attachments_.end()) {
+    LOG(WARNING) << "ignoring duplicate attachment " << attachment;
+    return;
+  }
+  attachments_.push_back(attachment);
+}
+
+void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentRemoved(
+    const base::FilePath& attachment) {
+  auto it = std::find(attachments_.begin(), attachments_.end(), attachment);
+  if (it == attachments_.end()) {
+    LOG(WARNING) << "ignoring non-existent attachment " << attachment;
+    return;
+  }
+  attachments_.erase(it);
+}
+
 }  // namespace crashpad
diff --git a/handler/win/crash_report_exception_handler.h b/handler/win/crash_report_exception_handler.h
@@ -79,12 +79,16 @@ class CrashReportExceptionHandler final
       HANDLE process,
       WinVMAddress exception_information_address,
       WinVMAddress debug_critical_section_address) override;
+  void ExceptionHandlerServerAttachmentAdded(
+      const base::FilePath& attachment) override;
+  void ExceptionHandlerServerAttachmentRemoved(
+      const base::FilePath& attachment) override;
 
  private:
   CrashReportDatabase* database_;  // weak
   CrashReportUploadThread* upload_thread_;  // weak
   const std::map<std::string, std::string>* process_annotations_;  // weak
-  const std::vector<base::FilePath>* attachments_;  // weak
+  std::vector<base::FilePath> attachments_;
   const base::FilePath* screenshot_;  // weak
   const bool wait_for_upload_;
   const UserStreamDataSources* user_stream_data_sources_;  // weak
diff --git a/util/linux/exception_handler_client.cc b/util/linux/exception_handler_client.cc
@@ -225,4 +225,23 @@ int ExceptionHandlerClient::WaitForCrashDumpComplete() {
   return errno;
 }
 
+void ExceptionHandlerClient::AddAttachment(const base::FilePath& attachment) {
+  ExceptionHandlerProtocol::ClientToServerMessage message;
+  message.type =
+      ExceptionHandlerProtocol::ClientToServerMessage::kTypeAddAttachment;
+  snprintf(
+      message.attachment_info.path, PATH_MAX, "%s", attachment.value().c_str());
+  UnixCredentialSocket::SendMsg(server_sock_, &message, sizeof(message));
+}
+
+void ExceptionHandlerClient::RemoveAttachment(
+    const base::FilePath& attachment) {
+  ExceptionHandlerProtocol::ClientToServerMessage message;
+  message.type =
+      ExceptionHandlerProtocol::ClientToServerMessage::kTypeRemoveAttachment;
+  snprintf(
+      message.attachment_info.path, PATH_MAX, "%s", attachment.value().c_str());
+  UnixCredentialSocket::SendMsg(server_sock_, &message, sizeof(message));
+}
+
 }  // namespace crashpad
diff --git a/util/linux/exception_handler_client.h b/util/linux/exception_handler_client.h
@@ -68,6 +68,12 @@ class ExceptionHandlerClient {
   //! \param[in] can_set_ptracer Whether SetPtracer should be enabled.
   void SetCanSetPtracer(bool can_set_ptracer);
 
+  //! \brief Adds an attachment to the crash report.
+  void AddAttachment(const base::FilePath& attachment);
+
+  //! \brief Removes an attachment from the crash report.
+  void RemoveAttachment(const base::FilePath& attachment);
+
  private:
   int SendCrashDumpRequest(
       const ExceptionHandlerProtocol::ClientInformation& info,
diff --git a/util/linux/exception_handler_protocol.h b/util/linux/exception_handler_protocol.h
@@ -16,6 +16,7 @@
 #define CRASHPAD_UTIL_LINUX_EXCEPTION_HANDLER_PROTOCOL_H_
 
 #include <errno.h>
+#include <linux/limits.h>
 #include <signal.h>
 #include <stdint.h>
 #include <sys/types.h>
@@ -59,6 +60,10 @@ class ExceptionHandlerProtocol {
 #endif
   };
 
+  struct AttachmentInformation {
+    char path[PATH_MAX];
+  };
+
   //! \brief The signal used to indicate a crash dump is complete.
   //!
   //! When multiple clients share a single socket connection with the handler,
@@ -81,7 +86,13 @@ class ExceptionHandlerProtocol {
       kTypeCheckCredentials,
 
       //! \brief Used to request a crash dump for the sending client.
-      kTypeCrashDumpRequest
+      kTypeCrashDumpRequest,
+
+      //! \brief Request that the server add an attachment.
+      kTypeAddAttachment,
+
+      //! \brief Request that the server remove an attachment.
+      kTypeRemoveAttachment,
     };
 
     Type type;
@@ -92,6 +103,9 @@ class ExceptionHandlerProtocol {
     union {
       //! \brief Valid for type == kCrashDumpRequest
       ClientInformation client_info;
+
+      //! \brief Valid for type == kAddAttachment || type == kRemoveAttachment
+      AttachmentInformation attachment_info;
     };
   };
 
diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc
@@ -455,6 +455,26 @@ bool ExceptionHandlerServer::ServiceClientConnection(
       // Handled below.
       break;
 
+    case ClientToServerMessage::kAddAttachment: {
+      ServerToClientMessage shutdown_response = {};
+      service_context.delegate()->ExceptionHandlerServerAttachmentAdded(
+          base::FilePath(message.attachment.path));
+      LoggingWriteFile(service_context.pipe(),
+                       &shutdown_response,
+                       sizeof(shutdown_response));
+      return false;
+    }
+
+    case ClientToServerMessage::kRemoveAttachment: {
+      ServerToClientMessage shutdown_response = {};
+      service_context.delegate()->ExceptionHandlerServerAttachmentRemoved(
+          base::FilePath(message.attachment.path));
+      LoggingWriteFile(service_context.pipe(),
+                       &shutdown_response,
+                       sizeof(shutdown_response));
+      return false;
+    }
+
     default:
       LOG(ERROR) << "unhandled message type: " << message.type;
       return false;
diff --git a/util/win/exception_handler_server.h b/util/win/exception_handler_server.h
@@ -58,6 +58,20 @@ class ExceptionHandlerServer {
         WinVMAddress exception_information_address,
         WinVMAddress debug_critical_section_address) = 0;
 
+    //! \brief Called when the server has received a request to add an
+    //! attachment.
+    //!
+    //! \param[in] attachment The path of the attachment.
+    virtual void ExceptionHandlerServerAttachmentAdded(
+        const base::FilePath& attachment) = 0;
+
+    //! \brief Called when the server has received a request to remove an
+    //! attachment.
+    //!
+    //! \param[in] attachment The path of the attachment.
+    virtual void ExceptionHandlerServerAttachmentRemoved(
+        const base::FilePath& attachment) = 0;
+
    protected:
     ~Delegate();
   };
diff --git a/util/win/registration_protocol_win_structs.h b/util/win/registration_protocol_win_structs.h
@@ -116,6 +116,12 @@ struct ShutdownRequest {
   uint64_t token;
 };
 
+//! \brief A file attachment request.
+struct AttachmentRequest {
+  //! \brief The path of the attachment.
+  wchar_t path[MAX_PATH];
+};
+
 //! \brief The message passed from client to server by
 //!     SendToCrashHandlerServer().
 struct ClientToServerMessage {
@@ -127,6 +133,12 @@ struct ClientToServerMessage {
     //! \brief For ShutdownRequest.
     kShutdown,
 
+    //! \brief For AttachmentRequest.
+    kAddAttachment,
+
+    //! \brief For AttachmentRequest.
+    kRemoveAttachment,
+
     //! \brief An empty message sent by the initial client in asynchronous mode.
     //!     No data is required, this just confirms that the server is ready to
     //!     accept client registrations.
@@ -136,6 +148,7 @@ struct ClientToServerMessage {
   union {
     RegistrationRequest registration;
     ShutdownRequest shutdown;
+    AttachmentRequest attachment;
   };
 };
 
