Skip to content

Commit

Permalink
Remove unknown files from PAYLOAD_TRANSFER offline frame
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 621691408
  • Loading branch information
realp2us authored and copybara-github committed Apr 17, 2024
1 parent e5279bf commit 967a84a
Show file tree
Hide file tree
Showing 10 changed files with 218 additions and 42 deletions.
5 changes: 1 addition & 4 deletions sharing/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,17 @@ cc_library(
deps = [
":connection_types",
"//internal/crypto_cros",
"//internal/interop:authentication_status",
"//internal/network:url",
"//sharing/common:compatible_u8_string",
"//sharing/common:enum",
"//sharing/internal/base",
"//sharing/internal/public:logging",
"//sharing/proto:enums_cc_proto",
"//sharing/proto:share_cc_proto",
"//sharing/proto:wire_format_cc_proto",
"@com_google_absl//absl/container:flat_hash_set",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:str_format",
"@com_google_absl//absl/time",
"@com_google_absl//absl/types:span",
],
)
Expand Down Expand Up @@ -326,7 +324,6 @@ cc_test(
"//internal/network:types",
"//internal/network:url",
"//internal/platform/implementation:account_manager",
"//internal/platform/implementation:types",
"//internal/platform/implementation/g3", # fixdeps: keep
"//internal/test",
"//proto:sharing_enums_cc_proto",
Expand Down
22 changes: 22 additions & 0 deletions sharing/fake_nearby_connections_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,28 @@ void FakeNearbyConnectionsManager::SetCustomSavePath(
custom_save_path_ = custom_save_path;
}

absl::flat_hash_set<std::filesystem::path>
FakeNearbyConnectionsManager::GetUnknownFilePathsToDelete() {
return file_paths_to_delete_;
}

void FakeNearbyConnectionsManager::ClearUnknownFilePathsToDelete() {
file_paths_to_delete_.clear();
}

void FakeNearbyConnectionsManager::AddUnknownFilePathsToDeleteForTesting(
std::filesystem::path file_path) {
file_paths_to_delete_.insert(file_path);
}

absl::flat_hash_set<std::filesystem::path>
FakeNearbyConnectionsManager::GetAndClearUnknownFilePathsToDelete() {
absl::flat_hash_set<std::filesystem::path> file_paths_to_delete =
file_paths_to_delete_;
file_paths_to_delete_.clear();
return file_paths_to_delete;
}

std::string FakeNearbyConnectionsManager::Dump() const { return ""; }

} // namespace sharing
Expand Down
9 changes: 8 additions & 1 deletion sharing/fake_nearby_connections_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ class FakeNearbyConnectionsManager : public NearbyConnectionsManager {
absl::string_view endpoint_id) override;
void UpgradeBandwidth(absl::string_view endpoint_id) override;
void SetCustomSavePath(absl::string_view custom_save_path) override;
absl::flat_hash_set<std::filesystem::path> GetUnknownFilePathsToDelete()
override;
absl::flat_hash_set<std::filesystem::path>
GetAndClearUnknownFilePathsToDelete() override;
void ClearUnknownFilePathsToDelete() override;

// Testing methods
void SetRawAuthenticationToken(absl::string_view endpoint_id,
Expand Down Expand Up @@ -135,6 +140,8 @@ class FakeNearbyConnectionsManager : public NearbyConnectionsManager {
return !incoming_payloads_.empty();
}

void AddUnknownFilePathsToDeleteForTesting(std::filesystem::path file_path);

private:
void HandleStartAdvertisingCallback(ConnectionsStatus status);
void HandleStopAdvertisingCallback(ConnectionsStatus status);
Expand Down Expand Up @@ -172,7 +179,7 @@ class FakeNearbyConnectionsManager : public NearbyConnectionsManager {
std::map<int64_t, std::unique_ptr<Payload>> incoming_payloads_
ABSL_GUARDED_BY(incoming_payloads_mutex_);
std::map<int64_t, std::filesystem::path> registered_payload_paths_;

absl::flat_hash_set<std::filesystem::path> file_paths_to_delete_;
std::string Dump() const override;
};

Expand Down
12 changes: 12 additions & 0 deletions sharing/nearby_connections_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <string>
#include <vector>

#include "absl/container/flat_hash_set.h"
#include "absl/strings/string_view.h"
#include "absl/types/span.h"
#include "sharing/common/nearby_share_enums.h"
Expand Down Expand Up @@ -162,6 +163,17 @@ class NearbyConnectionsManager {
// Sets a custom save path.
virtual void SetCustomSavePath(absl::string_view custom_save_path) = 0;

// Gets the file paths to delete.
virtual absl::flat_hash_set<std::filesystem::path>
GetUnknownFilePathsToDelete() = 0;

// Deletes the file paths to delete.
virtual void ClearUnknownFilePathsToDelete() = 0;

// Gets the file paths to delete and clear the hash set.
virtual absl::flat_hash_set<std::filesystem::path>
GetAndClearUnknownFilePathsToDelete() = 0;

// Dump internal state for debugging purposes.
virtual std::string Dump() const = 0;
};
Expand Down
42 changes: 38 additions & 4 deletions sharing/nearby_connections_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ std::string MediumSelectionToString(const MediumSelection& mediums) {
} // namespace

NearbyConnectionsManagerImpl::NearbyConnectionsManagerImpl(
Context* context,
ConnectivityManager& connectivity_manager,
Context* context, ConnectivityManager& connectivity_manager,
nearby::DeviceInfo& device_info,
std::unique_ptr<NearbyConnectionsService> nearby_connections_service)
: context_(context),
Expand Down Expand Up @@ -489,8 +488,7 @@ void NearbyConnectionsManagerImpl::Send(

if (transfer_managers_.contains(endpoint_id) && payload->content.is_file()) {
NL_LOG(INFO) << __func__ << ": Send payload " << payload->id << " to "
<< endpoint_id
<< " to transfer manager. payload is file: "
<< endpoint_id << " to transfer manager. payload is file: "
<< payload->content.is_file() << ", is bytes "
<< payload->content.is_bytes();
transfer_managers_.at(endpoint_id)
Expand Down Expand Up @@ -831,6 +829,17 @@ void NearbyConnectionsManagerImpl::OnPayloadTransferUpdate(

if (payload_it->second.content.type != PayloadContent::Type::kBytes) {
NL_LOG(WARNING) << "Received unknown payload of file type. Cancelling.";
if (!NearbyFlags::GetInstance().GetBoolFlag(
sharing::config_package_nearby::nearby_sharing_feature::
kDeleteUnexpectedReceivedFile)) {
// if we get kFile and have file_path, delete the file path.
if (payload_it->second.content.type == PayloadContent::Type::kFile) {
auto file_path = payload_it->second.content.file_payload.file.path;
NL_LOG(WARNING) << __func__
<< ": Payload is Type::kFile type. Removing.";
file_paths_to_delete_.insert(file_path);
}
}
nearby_connections_service_->CancelPayload(kServiceId, payload_it->first,
[](Status status) {});
return;
Expand Down Expand Up @@ -898,6 +907,31 @@ void NearbyConnectionsManagerImpl::SetCustomSavePath(
});
}

absl::flat_hash_set<std::filesystem::path>
NearbyConnectionsManagerImpl::GetUnknownFilePathsToDelete() {
MutexLock lock(&mutex_);
return file_paths_to_delete_;
}

void NearbyConnectionsManagerImpl::ClearUnknownFilePathsToDelete() {
MutexLock lock(&mutex_);
file_paths_to_delete_.clear();
}

absl::flat_hash_set<std::filesystem::path>
NearbyConnectionsManagerImpl::GetAndClearUnknownFilePathsToDelete() {
MutexLock lock(&mutex_);
auto file_paths_to_delete = file_paths_to_delete_;
file_paths_to_delete_.clear();
return file_paths_to_delete;
}

void NearbyConnectionsManagerImpl::AddUnknownFilePathsToDeleteForTesting(
std::filesystem::path file_path) {
MutexLock lock(&mutex_);
file_paths_to_delete_.insert(file_path);
}

std::string NearbyConnectionsManagerImpl::Dump() const {
return nearby_connections_service_->Dump();
}
Expand Down
15 changes: 13 additions & 2 deletions sharing/nearby_connections_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ namespace sharing {
class NearbyConnectionsManagerImpl : public NearbyConnectionsManager {
public:
explicit NearbyConnectionsManagerImpl(
Context* context,
nearby::ConnectivityManager& connectivity_manager,
Context* context, nearby::ConnectivityManager& connectivity_manager,
nearby::DeviceInfo& device_info,
std::unique_ptr<NearbyConnectionsService> nearby_connections_service);
~NearbyConnectionsManagerImpl() override;
Expand Down Expand Up @@ -85,12 +84,20 @@ class NearbyConnectionsManagerImpl : public NearbyConnectionsManager {
absl::string_view endpoint_id) override;
void UpgradeBandwidth(absl::string_view endpoint_id) override;
void SetCustomSavePath(absl::string_view custom_save_path) override;
absl::flat_hash_set<std::filesystem::path> GetUnknownFilePathsToDelete()
override;
absl::flat_hash_set<std::filesystem::path>
GetAndClearUnknownFilePathsToDelete() override;
void ClearUnknownFilePathsToDelete() override;

std::string Dump() const override;

NearbyConnectionsService* GetNearbyConnectionsService() const {
return nearby_connections_service_.get();
}

void AddUnknownFilePathsToDeleteForTesting(std::filesystem::path file_path);

private:
// EndpointDiscoveryListener:
void OnEndpointFound(absl::string_view endpoint_id,
Expand Down Expand Up @@ -176,6 +183,10 @@ class NearbyConnectionsManagerImpl : public NearbyConnectionsManager {
// Avoid calling to disconnect on an endpoint multiple times.
absl::flat_hash_set<std::string> disconnecting_endpoints_
ABSL_GUARDED_BY(mutex_);

// A set of file paths to delete.
absl::flat_hash_set<std::filesystem::path> file_paths_to_delete_
ABSL_GUARDED_BY(mutex_);
};

} // namespace sharing
Expand Down
39 changes: 39 additions & 0 deletions sharing/nearby_connections_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ namespace {
using ::nearby::sharing::proto::DataUsage;
using ::testing::ElementsAre;
using ::testing::FieldsAre;
using ::testing::UnorderedElementsAre;

constexpr char kServiceId[] = "NearbySharing";
constexpr Strategy kStrategy = Strategy::kP2pPointToPoint;
Expand Down Expand Up @@ -95,6 +96,8 @@ void InitializeTemporaryFile(std::filesystem::path& file) {

} // namespace

namespace NearbyConnectionsManagerUnitTests {

class MockDiscoveryListener
: public NearbyConnectionsManager::DiscoveryListener {
public:
Expand Down Expand Up @@ -1731,5 +1734,41 @@ TEST_F(NearbyConnectionsManagerImplTest,
notification.WaitForNotificationWithTimeout(kSynchronizationTimeOut));
}

TEST_F(NearbyConnectionsManagerImplTest, UnknownFilePathsToDelete) {
nearby_connections_manager_->AddUnknownFilePathsToDeleteForTesting(
"test1.txt");
nearby_connections_manager_->AddUnknownFilePathsToDeleteForTesting(
"test2.txt");
auto unknown_file_paths =
nearby_connections_manager_->GetUnknownFilePathsToDelete();
nearby_connections_manager_->AddUnknownFilePathsToDeleteForTesting(
"test3.txt");

// Test if we get copy of container.
EXPECT_NE(unknown_file_paths.size(), 3);
EXPECT_EQ(unknown_file_paths.size(), 2);
EXPECT_EQ(nearby_connections_manager_->GetUnknownFilePathsToDelete().size(),
3);
unknown_file_paths =
nearby_connections_manager_->GetUnknownFilePathsToDelete();
EXPECT_THAT(unknown_file_paths,
UnorderedElementsAre("test1.txt", "test2.txt", "test3.txt"));
nearby_connections_manager_->ClearUnknownFilePathsToDelete();
EXPECT_EQ(nearby_connections_manager_->GetUnknownFilePathsToDelete().size(),
0);

// Test GetAndClearUnknownFilePathsToDelete
nearby_connections_manager_->AddUnknownFilePathsToDeleteForTesting(
"test1.txt");
nearby_connections_manager_->AddUnknownFilePathsToDeleteForTesting(
"test2.txt");
unknown_file_paths =
nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
EXPECT_EQ(unknown_file_paths.size(), 2);
EXPECT_EQ(nearby_connections_manager_->GetUnknownFilePathsToDelete().size(),
0);
}

} // namespace NearbyConnectionsManagerUnitTests
} // namespace sharing
} // namespace nearby
23 changes: 15 additions & 8 deletions sharing/nearby_sharing_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,7 @@ void NearbySharingServiceImpl::Shutdown(
foreground_receive_callbacks_.Clear();
background_receive_callbacks_.Clear();

device_info_.UnregisterScreenLockedListener(
kScreenStateListenerName);
device_info_.UnregisterScreenLockedListener(kScreenStateListenerName);

settings_->RemoveSettingsObserver(this);

Expand Down Expand Up @@ -1160,8 +1159,7 @@ std::string NearbySharingServiceImpl::Dump() const {
sstream << " IsSendingFile: " << IsSendingFile() << std::endl;
sstream << " IsReceivingFile: " << IsReceivingFile() << std::endl;

sstream << " IsScreenLocked: " << device_info_.IsScreenLocked()
<< std::endl;
sstream << " IsScreenLocked: " << device_info_.IsScreenLocked() << std::endl;
sstream << " IsBluetoothPresent: " << IsBluetoothPresent() << std::endl;
sstream << " IsBluetoothPowered: " << IsBluetoothPowered() << std::endl;
sstream << " IsExtendedAdvertisingSupported: "
Expand Down Expand Up @@ -3972,7 +3970,6 @@ std::optional<ShareTarget> NearbySharingServiceImpl::CreateShareTarget(
absl::string_view endpoint_id, const Advertisement& advertisement,
std::optional<NearbyShareDecryptedPublicCertificate> certificate,
bool is_incoming) {

if (!advertisement.device_name() && !certificate.has_value()) {
NL_VLOG(1) << __func__
<< ": Failed to retrieve public certificate for contact "
Expand Down Expand Up @@ -4115,9 +4112,8 @@ bool NearbySharingServiceImpl::OnIncomingPayloadsComplete(

connection->SetDisconnectionListener([&, share_target_id]() {
RunOnNearbySharingServiceThread(
"disconnection_listener", [&, share_target_id]() {
UnregisterShareTarget(share_target_id);
});
"disconnection_listener",
[&, share_target_id]() { UnregisterShareTarget(share_target_id); });
});

if (!update_file_paths_in_progress_) {
Expand Down Expand Up @@ -4241,6 +4237,17 @@ void NearbySharingServiceImpl::RemoveIncomingPayloads(
NL_LOG(INFO) << __func__ << ": Cleaning up payloads due to transfer failure";
nearby_connections_manager_->ClearIncomingPayloads();
std::vector<std::filesystem::path> files_for_deletion;
if (NearbyFlags::GetInstance().GetBoolFlag(
config_package_nearby::nearby_sharing_feature::
kDeleteUnexpectedReceivedFile)) {
auto file_paths_to_delete =
nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
for (auto it = file_paths_to_delete.begin();
it != file_paths_to_delete.end(); ++it) {
NL_VLOG(1) << __func__ << ": Has unknown file path to delete.";
files_for_deletion.push_back(*it);
}
}
for (const auto& file : share_target.file_attachments) {
if (!file.file_path().has_value()) continue;
auto file_path = *file.file_path();
Expand Down
3 changes: 3 additions & 0 deletions sharing/nearby_sharing_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class NearbyShareContactManager;

namespace NearbySharingServiceUnitTests {
class NearbySharingServiceImplTest_CreateShareTarget_Test;
class NearbySharingServiceImplTest_RemoveIncomingPayloads_Test;
};

// All methods should be called from the same sequence that created the service.
Expand All @@ -103,6 +104,8 @@ class NearbySharingServiceImpl
public NearbyConnectionsManager::DiscoveryListener {
FRIEND_TEST(NearbySharingServiceUnitTests::NearbySharingServiceImplTest,
CreateShareTarget);
FRIEND_TEST(NearbySharingServiceUnitTests::NearbySharingServiceImplTest,
RemoveIncomingPayloads);

public:
NearbySharingServiceImpl(
Expand Down
Loading

0 comments on commit 967a84a

Please sign in to comment.