perf: add API for updating attachments

client/crashpad_client.h

@@ -865,6 +865,14 @@ class CrashpadClient {
   //! \param[in] attachment The path to the file to be added.
   void AddAttachment(const base::FilePath& attachment);
 
+  //! \brief Writes content to an attachment file.
+  bool WriteAttachment(
+      const base::FilePath& attachment, const std::string& data);
+
+  //! \brief Appends content to an attachment file.
+  bool AppendAttachment(
+      const base::FilePath& attachment, const std::string& data);
+
   //! \brief Removes a file from the list of files to be attached to the crash
   //!     report.
   //!

client/crashpad_client_linux.cc

@@ -20,6 +20,7 @@
 #include <linux/futex.h>
 #include <pthread.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <sys/mman.h>
 #include <sys/prctl.h>
 #include <sys/socket.h>
@@ -38,6 +39,7 @@
 #include "third_party/lss/lss.h"
 #include "util/file/file_io.h"
 #include "util/file/filesystem.h"
+#include "util/file/file_writer.h"
 #include "util/linux/exception_handler_client.h"
 #include "util/linux/exception_information.h"
 #include "util/linux/scoped_pr_set_dumpable.h"
@@ -832,6 +834,33 @@ void CrashpadClient::AddAttachment(const base::FilePath& attachment) {
   signal_handler->AddAttachment(attachment);
 }
 
+bool CrashpadClient::WriteAttachment(const base::FilePath& attachment,
+                                     const std::string& data) {
+  FileWriter writer;
+  if (!writer.Open(attachment,
+                   FileWriteMode::kTruncateOrCreate,
+                   FilePermissions::kOwnerOnly) ||
+      !writer.Write(data.data(), data.size())) {
+    LOG(ERROR) << "failed to write attachment " << attachment;
+    return false;
+  }
+  return true;
+}
+
+bool CrashpadClient::AppendAttachment(const base::FilePath& attachment,
+                                      const std::string& data) {
+  FileWriter writer;
+  if (!writer.Open(attachment,
+                   FileWriteMode::kReuseOrCreate,
+                   FilePermissions::kOwnerOnly) ||
+      writer.Seek(0, SEEK_END) < 0 ||
+      !writer.Write(data.data(), data.size())) {
+    LOG(ERROR) << "failed to write attachment " << attachment;
+    return false;
+  }
+  return true;
+}
+
 void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) {
   auto signal_handler = RequestCrashDumpHandler::Get();
   signal_handler->RemoveAttachment(attachment);

client/crashpad_client_mac.cc

@@ -19,6 +19,7 @@
 #include <mach/mach.h>
 #include <pthread.h>
 #include <stdint.h>
+#include <stdio.h>
 #include <unistd.h>
 
 #include <memory>
@@ -29,6 +30,7 @@
 #include "base/check_op.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
+#include "util/file/file_writer.h"
 #include "util/mac/mac_util.h"
 #include "util/mach/bootstrap.h"
 #include "util/mach/child_port_handshake.h"
@@ -637,6 +639,33 @@ void CrashpadClient::AddAttachment(const base::FilePath& attachment) {
                             attachment.value());
 }
 
+bool CrashpadClient::WriteAttachment(const base::FilePath& attachment,
+                                     const std::string& data) {
+  FileWriter writer;
+  if (!writer.Open(attachment,
+                   FileWriteMode::kTruncateOrCreate,
+                   FilePermissions::kOwnerOnly) ||
+      !writer.Write(data.data(), data.size())) {
+    LOG(ERROR) << "failed to write attachment " << attachment;
+    return false;
+  }
+  return true;
+}
+
+bool CrashpadClient::AppendAttachment(const base::FilePath& attachment,
+                                      const std::string& data) {
+  FileWriter writer;
+  if (!writer.Open(attachment,
+                   FileWriteMode::kReuseOrCreate,
+                   FilePermissions::kOwnerOnly) ||
+      writer.Seek(0, SEEK_END) < 0 ||
+      !writer.Write(data.data(), data.size())) {
+    LOG(ERROR) << "failed to write attachment " << attachment;
+    return false;
+  }
+  return true;
+}
+
 void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) {
   SendClientToServerMessage(exception_port_.get(),
                             ClientToServerMessage::kRemoveAttachment,

client/crashpad_client_win.cc

@@ -1216,19 +1216,109 @@
 }
 
 void CrashpadClient::AddAttachment(const base::FilePath& attachment) {
+  const size_t path_length_bytes =
+      (attachment.value().length() + 1) * sizeof(wchar_t);
+  if (path_length_bytes > kMaxPathBytes) {
+    LOG(ERROR) << "Path too long: " << path_length_bytes << " bytes";
+    return;
+  }
+
+  ClientToServerMessage message = {};
+  message.type = ClientToServerMessage::kAddAttachmentV2;
+  message.attachment_v2.path_length_bytes =
+      static_cast<uint32_t>(path_length_bytes);
+
+  ServerToClientMessage response = {};
+  SendPayloadToCrashHandlerServer(ipc_pipe_,
+                                  message,
+                                  attachment.value().c_str(),
+                                  static_cast<uint32_t>(path_length_bytes),
+                                  &response);
+}
+
+bool CrashpadClient::WriteAttachment(const base::FilePath& attachment,
+                                     const std::string& data) {
+  const size_t path_length_bytes =
+      (attachment.value().length() + 1) * sizeof(wchar_t);
+  if (path_length_bytes > kMaxPathBytes ||
+      data.size() > kMaxAttachmentPayloadBytes ||
+      data.size() > UINT32_MAX - path_length_bytes) {
+    LOG(ERROR) << "attachment content too large";
+    return false;
+  }
+
+  ClientToServerMessage message = {};
+  message.type = ClientToServerMessage::kWriteAttachment;
+  message.attachment_write.path_length_bytes =
+      static_cast<uint32_t>(path_length_bytes);
+  message.attachment_write.payload_length_bytes =
+      static_cast<uint32_t>(data.size());
+
+  std::string payload(path_length_bytes + data.size(), '\0');
+  memcpy(&payload[0], attachment.value().c_str(), path_length_bytes);
+  if (!data.empty()) {
+    memcpy(&payload[path_length_bytes], data.data(), data.size());
+  }
+
   ServerToClientMessage response = {};
-  SendAttachmentToCrashHandlerServer(ipc_pipe_,
-                                     ClientToServerMessage::kAddAttachmentV2,
-                                     attachment.value(),
-                                     &response);
+  return SendPayloadToCrashHandlerServer(ipc_pipe_,
+                                         message,
+                                         payload.data(),
+                                         static_cast<uint32_t>(payload.size()),
+                                         &response);
+}
+
+bool CrashpadClient::AppendAttachment(const base::FilePath& attachment,
+                                      const std::string& data) {
+  const size_t path_length_bytes =
+      (attachment.value().length() + 1) * sizeof(wchar_t);
+  if (path_length_bytes > kMaxPathBytes ||
+      data.size() > kMaxAttachmentPayloadBytes ||
+      data.size() > UINT32_MAX - path_length_bytes) {
+    LOG(ERROR) << "attachment content too large";
+    return false;
+  }
+
+  ClientToServerMessage message = {};
+  message.type = ClientToServerMessage::kAppendAttachment;
+  message.attachment_write.path_length_bytes =
+      static_cast<uint32_t>(path_length_bytes);
+  message.attachment_write.payload_length_bytes =
+      static_cast<uint32_t>(data.size());
+
+  std::string payload(path_length_bytes + data.size(), '\0');
+  memcpy(&payload[0], attachment.value().c_str(), path_length_bytes);
+  if (!data.empty()) {
+    memcpy(&payload[path_length_bytes], data.data(), data.size());
+  }
+
+  ServerToClientMessage response = {};
+  return SendPayloadToCrashHandlerServer(ipc_pipe_,
+                                         message,
+                                         payload.data(),
+                                         static_cast<uint32_t>(payload.size()),
+                                         &response);
 }
 
 void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) {
+  const size_t path_length_bytes =
+      (attachment.value().length() + 1) * sizeof(wchar_t);
+  if (path_length_bytes > kMaxPathBytes) {
+    LOG(ERROR) << "Path too long: " << path_length_bytes << " bytes";
+    return;
+  }
+
+  ClientToServerMessage message = {};
+  message.type = ClientToServerMessage::kRemoveAttachmentV2;
+  message.attachment_v2.path_length_bytes =
+      static_cast<uint32_t>(path_length_bytes);
+
   ServerToClientMessage response = {};
-  SendAttachmentToCrashHandlerServer(ipc_pipe_,
-                                     ClientToServerMessage::kRemoveAttachmentV2,
-                                     attachment.value(),
-                                     &response);
+  SendPayloadToCrashHandlerServer(ipc_pipe_,
+                                  message,
+                                  attachment.value().c_str(),
+                                  static_cast<uint32_t>(path_length_bytes),
+                                  &response);
 }
 
 void CrashpadClient::RequestRetry() {

handler/win/crash_report_exception_handler.cc

@@ -207,9 +207,34 @@
   attachments_.push_back(attachment);
 }
 
+void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentWritten(
+    const base::FilePath& attachment, const std::string& data) {
+  FileWriter writer;
+  if (!writer.Open(attachment,
+                   FileWriteMode::kTruncateOrCreate,
+                   FilePermissions::kOwnerOnly) ||
+      !writer.Write(data.data(), data.size())) {
+    LOG(ERROR) << "failed to write attachment " << attachment;
+    return;
+  }
+}
+
+void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentAppended(
+    const base::FilePath& attachment, const std::string& data) {
+  FileWriter writer;
+  if (!writer.Open(attachment,
+                   FileWriteMode::kReuseOrCreate,
+                   FilePermissions::kOwnerOnly) ||
+      writer.Seek(0, SEEK_END) < 0 ||
+      !writer.Write(data.data(), data.size())) {
+    LOG(ERROR) << "failed to write attachment " << attachment;
+    return;
+  }
+}
+
 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;

handler/win/crash_report_exception_handler.h

@@ -84,7 +84,11 @@
       WinVMAddress exception_information_address,
       WinVMAddress debug_critical_section_address) override;
   void ExceptionHandlerServerAttachmentAdded(
       const base::FilePath& attachment) override;
+  void ExceptionHandlerServerAttachmentWritten(
+      const base::FilePath& attachment, const std::string& data) override;
+  void ExceptionHandlerServerAttachmentAppended(
+      const base::FilePath& attachment, const std::string& data) override;
   void ExceptionHandlerServerAttachmentRemoved(
       const base::FilePath& attachment) override;
   void ExceptionHandlerServerRetryRequested() override;

util/win/exception_handler_server.cc

@@ -528,6 +528,99 @@
   LoggingWriteFile(service_context.pipe(), &response, sizeof(response));
 }
 
+static bool ReadAttachment(
+    const internal::PipeServiceContext& service_context,
+    const ClientToServerMessage& message,
+    base::FilePath* attachment,
+    std::string* payload) {
+  const uint32_t path_length_bytes =
+      message.attachment_write.path_length_bytes;
+  const uint32_t payload_length_bytes =
+      message.attachment_write.payload_length_bytes;
+
+  if (path_length_bytes == 0 || path_length_bytes > kMaxPathBytes ||
+      path_length_bytes % sizeof(wchar_t) != 0) {
+    LOG(ERROR) << "Invalid path length: " << path_length_bytes;
+    return false;
+  }
+  if (payload_length_bytes > kMaxAttachmentPayloadBytes) {
+    LOG(ERROR) << "Invalid attachment payload length: "
+               << payload_length_bytes;
+    return false;
+  }
+  if (payload_length_bytes > UINT32_MAX - path_length_bytes) {
+    LOG(ERROR) << "Invalid attachment write length";
+    return false;
+  }
+
+  const uint32_t request_payload_length_bytes =
+      path_length_bytes + payload_length_bytes;
+  std::string request_payload(request_payload_length_bytes, '\0');
+  if (!LoggingReadFileExactly(
+          service_context.pipe(),
+          &request_payload[0],
+          request_payload_length_bytes)) {
+    LOG(ERROR) << "Failed to read attachment write";
+    return false;
+  }
+
+  const size_t path_length = path_length_bytes / sizeof(wchar_t) - 1;
+  std::wstring path(path_length, L'\0');
+  if (path_length > 0) {
+    memcpy(&path[0],
+           request_payload.data(),
+           path_length_bytes - sizeof(wchar_t));
+  }
+
+  *attachment = base::FilePath(path);
+  *payload =
+      std::string(request_payload.data() + path_length_bytes,
+                  payload_length_bytes);
+  return true;
+}
+
+static void HandleWriteAttachment(
+    const internal::PipeServiceContext& service_context,
+    const ClientToServerMessage& message) {
+  base::FilePath attachment;
+  std::string payload;
+  if (!ReadAttachment(
+          service_context, message, &attachment, &payload)) {
+    return;
+  }
+
+  // Acknowledge IPC payload acceptance before disk I/O. This message exists to
+  // offload potentially slow disk I/O from the client; waiting for the file
+  // write would make the client block on the work this API is meant to avoid.
+  ServerToClientMessage response = {};
+  if (!LoggingWriteFile(service_context.pipe(), &response, sizeof(response))) {
+    return;
+  }
+  service_context.delegate()->ExceptionHandlerServerAttachmentWritten(
+      attachment, payload);
+}
+
+static void HandleAppendAttachment(
+    const internal::PipeServiceContext& service_context,
+    const ClientToServerMessage& message) {
+  base::FilePath attachment;
+  std::string payload;
+  if (!ReadAttachment(
+          service_context, message, &attachment, &payload)) {
+    return;
+  }
+
+  // Acknowledge IPC payload acceptance before disk I/O. This message exists to
+  // offload potentially slow disk I/O from the client; waiting for the file
+  // append would make the client block on the work this API is meant to avoid.
+  ServerToClientMessage response = {};
+  if (!LoggingWriteFile(service_context.pipe(), &response, sizeof(response))) {
+    return;
+  }
+  service_context.delegate()->ExceptionHandlerServerAttachmentAppended(
+      attachment, payload);
+}
+
 // This function must be called with service_context.pipe() already connected to
 // a client pipe. It exchanges data with the client and adds a ClientData record
 // to service_context->clients().
@@ -611,6 +704,22 @@
       return false;
     }
 
+    case ClientToServerMessage::kWriteAttachment: {
+      if (!RuntimeMessageOriginIsOwner(service_context)) {
+        return false;
+      }
+      HandleWriteAttachment(service_context, message);
+      return false;
+    }
+
+    case ClientToServerMessage::kAppendAttachment: {
+      if (!RuntimeMessageOriginIsOwner(service_context)) {
+        return false;
+      }
+      HandleAppendAttachment(service_context, message);
+      return false;
+    }
+
     case ClientToServerMessage::kRequestRetry: {
       if (!RuntimeMessageOriginIsOwner(service_context)) {
         return false;