This adds parent path and file name to the Payload constructor. This will affect chrome.

PiperOrigin-RevId: 435223740

connections/implementation/BUILD

@@ -129,6 +129,8 @@ cc_library(
         "//internal/platform:types",
         "//internal/platform:util",
         "//internal/platform/implementation:comm",
+        "//internal/platform/implementation:platform",
+        "//internal/platform/implementation/shared:file",
         "//proto:connections_enums_cc_proto",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/container:btree",

connections/implementation/internal_payload.h

@@ -42,6 +42,9 @@ class InternalPayload {
 
   Payload::Id GetId() const;
 
+  const std::string& GetParentFolder() { return payload_.GetParentFolder(); }
+  const std::string& GetFileName() { return payload_.GetFileName(); }
+
   // Returns the PayloadType of the Payload to which this object is bound.
   //
   // <p>Note that this is supposed to return the type from the OfflineFrame

connections/implementation/internal_payload_factory.cc

@@ -18,13 +18,18 @@
 #include <memory>
 
 #include "absl/memory/memory.h"
+#include "connections/implementation/offline_frames_validator.h"
 #include "connections/payload.h"
 #include "internal/platform/byte_array.h"
-#include "internal/platform/exception.h"
 #include "internal/platform/condition_variable.h"
+#include "internal/platform/exception.h"
+#include "internal/platform/feature_flags.h"
 #include "internal/platform/file.h"
+#include "internal/platform/implementation/platform.h"
+#include "internal/platform/implementation/shared/file.h"
 #include "internal/platform/logging.h"
 #include "internal/platform/mutex.h"
+#include "internal/platform/os_name.h"
 #include "internal/platform/pipe.h"
 
 namespace location {
@@ -285,6 +290,9 @@ class IncomingFileInternalPayload : public InternalPayload {
 
 }  // namespace
 
+using location::nearby::api::ImplementationPlatform;
+using location::nearby::api::OSName;
+
 std::unique_ptr<InternalPayload> CreateOutgoingInternalPayload(
     Payload payload) {
   switch (payload.GetType()) {
@@ -292,10 +300,6 @@ std::unique_ptr<InternalPayload> CreateOutgoingInternalPayload(
       return absl::make_unique<BytesInternalPayload>(std::move(payload));
 
     case Payload::Type::kFile: {
-      InputFile* file = payload.AsFile();
-      const PayloadId file_payload_id = file ? file->GetPayloadId() : 0;
-      const PayloadId payload_id = payload.GetId();
-      CHECK(payload_id == file_payload_id);
       return absl::make_unique<OutgoingFileInternalPayload>(std::move(payload));
     }
 
@@ -309,6 +313,15 @@ std::unique_ptr<InternalPayload> CreateOutgoingInternalPayload(
   }
 }
 
+std::string make_path(std::string& parent_folder, std::string& file_name) {
+  return api::ImplementationPlatform::GetDownloadPath(parent_folder, file_name);
+}
+
+std::string make_path(std::string& parent_folder, int64_t id) {
+  std::string file_name(std::to_string(id));
+  return api::ImplementationPlatform::GetDownloadPath(parent_folder, file_name);
+}
+
 std::unique_ptr<InternalPayload> CreateIncomingInternalPayload(
     const PayloadTransferFrame& frame) {
   if (frame.packet_type() != PayloadTransferFrame::DATA) {
@@ -334,10 +347,34 @@ std::unique_ptr<InternalPayload> CreateIncomingInternalPayload(
     }
 
     case PayloadTransferFrame::PayloadHeader::FILE: {
-      std::int64_t total_size = frame.payload_header().total_size();
-      return absl::make_unique<IncomingFileInternalPayload>(
-          Payload(payload_id, InputFile(payload_id, total_size)),
-          OutputFile(payload_id), total_size);
+      std::string file_path("");
+      int64_t total_size = 0;
+
+      if (frame.payload_header().has_parent_folder()) {
+        file_path = frame.payload_header().parent_folder();
+      }
+
+      if (frame.payload_header().has_file_name()) {
+        std::string file_name(frame.payload_header().file_name());
+        file_path = make_path(file_path, file_name);
+      }
+
+      if (frame.payload_header().has_total_size()) {
+        total_size = frame.payload_header().total_size();
+      }
+
+      // These are ordered, the output file must be created first otherwise
+      // there will be no input file to open.
+      // On Chrome the file path should be empty, so use the payload id.
+      if (ImplementationPlatform::GetCurrentOS() == OSName::kChromeOS) {
+        return absl::make_unique<IncomingFileInternalPayload>(
+            Payload(payload_id, InputFile(payload_id, total_size)),
+            OutputFile(payload_id), total_size);
+      } else {
+        return absl::make_unique<IncomingFileInternalPayload>(
+            Payload(payload_id, InputFile(file_path, total_size)),
+            OutputFile(file_path), total_size);
+      }
     }
     default:
       DCHECK(false);  // This should never happen.

connections/implementation/internal_payload_factory_test.cc

@@ -58,16 +58,15 @@ TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromStreamPayload) {
 
 TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFilePayload) {
   Payload::Id payload_id = Payload::GenerateId();
+  InputFile inputFile(payload_id, 512);
   std::unique_ptr<InternalPayload> internal_payload =
-      CreateOutgoingInternalPayload(
-          Payload{payload_id, InputFile(payload_id, 512)});
+      CreateOutgoingInternalPayload(Payload{payload_id, std::move(inputFile)});
   EXPECT_NE(internal_payload, nullptr);
   Payload payload = internal_payload->ReleasePayload();
   EXPECT_NE(payload.AsFile(), nullptr);
   EXPECT_EQ(payload.AsStream(), nullptr);
   EXPECT_EQ(payload.AsBytes(), ByteArray());
   EXPECT_EQ(payload.GetId(), payload_id);
-  EXPECT_EQ(payload.AsFile()->GetPayloadId(), payload_id);
 }
 
 TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromByteMessage) {
@@ -125,7 +124,6 @@ TEST(InternalPayloadFActoryTest, CanCreateIternalPayloadFromFileMessage) {
   EXPECT_EQ(payload.AsStream(), nullptr);
   EXPECT_EQ(payload.AsBytes(), ByteArray());
   EXPECT_EQ(payload.GetType(), Payload::Type::kFile);
-  EXPECT_EQ(payload.GetId(), payload.AsFile()->GetPayloadId());
 }
 
 void CreateFileWithContents(Payload::Id payload_id, const ByteArray& contents) {
@@ -141,9 +139,9 @@ TEST(InternalPayloadFActoryTest,
   size_t size_after_skip = contents.size() - kOffset;
   Payload::Id payload_id = Payload::GenerateId();
   CreateFileWithContents(payload_id, contents);
+  InputFile inputFile(payload_id, contents.size());
   std::unique_ptr<InternalPayload> internal_payload =
-      CreateOutgoingInternalPayload(
-          Payload{payload_id, InputFile(payload_id, contents.size())});
+      CreateOutgoingInternalPayload(Payload{payload_id, std::move(inputFile)});
   EXPECT_NE(internal_payload, nullptr);
 
   ExceptionOr<size_t> result = internal_payload->SkipToOffset(kOffset);

connections/implementation/offline_frames_validator.cc

@@ -16,9 +16,10 @@
 
 #include <regex>  //NOLINT
 
-#include "connections/implementation/proto/offline_wire_formats.pb.h"
 #include "connections/implementation/internal_payload.h"
 #include "connections/implementation/offline_frames.h"
+#include "connections/implementation/proto/offline_wire_formats.pb.h"
+#include "internal/platform/implementation/platform.h"
 
 namespace location {
 namespace nearby {
@@ -330,6 +331,29 @@ Exception EnsureValidBandwidthUpgradeNegotiationFrame(
   return {Exception::kSuccess};
 }
 
+bool CheckForIllegalCharacters(std::string toBeValidated,
+                               std::vector<std::string> illegalPatterns) {
+  if (toBeValidated.empty()) {
+    return false;
+  }
+
+  CHECK_GT(illegalPatterns.size(), 0);
+
+  return std::any_of(illegalPatterns.begin(), illegalPatterns.end(),
+                     [&toBeValidated](const auto& s) {
+                       size_t found = toBeValidated.find(s);
+                       if (found != std::string::npos) {
+                         // TODO(jfcarroll): Find a way to log messages
+                         //                  here.
+                         // NEARBY_LOGS(ERROR)
+                         //    << "Illegal character sequence found: \""
+                         //    << toBeValidated[found] << "\"";
+                         return true;
+                       }
+                       return false;
+                     });
+}
+
 }  // namespace
 
 Exception EnsureValidOfflineFrame(const OfflineFrame& offline_frame) {
@@ -352,6 +376,36 @@ Exception EnsureValidOfflineFrame(const OfflineFrame& offline_frame) {
       return {Exception::kInvalidProtocolBuffer};
 
     case V1Frame::PAYLOAD_TRANSFER:
+      if (offline_frame.has_v1() &&
+          (offline_frame.v1().payload_transfer().payload_header().has_type() &&
+           offline_frame.v1().payload_transfer().payload_header().type() ==
+               PayloadTransferFrame_PayloadHeader_PayloadType::
+                   PayloadTransferFrame_PayloadHeader_PayloadType_FILE)) {
+        if (offline_frame.v1()
+                .payload_transfer()
+                .payload_header()
+                .has_file_name()) {
+          if (CheckForIllegalCharacters(offline_frame.v1()
+                                            .payload_transfer()
+                                            .payload_header()
+                                            .file_name(),
+                                        kIllegalFileNamePatterns)) {
+            return {Exception::kIllegalCharacters};
+          }
+        }
+        if (offline_frame.v1()
+                .payload_transfer()
+                .payload_header()
+                .has_parent_folder()) {
+          if (CheckForIllegalCharacters(offline_frame.v1()
+                                            .payload_transfer()
+                                            .payload_header()
+                                            .parent_folder(),
+                                        kIllegalParentFolderPatterns)) {
+            return {Exception::kIllegalCharacters};
+          }
+        }
+      }
       if (offline_frame.has_v1() && offline_frame.v1().has_payload_transfer()) {
         return EnsureValidPayloadTransferFrame(
             offline_frame.v1().payload_transfer());

connections/implementation/offline_frames_validator.h

@@ -23,6 +23,23 @@ namespace nearby {
 namespace connections {
 namespace parser {
 
+#ifdef NEARBY_CHROMIUM
+const std::vector<std::string> kIllegalFileNamePatterns{
+    "/", "\\", "?",  "*",  "\"", "<",  ">",
+    "|", ":",  "..", "\n", "\r", "\t", "\f"};
+
+const std::vector<std::string> kIllegalParentFolderPatterns{
+    "\\", "?", "*", "\"", "<", ">", "|", ":", "..", "\n", "\r", "\t", "\f"};
+#else
+const std::vector<std::string> kIllegalFileNamePatterns{
+    "/", "\\", "?", "*", "\"", "<",  ">",  "|",  "[",
+    "]", ":",  ",", ";", "..", "\n", "\r", "\t", "\f"};
+
+const std::vector<std::string> kIllegalParentFolderPatterns{
+    "\\", "?", "*", "\"", "<",  ">",  "|",  "[", "]",
+    ":",  ",", ";", "..", "\n", "\r", "\t", "\f"};
+#endif
+
 Exception EnsureValidOfflineFrame(const OfflineFrame& offline_frame);
 
 }  // namespace parser

connections/implementation/offline_frames_validator_test.cc

@@ -213,6 +213,131 @@ TEST(OfflineFramesValidatorTest, ValidatesAsOkWithValidPayloadTransferFrame) {
   ASSERT_TRUE(ret_value.Ok());
 }
 
+TEST(OfflineFramesValidatorTest,
+     ValidatesAsOkTypeFileWithEmptyFilePathAndParent) {
+  PayloadTransferFrame::PayloadHeader header;
+  PayloadTransferFrame::PayloadChunk chunk;
+  header.set_id(12345);
+  header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
+  // Sending files larger than 2gb was previously broken (see cl/372382338).
+  // This tests a file larger than int max.
+  header.set_total_size(3e10);
+  header.set_file_name(std::string());
+  header.set_parent_folder(std::string());
+  chunk.set_body("payload data");
+  chunk.set_offset(150);
+  chunk.set_flags(1);
+
+  OfflineFrame offline_frame;
+
+  ByteArray bytes = ForDataPayloadTransfer(header, chunk);
+  offline_frame.ParseFromString(std::string(bytes));
+
+  auto ret_value = EnsureValidOfflineFrame(offline_frame);
+
+  ASSERT_TRUE(ret_value.Ok());
+}
+
+TEST(OfflineFramesValidatorTest, ValidatesAsOkTypeFileWithLegalFilePath) {
+  PayloadTransferFrame::PayloadHeader header;
+  PayloadTransferFrame::PayloadChunk chunk;
+  header.set_id(12345);
+  header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
+  // Sending files larger than 2gb was previously broken (see cl/372382338).
+  // This tests a file larger than int max.
+  header.set_total_size(3e10);
+  header.set_file_name(
+      std::string("earth_85MB_test (1) (3) (4) (8) (1) (2) (2) (1).jpg"));
+  header.set_parent_folder(std::string());
+  chunk.set_body("payload data");
+  chunk.set_offset(150);
+  chunk.set_flags(1);
+
+  OfflineFrame offline_frame;
+
+  ByteArray bytes = ForDataPayloadTransfer(header, chunk);
+  offline_frame.ParseFromString(std::string(bytes));
+
+  auto ret_value = EnsureValidOfflineFrame(offline_frame);
+
+  ASSERT_TRUE(ret_value.Ok());
+}
+
+TEST(OfflineFramesValidatorTest, ValidatesAsFailedTypeFileWithIllegalFilePath) {
+  PayloadTransferFrame::PayloadHeader header;
+  PayloadTransferFrame::PayloadChunk chunk;
+  header.set_id(12345);
+  header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
+  // Sending files larger than 2gb was previously broken (see cl/372382338).
+  // This tests a file larger than int max.
+  header.set_total_size(3e10);
+  header.set_file_name(
+      std::string("earth_85MB_test (1): (3) (4) (8) (1) (2) (2) (1).jpg"));
+  header.set_parent_folder(std::string());
+  chunk.set_body("payload data");
+  chunk.set_offset(150);
+  chunk.set_flags(1);
+
+  OfflineFrame offline_frame;
+
+  ByteArray bytes = ForDataPayloadTransfer(header, chunk);
+  offline_frame.ParseFromString(std::string(bytes));
+
+  auto ret_value = EnsureValidOfflineFrame(offline_frame);
+
+  ASSERT_TRUE(ret_value.value == Exception::kIllegalCharacters);
+}
+
+TEST(OfflineFramesValidatorTest, ValidatesAsOkTypeFileWithLegalParentFolder) {
+  PayloadTransferFrame::PayloadHeader header;
+  PayloadTransferFrame::PayloadChunk chunk;
+  header.set_id(12345);
+  header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
+  // Sending files larger than 2gb was previously broken (see cl/372382338).
+  // This tests a file larger than int max.
+  header.set_total_size(3e10);
+  header.set_file_name("");
+  header.set_parent_folder(std::string(
+      std::string("earth_85MB_test (1) (3) (4) (8) (1) (2) (2) (1).jpg")));
+  chunk.set_body("payload data");
+  chunk.set_offset(150);
+  chunk.set_flags(1);
+
+  OfflineFrame offline_frame;
+
+  ByteArray bytes = ForDataPayloadTransfer(header, chunk);
+  offline_frame.ParseFromString(std::string(bytes));
+
+  auto ret_value = EnsureValidOfflineFrame(offline_frame);
+
+  ASSERT_TRUE(ret_value.Ok());
+}
+
+TEST(OfflineFramesValidatorTest,
+     ValidatesAsFailedTypeFileWithIllegalParentFolder) {
+  PayloadTransferFrame::PayloadHeader header;
+  PayloadTransferFrame::PayloadChunk chunk;
+  header.set_id(12345);
+  header.set_type(PayloadTransferFrame::PayloadHeader::FILE);
+  // Sending files larger than 2gb was previously broken (see cl/372382338).
+  // This tests a file larger than int max.
+  header.set_total_size(3e10);
+  header.set_file_name("");
+  header.set_parent_folder(std::string(
+      std::string("earth_85MB_test (1): (3) (4) (8) (1) (2) (2) (1).jpg")));
+  chunk.set_body("payload data");
+  chunk.set_offset(150);
+  chunk.set_flags(1);
+
+  OfflineFrame offline_frame;
+
+  ByteArray bytes = ForDataPayloadTransfer(header, chunk);
+  offline_frame.ParseFromString(std::string(bytes));
+
+  auto ret_value = EnsureValidOfflineFrame(offline_frame);
+
+  ASSERT_TRUE(ret_value.value == Exception::kIllegalCharacters);
+}
 TEST(OfflineFramesValidatorTest, ValidatesAsFailWithNullPayloadTransferFrame) {
   PayloadTransferFrame::PayloadHeader header;
   PayloadTransferFrame::PayloadChunk chunk;

connections/implementation/payload_manager.cc

@@ -418,7 +418,10 @@ void PayloadManager::SendPayload(ClientProxy* client,
                                       internal_payload->GetTotalSize());
 
         PayloadTransferFrame::PayloadHeader payload_header{
-            CreatePayloadHeader(*internal_payload, resume_offset)};
+            CreatePayloadHeader(*internal_payload, resume_offset,
+                                internal_payload->GetParentFolder(),
+                                internal_payload->GetFileName())};
+
         bool should_continue = true;
         std::int64_t next_chunk_offset = 0;
         while (should_continue && !shutdown_.Get()) {
@@ -610,12 +613,19 @@ int PayloadManager::GetOptimalChunkSize(EndpointIds endpoint_ids) {
 }
 
 PayloadTransferFrame::PayloadHeader PayloadManager::CreatePayloadHeader(
-    const InternalPayload& internal_payload, size_t offset) {
+    const InternalPayload& internal_payload, size_t offset,
+    const std::string& parent_folder, const std::string& file_name) {
   PayloadTransferFrame::PayloadHeader payload_header;
   size_t payload_size = internal_payload.GetTotalSize();
 
   payload_header.set_id(internal_payload.GetId());
   payload_header.set_type(internal_payload.GetType());
+  if (internal_payload.GetType() ==
+      location::nearby::connections::PayloadTransferFrame::PayloadHeader::
+          PayloadType::PayloadTransferFrame_PayloadHeader_PayloadType_FILE) {
+    payload_header.set_file_name(file_name);
+    payload_header.set_parent_folder(parent_folder);
+  }
   payload_header.set_total_size(payload_size ==
                                         InternalPayload::kIndeterminateSize
                                     ? InternalPayload::kIndeterminateSize

connections/implementation/payload_manager.h

@@ -211,7 +211,9 @@ class PayloadManager : public EndpointManager::FrameProcessor {
   int GetOptimalChunkSize(EndpointIds endpoint_ids);
 
   PayloadTransferFrame::PayloadHeader CreatePayloadHeader(
-      const InternalPayload& payload, size_t offset);
+      const InternalPayload& internal_payload, size_t offset,
+      const std::string& parent_folder, const std::string& file_name);
+
   PayloadTransferFrame::PayloadChunk CreatePayloadChunk(std::int64_t offset,
                                                         ByteArray body);