From 517d77f5aa4fea8f4437125830cfc55f84705e3d Mon Sep 17 00:00:00 2001 From: jfcarroll Date: Wed, 16 Mar 2022 19:17:23 -0700 Subject: [PATCH] This adds parent path and file name to the Payload constructor. This will affect chrome. PiperOrigin-RevId: 435223740 --- connections/clients/windows/BUILD | 2 + .../clients/windows/dart/core_adapter_dart.cc | 11 +- connections/connection_options.h | 3 - connections/implementation/BUILD | 2 + connections/implementation/internal_payload.h | 3 + .../internal_payload_factory.cc | 55 ++- .../internal_payload_factory_test.cc | 10 +- .../offline_frames_validator.cc | 56 ++- .../implementation/offline_frames_validator.h | 17 + .../offline_frames_validator_test.cc | 125 ++++++ connections/implementation/payload_manager.cc | 14 +- connections/implementation/payload_manager.h | 4 +- connections/payload.cc | 23 +- connections/payload.h | 31 +- internal/platform/BUILD | 1 + internal/platform/exception.h | 4 +- internal/platform/file.cc | 23 +- internal/platform/file.h | 20 +- internal/platform/implementation/BUILD | 1 + .../platform/implementation/g3/platform.cc | 78 +++- .../ios/Source/Internal/GNCCoreConnection.mm | 3 +- .../ios/Source/Internal/platform.mm | 32 +- .../ios/Source/Platform/input_file.h | 6 + .../ios/Source/Platform/input_file.mm | 5 + internal/platform/implementation/platform.h | 17 +- internal/platform/implementation/shared/BUILD | 1 + .../platform/implementation/shared/file.cc | 17 +- .../platform/implementation/shared/file.h | 14 +- .../platform/implementation/windows/BUILD | 2 +- .../implementation/windows/platform.cc | 95 +++- .../implementation/windows/platform_test.cc | 415 ++++++++++++++++++ .../implementation/windows/test_utils.cc | 20 +- internal/platform/os_name.h | 28 ++ 33 files changed, 1017 insertions(+), 121 deletions(-) create mode 100644 internal/platform/implementation/windows/platform_test.cc create mode 100644 internal/platform/os_name.h diff --git a/connections/clients/windows/BUILD b/connections/clients/windows/BUILD index 79969e63..357d3e6b 100644 --- a/connections/clients/windows/BUILD +++ b/connections/clients/windows/BUILD @@ -30,6 +30,7 @@ cc_windows_dll( deps = [ "//third_party/dart_lang/v2:dart_api_dl", "//connections:core", + "//internal/platform:base", "//internal/platform/implementation/windows", "//third_party/webrtc/files/stable/webrtc/api:create_peerconnection_factory", "@com_google_absl//absl/strings", @@ -54,6 +55,7 @@ cc_windows_dll( deps = [ "//third_party/dart_lang/v2:dart_api_dl", "//connections:core", + "//internal/platform:base", "//internal/platform/implementation/windows", "//third_party/webrtc/files/stable/webrtc/api:create_peerconnection_factory", "@com_google_absl//absl/strings", diff --git a/connections/clients/windows/dart/core_adapter_dart.cc b/connections/clients/windows/dart/core_adapter_dart.cc index 9697ed38..a2da7d68 100644 --- a/connections/clients/windows/dart/core_adapter_dart.cc +++ b/connections/clients/windows/dart/core_adapter_dart.cc @@ -567,12 +567,11 @@ void SendPayloadDart(Core *pCore, const char *endpoint_id, /*FailIfFileAlreadyExists=*/ false); NEARBY_LOGS(INFO) << "Copy File to " << download_path; - InputFile input_file{id, payload_dart.size}; - { - Payload payload{id, std::move(input_file)}; - SendPayload(pCore, absl::Span(endpoint_ids), - std::move(payload), callback); - } + InputFile input_file(std::to_string(id), payload_dart.size); + Payload payload = Payload(id, std::move(input_file)); + SendPayload(pCore, absl::Span(endpoint_ids), + std::move(payload), callback); + SetResultCallback(callback, result_cb); break; diff --git a/connections/connection_options.h b/connections/connection_options.h index 0afd2a21..75fabd26 100644 --- a/connections/connection_options.h +++ b/connections/connection_options.h @@ -15,10 +15,7 @@ #define CORE_CONNECTION_OPTIONS_H_ #include -#include "connections/medium_selector.h" #include "connections/options_base.h" -#include "connections/power_level.h" -#include "connections/strategy.h" #include "internal/platform/byte_array.h" #include "proto/connections_enums.pb.h" diff --git a/connections/implementation/BUILD b/connections/implementation/BUILD index d1bb5f1d..8b2e4943 100644 --- a/connections/implementation/BUILD +++ b/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", diff --git a/connections/implementation/internal_payload.h b/connections/implementation/internal_payload.h index ced64eff..9d0de221 100644 --- a/connections/implementation/internal_payload.h +++ b/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. // //

Note that this is supposed to return the type from the OfflineFrame diff --git a/connections/implementation/internal_payload_factory.cc b/connections/implementation/internal_payload_factory.cc index 61699f92..f5994752 100644 --- a/connections/implementation/internal_payload_factory.cc +++ b/connections/implementation/internal_payload_factory.cc @@ -18,13 +18,18 @@ #include #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 CreateOutgoingInternalPayload( Payload payload) { switch (payload.GetType()) { @@ -292,10 +300,6 @@ std::unique_ptr CreateOutgoingInternalPayload( return absl::make_unique(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(std::move(payload)); } @@ -309,6 +313,15 @@ std::unique_ptr 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 CreateIncomingInternalPayload( const PayloadTransferFrame& frame) { if (frame.packet_type() != PayloadTransferFrame::DATA) { @@ -334,10 +347,34 @@ std::unique_ptr CreateIncomingInternalPayload( } case PayloadTransferFrame::PayloadHeader::FILE: { - std::int64_t total_size = frame.payload_header().total_size(); - return absl::make_unique( - 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( + Payload(payload_id, InputFile(payload_id, total_size)), + OutputFile(payload_id), total_size); + } else { + return absl::make_unique( + Payload(payload_id, InputFile(file_path, total_size)), + OutputFile(file_path), total_size); + } } default: DCHECK(false); // This should never happen. diff --git a/connections/implementation/internal_payload_factory_test.cc b/connections/implementation/internal_payload_factory_test.cc index 7ef7bd2f..b5b28139 100644 --- a/connections/implementation/internal_payload_factory_test.cc +++ b/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 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 internal_payload = - CreateOutgoingInternalPayload( - Payload{payload_id, InputFile(payload_id, contents.size())}); + CreateOutgoingInternalPayload(Payload{payload_id, std::move(inputFile)}); EXPECT_NE(internal_payload, nullptr); ExceptionOr result = internal_payload->SkipToOffset(kOffset); diff --git a/connections/implementation/offline_frames_validator.cc b/connections/implementation/offline_frames_validator.cc index 00260f55..7f2bd15e 100644 --- a/connections/implementation/offline_frames_validator.cc +++ b/connections/implementation/offline_frames_validator.cc @@ -16,9 +16,10 @@ #include //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 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()); diff --git a/connections/implementation/offline_frames_validator.h b/connections/implementation/offline_frames_validator.h index 14a5de75..0df165dc 100644 --- a/connections/implementation/offline_frames_validator.h +++ b/connections/implementation/offline_frames_validator.h @@ -23,6 +23,23 @@ namespace nearby { namespace connections { namespace parser { +#ifdef NEARBY_CHROMIUM +const std::vector kIllegalFileNamePatterns{ + "/", "\\", "?", "*", "\"", "<", ">", + "|", ":", "..", "\n", "\r", "\t", "\f"}; + +const std::vector kIllegalParentFolderPatterns{ + "\\", "?", "*", "\"", "<", ">", "|", ":", "..", "\n", "\r", "\t", "\f"}; +#else +const std::vector kIllegalFileNamePatterns{ + "/", "\\", "?", "*", "\"", "<", ">", "|", "[", + "]", ":", ",", ";", "..", "\n", "\r", "\t", "\f"}; + +const std::vector kIllegalParentFolderPatterns{ + "\\", "?", "*", "\"", "<", ">", "|", "[", "]", + ":", ",", ";", "..", "\n", "\r", "\t", "\f"}; +#endif + Exception EnsureValidOfflineFrame(const OfflineFrame& offline_frame); } // namespace parser diff --git a/connections/implementation/offline_frames_validator_test.cc b/connections/implementation/offline_frames_validator_test.cc index 3e069733..e70e251d 100644 --- a/connections/implementation/offline_frames_validator_test.cc +++ b/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; diff --git a/connections/implementation/payload_manager.cc b/connections/implementation/payload_manager.cc index 7847df0d..eb3188a5 100644 --- a/connections/implementation/payload_manager.cc +++ b/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 diff --git a/connections/implementation/payload_manager.h b/connections/implementation/payload_manager.h index 8b5994da..58cadd15 100644 --- a/connections/implementation/payload_manager.h +++ b/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); diff --git a/connections/payload.cc b/connections/payload.cc index 4979d054..c0b32a0f 100644 --- a/connections/payload.cc +++ b/connections/payload.cc @@ -33,11 +33,19 @@ Payload::Payload(ByteArray&& bytes) : content_(std::move(bytes)) {} Payload::Payload(const ByteArray& bytes) : content_(bytes) {} -Payload::Payload(InputFile file) - : content_(std::move(file)), - id_(std::hash()(file.GetFilePath())) {} +Payload::Payload(InputFile input_file) + : content_(std::move(input_file)), + id_(std::hash()(input_file.GetFilePath())) {} + +Payload::Payload(Id id, InputFile input_file) + : content_(std::move(input_file)), id_(id) {} + +Payload::Payload(std::string parent_folder, std::string file_name, + InputFile input_file) + : content_(std::move(input_file)), + parent_folder_(parent_folder), + file_name_(file_name) {} -// TODO(jfcarroll): Convert std::function to function pointer Payload::Payload(std::function stream) : content_(std::move(stream)) {} @@ -47,9 +55,6 @@ Payload::Payload(Id id, ByteArray&& bytes) Payload::Payload(Id id, const ByteArray& bytes) : content_(bytes), id_(id) {} -Payload::Payload(Id id, InputFile file) : content_(std::move(file)), id_(id) {} - -// TODO(jfcarroll): Convert std::function to function pointer Payload::Payload(Id id, std::function stream) : content_(std::move(stream)), id_(id) {} @@ -96,6 +101,10 @@ Payload::Type Payload::FindType() const { return static_cast(content_.index()); } +const std::string& Payload::GetParentFolder() const { return parent_folder_; } + +const std::string& Payload::GetFileName() const { return file_name_; } + } // namespace connections } // namespace nearby } // namespace location diff --git a/connections/payload.h b/connections/payload.h index e82505a0..a80bec21 100644 --- a/connections/payload.h +++ b/connections/payload.h @@ -22,12 +22,12 @@ #include "absl/types/variant.h" #include "internal/platform/byte_array.h" -#include "internal/platform/input_stream.h" -#include "internal/platform/payload_id.h" -#include "internal/platform/prng.h" #include "internal/platform/core_config.h" #include "internal/platform/file.h" +#include "internal/platform/input_stream.h" #include "internal/platform/logging.h" +#include "internal/platform/payload_id.h" +#include "internal/platform/prng.h" namespace location { namespace nearby { @@ -56,7 +56,23 @@ class DLL_API Payload { explicit Payload(ByteArray&& bytes); explicit Payload(const ByteArray& bytes); - explicit Payload(InputFile file); + explicit Payload(InputFile input_file); + + // InputFile is just "a pointer to a file on your disc", a wrapper around a + // file name or file descriptor. It has no understanding that Nearby is going + // to create a copy of it on the remote device. + // + // FileName and ParentFolder are what Nearby is saying the remote device + // should save this incoming payload as. + // + // Notably, FileName does not have to be respected (you could ask to save it + // as "photo.png" but instead the recipient saves it as "photo (1).png"). + // + // ParentFolder must be a relative path, not a full path. + + explicit Payload(std::string parent_folder, std::string file_name, + InputFile file); + explicit Payload(std::function stream); // Constructors for incoming payloads. @@ -65,7 +81,6 @@ class DLL_API Payload { Payload(Id id, InputFile file); Payload(Id id, std::function stream); - // Returns ByteArray payload, if it has been defined, or empty ByteArray. const ByteArray& AsBytes() const&; ByteArray&& AsBytes() &&; @@ -88,6 +103,9 @@ class DLL_API Payload { // Generate Payload Id; to be passed to outgoing file constructor. static Id GenerateId(); + const std::string& GetFileName() const; + const std::string& GetParentFolder() const; + private: Type FindType() const; @@ -95,6 +113,9 @@ class DLL_API Payload { Id id_{GenerateId()}; Type type_{FindType()}; size_t offset_{0}; + + std::string parent_folder_; + std::string file_name_; }; } // namespace connections diff --git a/internal/platform/BUILD b/internal/platform/BUILD index 5f47fe4f..87210560 100644 --- a/internal/platform/BUILD +++ b/internal/platform/BUILD @@ -35,6 +35,7 @@ cc_library( "input_stream.h", "listeners.h", "nsd_service_info.h", + "os_name.h", "output_stream.h", "payload_id.h", "prng.h", diff --git a/internal/platform/exception.h b/internal/platform/exception.h index e8bcd8c4..4a3e12f7 100644 --- a/internal/platform/exception.h +++ b/internal/platform/exception.h @@ -30,7 +30,9 @@ struct Exception { kInterrupted = 2, // Operation was interrupted. kInvalidProtocolBuffer = 3, // Couldn't parse. kExecution = 4, // Couldn't execute. - kTimeout = 5, // Operarion did not finish within specified time. + kTimeout = 5, // Operarion did not finish within specified time. + kIllegalCharacters = 6, // File name or parent path contained + // illegal chars }; bool Ok() const { return value == kSuccess; } bool Raised() const { return !Ok(); } diff --git a/internal/platform/file.cc b/internal/platform/file.cc index 6deaec25..c1516467 100644 --- a/internal/platform/file.cc +++ b/internal/platform/file.cc @@ -17,11 +17,13 @@ namespace location { namespace nearby { -InputFile::InputFile(PayloadId payload_id, std::int64_t size) - : impl_(Platform::CreateInputFile(payload_id, size)), id_(payload_id) {} +InputFile::InputFile(PayloadId id, std::int64_t size) + : impl_(Platform::CreateInputFile(id, size)) {} +InputFile::InputFile(std::string file_path, std::int64_t size) + : impl_(Platform::CreateInputFile(file_path, size)) {} InputFile::~InputFile() = default; -InputFile::InputFile(InputFile&&) noexcept = default; -InputFile& InputFile::operator=(InputFile&&) noexcept = default; +InputFile::InputFile(InputFile&& other) noexcept = default; +InputFile& InputFile::operator=(InputFile&& other) = default; // Reads up to size bytes and returns as a ByteArray object wrapped by // ExceptionOr. @@ -53,14 +55,12 @@ Exception InputFile::Close() { return impl_->Close(); } // versa. InputStream& InputFile::GetInputStream() { return *impl_; } -// Returns payload id of this file. The closest "file" equivalent is inode. -PayloadId InputFile::GetPayloadId() const { return id_; } - -OutputFile::OutputFile(PayloadId payload_id) - : impl_(Platform::CreateOutputFile(payload_id)), id_(payload_id) {} +OutputFile::OutputFile(std::string file_path) + : impl_(Platform::CreateOutputFile(file_path)) {} +OutputFile::OutputFile(PayloadId id) : impl_(Platform::CreateOutputFile(id)) {} OutputFile::~OutputFile() = default; OutputFile::OutputFile(OutputFile&&) noexcept = default; -OutputFile& OutputFile::operator=(OutputFile&&) noexcept = default; +OutputFile& OutputFile::operator=(OutputFile&&) = default; // Writes all data from ByteArray object to the underlying stream. // Returns Exception::kIo on error, Exception::kSuccess otherwise. @@ -85,8 +85,5 @@ Exception OutputFile::Close() { return impl_->Close(); } // versa. OutputStream& OutputFile::GetOutputStream() { return *impl_; } -// Returns payload id of this file. The closest "file" equivalent is inode. -PayloadId OutputFile::GetPayloadId() const { return id_; } - } // namespace nearby } // namespace location diff --git a/internal/platform/file.h b/internal/platform/file.h index 718d5414..e9b3f639 100644 --- a/internal/platform/file.h +++ b/internal/platform/file.h @@ -19,14 +19,14 @@ #include #include +#include "internal/platform/byte_array.h" +#include "internal/platform/core_config.h" +#include "internal/platform/exception.h" #include "internal/platform/implementation/input_file.h" #include "internal/platform/implementation/output_file.h" #include "internal/platform/implementation/platform.h" -#include "internal/platform/byte_array.h" -#include "internal/platform/exception.h" #include "internal/platform/input_stream.h" #include "internal/platform/output_stream.h" -#include "internal/platform/core_config.h" namespace location { namespace nearby { @@ -35,9 +35,10 @@ class DLL_API InputFile final { public: using Platform = api::ImplementationPlatform; InputFile(PayloadId payload_id, std::int64_t size); + InputFile(std::string file_path, std::int64_t size); ~InputFile(); InputFile(InputFile&&) noexcept; - InputFile& operator=(InputFile&&) noexcept; + InputFile& operator=(InputFile&&); // Reads up to size bytes and returns as a ByteArray object wrapped by // ExceptionOr. @@ -65,21 +66,18 @@ class DLL_API InputFile final { // versa. InputStream& GetInputStream(); - // Returns payload id of this file. The closest "file" equivalent is inode. - PayloadId GetPayloadId() const; - private: std::unique_ptr impl_; - PayloadId id_; }; class DLL_API OutputFile final { public: using Platform = api::ImplementationPlatform; explicit OutputFile(PayloadId payload_id); + explicit OutputFile(std::string file_path); ~OutputFile(); OutputFile(OutputFile&&) noexcept; - OutputFile& operator=(OutputFile&&) noexcept; + OutputFile& operator=(OutputFile&&); // Writes all data from ByteArray object to the underlying stream. // Returns Exception::kIo on error, Exception::kSuccess otherwise. @@ -102,12 +100,8 @@ class DLL_API OutputFile final { // versa. OutputStream& GetOutputStream(); - // Returns payload id of this file. The closest "file" equivalent is inode. - PayloadId GetPayloadId() const; - private: std::unique_ptr impl_; - PayloadId id_; }; } // namespace nearby diff --git a/internal/platform/implementation/BUILD b/internal/platform/implementation/BUILD index 75b9ad65..95d4d64e 100644 --- a/internal/platform/implementation/BUILD +++ b/internal/platform/implementation/BUILD @@ -83,6 +83,7 @@ cc_library( ], defines = ["NO_WEBRTC"], visibility = [ + "//connections/implementation:__subpackages__", "//googlemac/iPhone/Shared/Nearby/Connections:__subpackages__", "//internal/platform:__pkg__", "//internal/platform/implementation:__subpackages__", diff --git a/internal/platform/implementation/g3/platform.cc b/internal/platform/implementation/g3/platform.cc index 10139675..cb6046f1 100644 --- a/internal/platform/implementation/g3/platform.cc +++ b/internal/platform/implementation/g3/platform.cc @@ -57,11 +57,55 @@ namespace location { namespace nearby { namespace api { -namespace { -std::string GetPayloadPath(PayloadId payload_id) { - return absl::StrCat("/tmp/", payload_id); +std::string ImplementationPlatform::GetDownloadPath(std::string& parent_folder, + std::string& file_name) { + std::string fullPath("/tmp/"); + + // If parent_folder starts with a \\ or /, then strip it + while (!parent_folder.empty() && + (*parent_folder.begin() == '\\' || *parent_folder.begin() == '/')) { + parent_folder.erase(0, 1); + } + + // If parent_folder ends with a \\ or /, then strip it + while (!parent_folder.empty() && + (*parent_folder.rbegin() == '\\' || *parent_folder.rbegin() == '/')) { + parent_folder.erase(parent_folder.size() - 1); + } + + // If file_name starts with a \\, then strip it + while (!file_name.empty() && + (*file_name.begin() == '\\' || *file_name.begin() == '/')) { + file_name.erase(0, 1); + } + + // If file_name ends with a \\, then strip it + while (!file_name.empty() && + (*file_name.rbegin() == '\\' || *file_name.rbegin() == '/')) { + file_name.erase(file_name.size() - 1); + } + + std::stringstream path; + + if (parent_folder.empty() && file_name.empty()) { + path << fullPath.c_str(); + return path.str(); + } + if (parent_folder.empty()) { + path << fullPath.c_str() << "\\" << file_name.c_str(); + return path.str(); + } + if (file_name.empty()) { + path << fullPath.c_str() << "\\" << parent_folder.c_str(); + return path.str(); + } + + path << fullPath.c_str() << "\\" << parent_folder.c_str() << "\\" + << file_name.c_str(); + return path.str(); } -} // namespace + +OSName ImplementationPlatform::GetCurrentOS() { return OSName::kLinux; } int GetCurrentTid() { const LiveThread* my = Thread_GetMyLiveThread(); @@ -103,15 +147,33 @@ std::unique_ptr ImplementationPlatform::CreateAtomicBoolean( return std::make_unique(initial_value); } +ABSL_DEPRECATED("This interface will be deleted in the near future.") std::unique_ptr ImplementationPlatform::CreateInputFile( PayloadId payload_id, std::int64_t total_size) { - return shared::IOFile::CreateInputFile(GetPayloadPath(payload_id), - total_size); + std::string parent_folder(""); + std::string file_name(std::to_string(payload_id)); + return shared::IOFile::CreateInputFile( + GetDownloadPath(parent_folder, file_name), total_size); +} + +std::unique_ptr ImplementationPlatform::CreateInputFile( + absl::string_view file_path, size_t size) { + return shared::IOFile::CreateInputFile(file_path, size); +} + +ABSL_DEPRECATED("This interface will be deleted in the near future.") +std::unique_ptr ImplementationPlatform::CreateOutputFile( + PayloadId payload_id) { + std::string parent_folder(""); + std::string file_name(std::to_string(payload_id)); + + return shared::IOFile::CreateOutputFile( + GetDownloadPath(parent_folder, file_name)); } std::unique_ptr ImplementationPlatform::CreateOutputFile( - PayloadId payload_id) { - return shared::IOFile::CreateOutputFile(GetPayloadPath(payload_id)); + absl::string_view file_path) { + return shared::IOFile::CreateOutputFile(file_path); } std::unique_ptr ImplementationPlatform::CreateLogMessage( diff --git a/internal/platform/implementation/ios/Source/Internal/GNCCoreConnection.mm b/internal/platform/implementation/ios/Source/Internal/GNCCoreConnection.mm index a389763e..896372e5 100644 --- a/internal/platform/implementation/ios/Source/Internal/GNCCoreConnection.mm +++ b/internal/platform/implementation/ios/Source/Internal/GNCCoreConnection.mm @@ -161,7 +161,8 @@ class GNCInputStreamFromNSStream : public InputStream { PayloadId payloadId = payload.identifier; // Add the pair of payloadId and fileURL to the map in the GNCCore. [_core insertURLToMapWithPayloadID:payloadId urlToSend:fileURL]; - Payload corePayload(payloadId, InputFile(payloadId, fileSize)); + InputFile inputFile(payloadId, fileSize); + Payload corePayload(payloadId, std::move(inputFile)); progress.totalUnitCount = fileSize; return [self sendPayload:std::move(corePayload) size:fileSize diff --git a/internal/platform/implementation/ios/Source/Internal/platform.mm b/internal/platform/implementation/ios/Source/Internal/platform.mm index ada045ce..4c559165 100644 --- a/internal/platform/implementation/ios/Source/Internal/platform.mm +++ b/internal/platform/implementation/ios/Source/Internal/platform.mm @@ -37,18 +37,21 @@ namespace location { namespace nearby { namespace api { -namespace { -std::string GetPayloadPath(PayloadId payload_id) { +std::string ImplementationPlatform::GetDownloadPath(std::string& parent_folder, + std::string& file_name) { // This is to get a file path, e.g. /tmp/[payload_id], for the storage of payload file. // NOTE: Per // https://developer.apple.com/library/content/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/FileSystemOverview/FileSystemOverview.html // Files saved in the /tmp directory will be deleted by the system. Callers should be responsible // for copying the files to the permanent storage. - NSString* payloadIdString = ObjCStringFromCppString(std::to_string(payload_id)); + // TODO(jfcarroll): This needs to be done correctly, we now have a file name and parent folder, + // they should be combined with the default download path + NSString* payloadIdString = ObjCStringFromCppString(file_name); return CppStringFromObjCString( [NSTemporaryDirectory() stringByAppendingPathComponent:payloadIdString]); } -} // namespace + +OSName ImplementationPlatform::GetCurrentOS() { return OSName::kiOS; } // Atomics: std::unique_ptr ImplementationPlatform::CreateAtomicBoolean(bool initial_value) { @@ -78,6 +81,7 @@ std::unique_ptr ImplementationPlatform::CreateConditionVariab return std::make_unique(static_cast(mutex)); } +ABSL_DEPRECATED("This interface will be deleted in the near future.") std::unique_ptr ImplementationPlatform::CreateInputFile(PayloadId payload_id, std::int64_t total_size) { // Extract the NSURL object with payload_id from |GNCCore| which stores the maps. If the retrieved @@ -86,14 +90,28 @@ std::unique_ptr ImplementationPlatform::CreateInputFile(PayloadId pay GNCCore* core = GNCGetCore(); NSURL* url = [core extractURLWithPayloadID:payload_id]; if (url != nil) { - return absl::make_unique(url); + return std::make_unique(url); } else { - return shared::IOFile::CreateInputFile(GetPayloadPath(payload_id), total_size); + std::string parent_folder(""); + std::string file_name(std::to_string(payload_id)); + return shared::IOFile::CreateInputFile(GetDownloadPath(parent_folder, file_name), total_size); } } +std::unique_ptr ImplementationPlatform::CreateInputFile(absl::string_view file_path, + size_t size) { + return shared::IOFile::CreateInputFile(file_path, size); +} + +ABSL_DEPRECATED("This interface will be deleted in the near future.") std::unique_ptr ImplementationPlatform::CreateOutputFile(PayloadId payload_id) { - return shared::IOFile::CreateOutputFile(GetPayloadPath(payload_id)); + std::string parent_folder(""); + std::string file_name(std::to_string(payload_id)); + return shared::IOFile::CreateOutputFile(GetDownloadPath(parent_folder, file_name)); +} + +std::unique_ptr ImplementationPlatform::CreateOutputFile(absl::string_view file_path) { + return shared::IOFile::CreateOutputFile(file_path); } std::unique_ptr ImplementationPlatform::CreateLogMessage( diff --git a/internal/platform/implementation/ios/Source/Platform/input_file.h b/internal/platform/implementation/ios/Source/Platform/input_file.h index 6ccd59f7..cb2ca93e 100644 --- a/internal/platform/implementation/ios/Source/Platform/input_file.h +++ b/internal/platform/implementation/ios/Source/Platform/input_file.h @@ -17,6 +17,7 @@ #import +#include "absl/strings/string_view.h" #include "internal/platform/implementation/input_file.h" namespace location { @@ -27,6 +28,8 @@ namespace ios { class InputFile : public api::InputFile { public: explicit InputFile(NSURL *nsURL); + explicit InputFile(absl::string_view file_path, std::int64_t size); + ~InputFile() override = default; InputFile(InputFile &&) = default; InputFile &operator=(InputFile &&) = default; @@ -39,6 +42,9 @@ class InputFile : public api::InputFile { private: NSURL *nsURL_; NSInputStream *nsStream_; + + std::string path_; + size_t total_size_; }; } // namespace ios diff --git a/internal/platform/implementation/ios/Source/Platform/input_file.mm b/internal/platform/implementation/ios/Source/Platform/input_file.mm index 532f7e34..c4b0a30e 100644 --- a/internal/platform/implementation/ios/Source/Platform/input_file.mm +++ b/internal/platform/implementation/ios/Source/Platform/input_file.mm @@ -30,6 +30,11 @@ InputFile::InputFile(NSURL *nsURL) : nsURL_(nsURL) { [nsStream_ open]; } +InputFile::InputFile(absl::string_view file_path, std::int64_t size) + : path_(file_path), total_size_(size) { + // TODO(jfcarroll): This is not implemented for iOS yet. +} + ExceptionOr InputFile::Read(std::int64_t size) { uint8_t *bytes_read = new uint8_t[size]; NSUInteger numberOfBytesToRead = [[NSNumber numberWithLongLong:size] unsignedIntegerValue]; diff --git a/internal/platform/implementation/platform.h b/internal/platform/implementation/platform.h index 2bd8a147..c7929a6c 100644 --- a/internal/platform/implementation/platform.h +++ b/internal/platform/implementation/platform.h @@ -43,6 +43,7 @@ #endif #include "internal/platform/implementation/wifi.h" #include "internal/platform/implementation/wifi_lan.h" +#include "internal/platform/os_name.h" #include "internal/platform/payload_id.h" namespace location { @@ -62,6 +63,10 @@ class ImplementationPlatform { // - CountDownLatch : to ensure at least N threads are waiting. // - file I/O // - Logging + static std::string GetDownloadPath(std::string& parent_folder, + std::string& file_name); + + static OSName GetCurrentOS(); // Atomics: // ======= @@ -81,9 +86,15 @@ class ImplementationPlatform { static std::unique_ptr CreateMutex(Mutex::Mode mode); static std::unique_ptr CreateConditionVariable( Mutex* mutex); - static std::unique_ptr CreateInputFile(PayloadId payload_id, - std::int64_t total_size); - static std::unique_ptr CreateOutputFile(PayloadId payload_id); + + static std::unique_ptr CreateInputFile(PayloadId, std::int64_t); + + static std::unique_ptr CreateInputFile(absl::string_view, size_t); + + static std::unique_ptr CreateOutputFile(PayloadId); + + static std::unique_ptr CreateOutputFile(absl::string_view); + static std::unique_ptr CreateLogMessage( const char* file, int line, LogMessage::Severity severity); diff --git a/internal/platform/implementation/shared/BUILD b/internal/platform/implementation/shared/BUILD index 0260d0d5..d0f14a54 100644 --- a/internal/platform/implementation/shared/BUILD +++ b/internal/platform/implementation/shared/BUILD @@ -46,6 +46,7 @@ cc_library( srcs = ["file.cc"], hdrs = ["file.h"], visibility = [ + "//connections/implementation:__subpackages__", "//internal/platform/implementation:__subpackages__", ], deps = [ diff --git a/internal/platform/implementation/shared/file.cc b/internal/platform/implementation/shared/file.cc index 12a5e8d1..0d22d474 100644 --- a/internal/platform/implementation/shared/file.cc +++ b/internal/platform/implementation/shared/file.cc @@ -26,24 +26,25 @@ namespace nearby { namespace shared { // InputFile -std::unique_ptr IOFile::CreateInputFile(const absl::string_view path, - size_t size) { - return absl::WrapUnique(new IOFile(path, size)); +std::unique_ptr IOFile::CreateInputFile( + const absl::string_view file_path, size_t size) { + return absl::WrapUnique(new IOFile(file_path, size)); } -IOFile::IOFile(const absl::string_view path, size_t size) - : file_(std::string(path.data(), path.size()), +IOFile::IOFile(const absl::string_view file_path, size_t size) + : file_(std::string(file_path.data(), file_path.size()), std::ios::binary | std::ios::in), - path_(path), + path_(file_path), total_size_(size) {} std::unique_ptr IOFile::CreateOutputFile(const absl::string_view path) { return std::unique_ptr(new IOFile(path)); } -IOFile::IOFile(const absl::string_view path) - : file_(std::string(path.data(), path.size()), +IOFile::IOFile(const absl::string_view file_path) + : file_(std::string(file_path.data(), file_path.size()), std::ios::binary | std::ios::out | std::ios::trunc), + path_({file_path.data(), file_path.size()}), total_size_(0) {} ExceptionOr IOFile::Read(std::int64_t size) { diff --git a/internal/platform/implementation/shared/file.h b/internal/platform/implementation/shared/file.h index cfd912f5..2efcc6d4 100644 --- a/internal/platform/implementation/shared/file.h +++ b/internal/platform/implementation/shared/file.h @@ -19,9 +19,9 @@ #include #include "absl/strings/string_view.h" +#include "internal/platform/exception.h" #include "internal/platform/implementation/input_file.h" #include "internal/platform/implementation/output_file.h" -#include "internal/platform/exception.h" namespace location { namespace nearby { @@ -29,8 +29,9 @@ namespace shared { class IOFile final : public api::InputFile, public api::OutputFile { public: - static std::unique_ptr CreateInputFile(const absl::string_view path, - size_t size); + static std::unique_ptr CreateInputFile( + const absl::string_view file_path, size_t size); + static std::unique_ptr CreateOutputFile(const absl::string_view path); ExceptionOr Read(std::int64_t size) override; @@ -44,10 +45,11 @@ class IOFile final : public api::InputFile, public api::OutputFile { Exception Flush() override; private: - explicit IOFile(const absl::string_view path, size_t size); - explicit IOFile(const absl::string_view path); + explicit IOFile(const absl::string_view file_path, size_t size); + explicit IOFile(const absl::string_view file_path); + std::fstream file_; - absl::string_view path_; + std::string path_; std::int64_t total_size_; }; diff --git a/internal/platform/implementation/windows/BUILD b/internal/platform/implementation/windows/BUILD index d6b011ef..b1e4291a 100644 --- a/internal/platform/implementation/windows/BUILD +++ b/internal/platform/implementation/windows/BUILD @@ -120,7 +120,6 @@ cc_library( "//internal/platform:types", "//internal/platform/implementation:comm", "//internal/platform/implementation:platform", - "//internal/platform/implementation:types", "//internal/platform/implementation/shared:count_down_latch", "//internal/platform/implementation/shared:file", "//internal/platform/implementation/windows/generated:types", @@ -153,6 +152,7 @@ cc_test( "count_down_latch_test.cc", "crypto_test.cc", "executor_test.cc", + "platform_test.cc", "scheduled_executor_test.cc", "submittable_executor_test.cc", ], diff --git a/internal/platform/implementation/windows/platform.cc b/internal/platform/implementation/windows/platform.cc index c98dfbd2..ae3a38fa 100644 --- a/internal/platform/implementation/windows/platform.cc +++ b/internal/platform/implementation/windows/platform.cc @@ -14,7 +14,12 @@ #include "internal/platform/implementation/platform.h" +#include #include +#include + +#include +#include #include "internal/platform/implementation/shared/count_down_latch.h" #include "internal/platform/implementation/shared/file.h" @@ -41,9 +46,8 @@ namespace location { namespace nearby { namespace api { -namespace { - -std::string GetPayloadPath(PayloadId payload_id) { +std::string ImplementationPlatform::GetDownloadPath(std::string& parent_folder, + std::string& file_name) { PWSTR basePath; // Retrieves the full path of a known folder identified by the folder's @@ -62,15 +66,59 @@ std::string GetPayloadPath(PayloadId payload_id) { // SHGetKnownFolderPath succeeds or not. size_t bufferSize; wcstombs_s(&bufferSize, NULL, 0, basePath, 0); - char* fullpathUTF8 = new char[bufferSize + 1]; - memset(fullpathUTF8, 0, bufferSize); - wcstombs_s(&bufferSize, fullpathUTF8, bufferSize, basePath, bufferSize - 1); - std::string fullPath = std::string(fullpathUTF8); - auto retval = absl::StrCat(fullPath += "\\", payload_id); - delete[] fullpathUTF8; - return retval; + std::string fullpathUTF8(bufferSize, '\0'); + wcstombs_s(&bufferSize, fullpathUTF8.data(), bufferSize, basePath, _TRUNCATE); + std::string fullPath = fullpathUTF8; + + // If parent_folder starts with a \\ or /, then strip it + while (!parent_folder.empty() && + (*parent_folder.begin() == '\\' || *parent_folder.begin() == '/')) { + parent_folder.erase(0, 1); + } + + // If parent_folder ends with a \\ or /, then strip it + while (!parent_folder.empty() && + (*parent_folder.rbegin() == '\\' || *parent_folder.rbegin() == '/')) { + parent_folder.erase(parent_folder.size() - 1, 1); + } + + // If file_name starts with a \\, then strip it + while (!file_name.empty() && + (*file_name.begin() == '\\' || *file_name.begin() == '/')) { + file_name.erase(0, 1); + } + + // If file_name ends with a \\, then strip it + while (!file_name.empty() && + (*file_name.rbegin() == '\\' || *file_name.rbegin() == '/')) { + file_name.erase(file_name.size() - 1, 1); + } + + CoTaskMemFree(basePath); + + std::stringstream path(""); + + if (parent_folder.empty() && file_name.empty()) { + return fullPath; + } + if (parent_folder.empty()) { + path << fullPath.c_str() << "\\" << file_name.c_str(); + std::string retVal = path.str(); + return retVal; + } + if (file_name.empty()) { + path << fullPath.c_str() << "\\" << parent_folder.c_str(); + std::string retVal = path.str(); + return retVal; + } + + path << fullPath.c_str() << "\\" << parent_folder.c_str() << "\\" + << file_name.c_str(); + std::string retVal = path.str(); + return retVal; } -} // namespace + +OSName ImplementationPlatform::GetCurrentOS() { return OSName::kWindows; } std::unique_ptr ImplementationPlatform::CreateAtomicBoolean( bool initial_value) { @@ -96,15 +144,32 @@ ImplementationPlatform::CreateConditionVariable(Mutex* mutex) { return absl::make_unique(mutex); } +ABSL_DEPRECATED("This interface will be deleted in the near future.") std::unique_ptr ImplementationPlatform::CreateInputFile( PayloadId payload_id, std::int64_t total_size) { - return shared::IOFile::CreateInputFile(GetPayloadPath(payload_id), - total_size); + std::string parent_folder(""); + std::string file_name(std::to_string(payload_id)); + return shared::IOFile::CreateInputFile( + GetDownloadPath(parent_folder, file_name), total_size); +} + +std::unique_ptr ImplementationPlatform::CreateInputFile( + absl::string_view file_path, size_t size) { + return shared::IOFile::CreateInputFile(file_path, size); +} + +ABSL_DEPRECATED("This interface will be deleted in the near future.") +std::unique_ptr ImplementationPlatform::CreateOutputFile( + PayloadId payload_id) { + std::string parent_folder(""); + std::string file_name(std::to_string(payload_id)); + return shared::IOFile::CreateOutputFile( + GetDownloadPath(parent_folder, file_name)); } std::unique_ptr ImplementationPlatform::CreateOutputFile( - PayloadId payload_id) { - return shared::IOFile::CreateOutputFile(GetPayloadPath(payload_id)); + absl::string_view file_path) { + return shared::IOFile::CreateOutputFile(file_path); } // TODO(b/184975123): replace with real implementation. diff --git a/internal/platform/implementation/windows/platform_test.cc b/internal/platform/implementation/windows/platform_test.cc new file mode 100644 index 00000000..af04706a --- /dev/null +++ b/internal/platform/implementation/windows/platform_test.cc @@ -0,0 +1,415 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "internal/platform/implementation/platform.h" + +#include +#include +#include + +#include + +#include "gtest/gtest.h" + +// Can't run on google 3, I presume the SHGetKnownFolderPath +// fails. +#if 0 +class ImplementationPlatformTests : public testing::Test +{ + protected: + // You can define per-test set-up logic as usual. + void SetUp() override + { + PWSTR basePath; + + SHGetKnownFolderPath( + FOLDERID_Downloads, // rfid: A reference to the KNOWNFOLDERID that + // identifies the folder. + 0, // dwFlags: Flags that specify special retrieval + // options. + NULL, // hToken: An access token that represents a + // particular user. + &basePath); // ppszPath: When this method returns, contains + // the address of a pointer to a + // null-terminated Unicode string that + // specifies the path of the known + // folder. The calling process is + // responsible for freeing this resource + // once it is no longer needed by + // calling CoTaskMemFree, whether + // SHGetKnownFolderPath succeeds or not. + + size_t bufferSize; + wcstombs_s(&bufferSize, NULL, 0, basePath, 0); + std::string fullpathUTF8(bufferSize, '\0'); + wcstombs_s(&bufferSize, fullpathUTF8.data(), bufferSize, basePath, + _TRUNCATE); + default_download_path_ = fullpathUTF8; + } + + std::string default_download_path_; +}; + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithEmptyStringArgumentsShouldReturnBaseDownloadPath) +{ + // Arrange + std::string parent_folder(""); + std::string file_name(""); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, default_download_path_); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithSlashParentFolderArgumentsShouldReturn\ +BaseDownloadPath) +{ + // Arrange + std::string parent_folder("/"); + std::string file_name(""); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, default_download_path_); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithBackslashParentFolderArgumentsShouldReturn\ +BaseDownloadPath) +{ + // Arrange + std::string parent_folder("\\"); + std::string file_name(""); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, default_download_path_); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithSlashFileNameArgumentsShouldReturnBaseDownloadPath) +{ + // Arrange + std::string parent_folder(""); + std::string file_name("/"); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, default_download_path_); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithBackslashFileNameArgumentsShouldReturn\ +BaseDownloadPath) +{ + // Arrange + std::string parent_folder(""); + std::string file_name("\\"); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + auto result_size = result.size(); + auto default_size = default_download_path_.size(); + + // Assert + EXPECT_EQ(result, default_download_path_); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithParentFolderShouldReturnParentFolder\ +AppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder("test_parent_folder"); + std::string file_name(""); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_parent_folder"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithParentFolderStartingWithSlashArgumentsShouldReturn\ +ParentFolderAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder("/test_parent_folder"); + std::string file_name(""); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_parent_folder"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithParentFolderStartingWithBackslashArguments\ +ShouldReturnParentFolderAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder("\\test_parent_folder"); + std::string file_name(""); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_parent_folder"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithParentFolderEndingWithSlashArgumentsShouldReturn\ +ParentFolderAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder("test_parent_folder/"); + std::string file_name(""); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_parent_folder"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithParentFolderEndingWithBackslashArguments\ +ShouldReturnParentFolderAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder("test_parent_folder\\"); + std::string file_name(""); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_parent_folder"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithFileNameBeginningWithSlashArgumentsShouldReturn\ +FileNameAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder(""); + std::string file_name("/test_file_name.name"); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_file_name.name"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithFileNameBeginningWithBackslashArgumentsShouldReturn\ +FileNameAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder(""); + std::string file_name("\\test_file_name.name"); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_file_name.name"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithFileNameEndingWithSlashArgumentsShouldReturnFileName\ +AppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder(""); + std::string file_name("test_file_name.name/"); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_file_name.name"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithFileNameEndingWithBackslashArgumentsShouldReturn\ +FileNameAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder(""); + std::string file_name("test_file_name.name\\"); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_file_name.name"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithParentFolderAndFileNameArgumentsShouldReturn\ +ParentFolderAndFileNameAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder("test_parent_folder"); + std::string file_name("test_file_name.name"); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_parent_folder" + << "\\" + << "test_file_name.name"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F(ImplementationPlatformTests, + GetDownloadPathWithParentFolderEndingWithBackslashAndFileNameArguments\ +ShouldReturnParentFolderAndFileNameAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder("test_parent_folder\\"); + std::string file_name("test_file_name.name"); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_parent_folder" + << "\\" + << "test_file_name.name"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} + +TEST_F( + ImplementationPlatformTests, + GetDownloadPathWithFileNameStartingWithBackslashAndParentFolderArguments\ +ShouldReturnParentFolderAndFileNameAppendedToBaseDownloadPath) +{ + // Arrange + std::string parent_folder("test_parent_folder"); + std::string file_name("\\test_file_name.name"); + + std::stringstream path(""); + path << default_download_path_.c_str() << "\\" + << "test_parent_folder" + << "\\" + << "test_file_name.name"; + + std::string expected = path.str(); + + // Act + auto result = location::nearby::api::ImplementationPlatform::GetDownloadPath( + parent_folder, file_name); + + // Assert + EXPECT_EQ(result, expected); +} +#endif diff --git a/internal/platform/implementation/windows/test_utils.cc b/internal/platform/implementation/windows/test_utils.cc index 71cb79ab..766383cc 100644 --- a/internal/platform/implementation/windows/test_utils.cc +++ b/internal/platform/implementation/windows/test_utils.cc @@ -16,6 +16,8 @@ #include +#include + #include "absl/strings/str_cat.h" namespace test_utils { @@ -49,13 +51,21 @@ std::string GetPayloadPath(location::nearby::PayloadId payload_id) { // SHGetKnownFolderPath succeeds or not. size_t bufferSize; + // Get the required buffer size. wcstombs_s(&bufferSize, NULL, 0, basePath, 0); - char* fullpathUTF8 = new char[bufferSize + 1]; - memset(fullpathUTF8, 0, bufferSize); - wcstombs_s(&bufferSize, fullpathUTF8, bufferSize, basePath, bufferSize - 1); + std::string fullpathUTF8(bufferSize, NULL); + wcstombs_s(&bufferSize, fullpathUTF8.data(), bufferSize, basePath, _TRUNCATE); std::string fullPath = std::string(fullpathUTF8); - auto retval = absl::StrCat(fullPath += "\\", payload_id); - delete[] fullpathUTF8; + // Clean up the string by removing null's + fullPath.erase(std::find(fullPath.begin(), fullPath.end(), '\0'), + fullPath.end()); + + CoTaskMemFree(basePath); + + std::stringstream path(""); + + path << fullPath << "\\" << std::to_string(payload_id); + auto retval = path.str(); return retval; } } // namespace test_utils diff --git a/internal/platform/os_name.h b/internal/platform/os_name.h new file mode 100644 index 00000000..028bfa55 --- /dev/null +++ b/internal/platform/os_name.h @@ -0,0 +1,28 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef PLATFORM_OS_NAME_H_ +#define PLATFORM_OS_NAME_H_ + +namespace location { +namespace nearby { +namespace api { + +enum class OSName { kLinux, kWindows, kiOS, kChromeOS }; + +} // namespace api +} // namespace nearby +} // namespace location + +#endif // PLATFORM_OS_NAME_H_