Move unknown file filter to OnPayloadReceived()

PiperOrigin-RevId: 682505406
This commit is contained in:
Eiden Kim
2024-10-04 17:33:26 -07:00
committed by Copybara-Service
parent 7f3c7f55cc
commit 5d8b9156e0
4 changed files with 224 additions and 69 deletions
@@ -32,6 +32,9 @@ namespace nearby_sharing_feature {
// Time to delay the endpoint loss in milliseconds.
constexpr auto kDelayEndpointLossMs =
flags::Flag<int64_t>(kConfigPackage, "45632386", 500);
// When true, delete the file payload which received unexpectedly.
constexpr auto kDeleteUnexpectedReceivedFileFix =
flags::Flag<bool>(kConfigPackage, "45657036", false);
// Enable/disable the use of BLE as a connection medium.
constexpr auto kEnableBleForTransfer =
flags::Flag<bool>(kConfigPackage, "45427466", false);
@@ -88,6 +91,7 @@ constexpr auto kUpdateTrack =
inline absl::btree_map<int, const flags::Flag<bool>&> GetBoolFlags() {
return {
{45657036, kDeleteUnexpectedReceivedFileFix},
{45427466, kEnableBleForTransfer},
{45409184, kEnableCertificatesDump},
{45418905, kEnableMediumWebRtc},
+91 -56
View File
@@ -206,7 +206,7 @@ void NearbyConnectionsManagerImpl::StartAdvertising(
NearbyConnectionsService::ConnectionListener connection_listener;
connection_listener.initiated_cb =
[this](absl::string_view endpoint_id,
const ConnectionInfo& connection_info) {
const ConnectionInfo& connection_info) {
OnConnectionInitiated(endpoint_id, connection_info);
};
connection_listener.accepted_cb = [this](absl::string_view endpoint_id) {
@@ -232,7 +232,7 @@ void NearbyConnectionsManagerImpl::StartAdvertising(
Uuid fast_advertisement_service_uuid;
NL_LOG(INFO) << __func__ << ": Nearby Sharing flag kEnableBleV2 is enabled.";
LOG(INFO) << __func__ << ": Nearby Sharing flag kEnableBleV2 is enabled.";
// Uses fast advertisement when advertisement data size is less than
// kMinimumAdvertisementSize. Nearby Connections will decide whether to use
// GATT server with this information.
@@ -423,7 +423,7 @@ void NearbyConnectionsManagerImpl::Connect(
void NearbyConnectionsManagerImpl::OnConnectionTimedOut(
absl::string_view endpoint_id) {
MutexLock lock(&mutex_);
NL_LOG(ERROR) << "Failed to connect to the remote shareTarget: Timed out.";
LOG(ERROR) << "Failed to connect to the remote shareTarget: Timed out.";
if (pending_outgoing_connections_.contains(endpoint_id)) {
auto it = connection_info_map_.find(endpoint_id);
if (it != connection_info_map_.end()) {
@@ -439,8 +439,8 @@ void NearbyConnectionsManagerImpl::OnConnectionRequested(
auto it = pending_outgoing_connections_.find(endpoint_id);
if (it == pending_outgoing_connections_.end()) return;
if (status != ConnectionsStatus::kSuccess) {
NL_LOG(ERROR) << "Failed to connect to the remote shareTarget: "
<< ConnectionsStatusToString(status);
LOG(ERROR) << "Failed to connect to the remote shareTarget: "
<< ConnectionsStatusToString(status);
auto info_it = connection_info_map_.find(endpoint_id);
if (info_it != connection_info_map_.end()) {
info_it->second.connection_layer_status = status;
@@ -454,13 +454,13 @@ void NearbyConnectionsManagerImpl::Disconnect(absl::string_view endpoint_id) {
MutexLock lock(&mutex_);
if (!pending_outgoing_connections_.contains(endpoint_id) &&
!connection_info_map_.contains(endpoint_id)) {
NL_LOG(WARNING) << "No connection for endpoint " << endpoint_id;
LOG(WARNING) << "No connection for endpoint " << endpoint_id;
return;
}
if (disconnecting_endpoints_.contains(endpoint_id)) {
NL_LOG(INFO) << "Another Disconnecting is running for endpoint_id "
<< endpoint_id;
LOG(INFO) << "Another Disconnecting is running for endpoint_id "
<< endpoint_id;
return;
}
@@ -480,7 +480,7 @@ void NearbyConnectionsManagerImpl::Disconnect(absl::string_view endpoint_id) {
disconnecting_endpoints_.erase(endpoint_id);
}
});
NL_LOG(INFO) << "Disconnected from " << endpoint_id;
LOG(INFO) << "Disconnected from " << endpoint_id;
});
}
@@ -493,15 +493,15 @@ 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: "
<< payload->content.is_file() << ", is bytes "
<< payload->content.is_bytes();
LOG(INFO) << __func__ << ": Send payload " << payload->id << " to "
<< endpoint_id << " to transfer manager. payload is file: "
<< payload->content.is_file() << ", is bytes "
<< payload->content.is_bytes();
transfer_managers_.at(endpoint_id)
->Send([&, endpoint_id = std::string(endpoint_id),
payload_copy = *payload]() {
NL_LOG(INFO) << __func__ << ": Send payload " << payload_copy.id
<< " to " << endpoint_id;
LOG(INFO) << __func__ << ": Send payload " << payload_copy.id
<< " to " << endpoint_id;
auto sent_payload = std::make_unique<Payload>(payload_copy);
SendWithoutDelay(endpoint_id, std::move(sent_payload));
});
@@ -514,15 +514,14 @@ void NearbyConnectionsManagerImpl::Send(
void NearbyConnectionsManagerImpl::SendWithoutDelay(
absl::string_view endpoint_id, std::unique_ptr<Payload> payload) {
NL_LOG(INFO) << __func__ << ": Send payload " << payload->id << " to "
<< endpoint_id;
LOG(INFO) << __func__ << ": Send payload " << payload->id << " to "
<< endpoint_id;
nearby_connections_service_->SendPayload(
kServiceId, {std::string(endpoint_id)}, std::move(payload),
[endpoint_id = std::string(endpoint_id)](ConnectionsStatus status) {
NL_LOG(INFO) << __func__ << ": Sending payload to endpoint "
<< endpoint_id
<< " attempted over Nearby Connections with result: "
<< ConnectionsStatusToString(status);
LOG(INFO) << __func__ << ": Sending payload to endpoint " << endpoint_id
<< " attempted over Nearby Connections with result: "
<< ConnectionsStatusToString(status);
});
}
@@ -566,7 +565,7 @@ void NearbyConnectionsManagerImpl::Cancel(int64_t payload_id) {
<< ConnectionsStatusToString(status);
});
NL_LOG(INFO) << "Cancelling payload: " << payload_id;
LOG(INFO) << "Cancelling payload: " << payload_id;
}
void NearbyConnectionsManagerImpl::ClearIncomingPayloads() {
@@ -616,44 +615,44 @@ void NearbyConnectionsManagerImpl::OnEndpointFound(
absl::string_view endpoint_id, const DiscoveredEndpointInfo& info) {
MutexLock lock(&mutex_);
if (!discovery_listener_) {
NL_LOG(INFO) << "Ignoring discovered endpoint "
<< nearby::utils::HexEncode(info.endpoint_info)
<< " because we're no longer "
"in discovery mode";
LOG(INFO) << "Ignoring discovered endpoint "
<< nearby::utils::HexEncode(info.endpoint_info)
<< " because we're no longer "
"in discovery mode";
return;
}
auto result = discovered_endpoints_.insert(std::string(endpoint_id));
if (!result.second) {
NL_LOG(INFO) << "Ignoring discovered endpoint "
<< nearby::utils::HexEncode(info.endpoint_info)
<< " because we've already "
"reported this endpoint";
LOG(INFO) << "Ignoring discovered endpoint "
<< nearby::utils::HexEncode(info.endpoint_info)
<< " because we've already "
"reported this endpoint";
return;
}
discovery_listener_->OnEndpointDiscovered(endpoint_id, info.endpoint_info);
NL_LOG(INFO) << "Discovered " << nearby::utils::HexEncode(info.endpoint_info)
<< " over Nearby Connections";
LOG(INFO) << "Discovered " << nearby::utils::HexEncode(info.endpoint_info)
<< " over Nearby Connections";
}
void NearbyConnectionsManagerImpl::OnEndpointLost(
absl::string_view endpoint_id) {
MutexLock lock(&mutex_);
if (!discovered_endpoints_.erase(endpoint_id)) {
NL_LOG(INFO) << "Ignoring lost endpoint " << endpoint_id
<< " because we haven't reported this endpoint";
LOG(INFO) << "Ignoring lost endpoint " << endpoint_id
<< " because we haven't reported this endpoint";
return;
}
if (!discovery_listener_) {
NL_LOG(INFO) << "Ignoring lost endpoint " << endpoint_id
<< " because we're no longer in discovery mode";
LOG(INFO) << "Ignoring lost endpoint " << endpoint_id
<< " because we're no longer in discovery mode";
return;
}
discovery_listener_->OnEndpointLost(endpoint_id);
NL_LOG(INFO) << "Endpoint " << endpoint_id << " lost over Nearby Connections";
LOG(INFO) << "Endpoint " << endpoint_id << " lost over Nearby Connections";
}
void NearbyConnectionsManagerImpl::OnConnectionInitiated(
@@ -666,7 +665,7 @@ void NearbyConnectionsManagerImpl::OnConnectionInitiated(
NearbyConnectionsService::PayloadListener payload_listener;
payload_listener.payload_cb = [this](absl::string_view endpoint_id,
Payload payload) {
Payload payload) {
OnPayloadReceived(endpoint_id, payload);
};
@@ -784,10 +783,37 @@ void NearbyConnectionsManagerImpl::OnBandwidthChanged(
void NearbyConnectionsManagerImpl::OnPayloadReceived(
absl::string_view endpoint_id, Payload& payload) {
MutexLock lock(&mutex_);
NL_LOG(INFO) << "Received payload id=" << payload.id;
[[maybe_unused]] auto result =
LOG(INFO) << "Received payload id=" << payload.id;
if (NearbyFlags::GetInstance().GetBoolFlag(
sharing::config_package_nearby::nearby_sharing_feature::
kDeleteUnexpectedReceivedFileFix)) {
if (payload.content.type != PayloadContent::Type::kBytes &&
!payload_status_listeners_.contains(payload.id)) {
LOG(WARNING) << __func__
<< ": Received unknown payload. Canceling.";
DeleteUnknownFilePayloadAndCancel(payload);
return;
}
if (!incoming_payloads_.contains(payload.id)) {
incoming_payloads_.emplace(payload.id, std::move(payload));
NL_DCHECK(result.second);
return;
}
LOG(WARNING) << __func__ << ": Payload id already exists. Canceling.";
DeleteUnknownFilePayloadAndCancel(payload);
} else {
[[maybe_unused]] auto result =
incoming_payloads_.emplace(payload.id, std::move(payload));
NL_DCHECK(result.second);
}
}
void NearbyConnectionsManagerImpl::DeleteUnknownFilePayloadAndCancel(
Payload& payload) {
if (payload.content.type == PayloadContent::Type::kFile) {
MutexLock lock(&mutex_);
file_paths_to_delete_.insert(payload.content.file_payload.file.path);
}
Cancel(payload.id);
}
void NearbyConnectionsManagerImpl::ProcessUnknownFilePathsToDelete(
@@ -796,10 +822,10 @@ void NearbyConnectionsManagerImpl::ProcessUnknownFilePathsToDelete(
// Unknown payload comes as kInProgress and kCanceled status with kFile type
// from NearbyConnections. Delete it.
if ((status == PayloadStatus::kCanceled ||
status == PayloadStatus::kInProgress) &&
status == PayloadStatus::kInProgress) &&
type == PayloadContent::Type::kFile) {
NL_LOG(WARNING) << __func__
<< ": Unknown payload has been canceled, removing.";
LOG(WARNING) << __func__
<< ": Unknown payload has been canceled, removing.";
MutexLock lock(&mutex_);
file_paths_to_delete_.insert(path);
}
@@ -833,10 +859,10 @@ void NearbyConnectionsManagerImpl::RemoveStatusListenerForPayloadId(
void NearbyConnectionsManagerImpl::OnPayloadTransferUpdate(
absl::string_view endpoint_id, const PayloadTransferUpdate& update) {
NL_LOG(INFO) << "Received payload transfer update id=" << update.payload_id
<< ",status=" << PayloadStatusToString(update.status)
<< ",total=" << update.total_bytes
<< ",bytes_transferred=" << update.bytes_transferred;
LOG(INFO) << "Received payload transfer update id=" << update.payload_id
<< ",status=" << PayloadStatusToString(update.status)
<< ",total=" << update.total_bytes
<< ",bytes_transferred=" << update.bytes_transferred;
// If this is a payload we've registered for, then forward its status to
// the PayloadStatusListener if it still exists. We don't need to do
@@ -870,13 +896,17 @@ void NearbyConnectionsManagerImpl::OnPayloadTransferUpdate(
auto payload = GetIncomingPayload(update.payload_id);
if (payload == nullptr) return;
if (payload->content.type != PayloadContent::Type::kBytes) {
NL_LOG(WARNING) << "Received unknown payload of file type. Cancelling.";
nearby_connections_service_->CancelPayload(kServiceId, payload->id,
[](Status status) {});
ProcessUnknownFilePathsToDelete(update.status, payload->content.type,
payload->content.file_payload.file.path);
return;
if (!NearbyFlags::GetInstance().GetBoolFlag(
sharing::config_package_nearby::nearby_sharing_feature::
kDeleteUnexpectedReceivedFileFix)) {
if (payload->content.type != PayloadContent::Type::kBytes) {
LOG(WARNING) << "Received unknown payload of file type. Cancelling.";
nearby_connections_service_->CancelPayload(kServiceId, payload->id,
[](Status status) {});
ProcessUnknownFilePathsToDelete(update.status, payload->content.type,
payload->content.file_payload.file.path);
return;
}
}
if (update.status != PayloadStatus::kSuccess) return;
@@ -884,7 +914,7 @@ void NearbyConnectionsManagerImpl::OnPayloadTransferUpdate(
NearbyConnectionImpl* connection = GetConnectionForId(endpoint_id);
if (connection == nullptr) return;
NL_LOG(INFO) << "Writing incoming byte message to NearbyConnection.";
LOG(INFO) << "Writing incoming byte message to NearbyConnection.";
connection->WriteMessage(payload->content.bytes_payload.bytes);
}
@@ -976,6 +1006,11 @@ void NearbyConnectionsManagerImpl::OnPayloadTransferUpdateForTesting(
OnPayloadTransferUpdate(endpoint_id, update);
}
void NearbyConnectionsManagerImpl::OnPayloadReceivedForTesting(
absl::string_view endpoint_id, Payload& payload) {
OnPayloadReceived(endpoint_id, payload);
}
std::string NearbyConnectionsManagerImpl::Dump() const {
return nearby_connections_service_->Dump();
}
@@ -102,6 +102,8 @@ class NearbyConnectionsManagerImpl : public NearbyConnectionsManager {
const std::filesystem::path& path);
void OnPayloadTransferUpdateForTesting(absl::string_view endpoint_id,
const PayloadTransferUpdate& update);
void OnPayloadReceivedForTesting(absl::string_view endpoint_id,
Payload& payload);
private:
// EndpointDiscoveryListener:
@@ -127,6 +129,7 @@ class NearbyConnectionsManagerImpl : public NearbyConnectionsManager {
void ProcessUnknownFilePathsToDelete(PayloadStatus status,
PayloadContent::Type type,
const std::filesystem::path& path);
void DeleteUnknownFilePayloadAndCancel(Payload& payload);
absl::flat_hash_set<std::filesystem::path> GetUnknownFilePathsToDelete();
std::optional<std::weak_ptr<PayloadStatusListener>> GetStatusListenerForId(
+126 -13
View File
@@ -150,13 +150,7 @@ class NearbyConnectionsManagerImplTest : public testing::Test {
}
void TearDown() override {
NearbyFlags::GetInstance().OverrideBoolFlagValue(
config_package_nearby::nearby_sharing_feature::kEnableMediumWebRtc,
false);
NearbyFlags::GetInstance().OverrideBoolFlagValue(
config_package_nearby::nearby_sharing_feature::kEnableMediumWifiLan,
true);
NearbyFlags::GetInstance().ResetOverridedValues();
fake_task_runner_.SyncWithTimeout(absl::Seconds(1));
}
@@ -1448,10 +1442,9 @@ TEST_F(NearbyConnectionsManagerImplTest, IncomingBytesPayload) {
StartAdvertising(connection_listener_remote, incoming_connection_listener);
NearbyConnectionsService::PayloadListener payload_listener_remote;
NearbyConnection* connection = OnIncomingConnection(
ASSERT_TRUE(OnIncomingConnection(
connection_listener_remote, incoming_connection_listener,
payload_listener_remote);
EXPECT_TRUE(connection);
payload_listener_remote) != nullptr);
auto payload_listener =
std::make_shared<testing::NiceMock<MockPayloadStatusListener>>();
@@ -1827,10 +1820,9 @@ TEST_F(NearbyConnectionsManagerImplTest,
StartAdvertising(connection_listener_remote, incoming_connection_listener);
NearbyConnectionsService::PayloadListener payload_listener_remote;
NearbyConnection* connection = OnIncomingConnection(
ASSERT_TRUE(OnIncomingConnection(
connection_listener_remote, incoming_connection_listener,
payload_listener_remote);
EXPECT_TRUE(connection);
payload_listener_remote) != nullptr);
std::filesystem::path file(std::filesystem::temp_directory_path() /
"file.jpg");
payload_listener_remote.payload_cb(kRemoteEndpointId,
@@ -1855,6 +1847,127 @@ TEST_F(NearbyConnectionsManagerImplTest,
EXPECT_TRUE(unknown_file_paths.empty());
}
TEST_F(NearbyConnectionsManagerImplTest,
OnPayloadReceivedForUnknownFile) {
NearbyConnectionsService::ConnectionListener connection_listener_remote;
testing::NiceMock<MockIncomingConnectionListener>
incoming_connection_listener;
StartAdvertising(connection_listener_remote, incoming_connection_listener);
NearbyConnectionsService::PayloadListener payload_listener_remote;
ASSERT_TRUE(OnIncomingConnection(
connection_listener_remote, incoming_connection_listener,
payload_listener_remote) != nullptr);
std::filesystem::path file(std::filesystem::temp_directory_path() /
"file.jpg");
payload_listener_remote.payload_cb(kRemoteEndpointId,
Payload(kPayloadId, InputFile(file)));
// Flag is on. Add unknown file paths with kCanceled to the list.
NearbyFlags::GetInstance().OverrideBoolFlagValue(
config_package_nearby::nearby_sharing_feature::
kDeleteUnexpectedReceivedFileFix,
true);
nearby_connections_manager_->ClearIncomingPayloads();
Payload payload(kPayloadId, InputFile(file));
nearby_connections_manager_->OnPayloadReceivedForTesting(
kRemoteEndpointId, payload);
std::filesystem::path file2(std::filesystem::temp_directory_path() /
"file2.jpg");
Payload payload2(kPayloadId, InputFile(file2));
nearby_connections_manager_->OnPayloadReceivedForTesting(
kRemoteEndpointId, payload2);
auto unknown_file_paths =
nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
EXPECT_EQ(unknown_file_paths.size(), 2);
auto payload_listener =
std::make_shared<testing::NiceMock<MockPayloadStatusListener>>();
nearby_connections_manager_->RegisterPayloadStatusListener(
kPayloadId, payload_listener->GetWeakPtr());
std::filesystem::path file3(std::filesystem::temp_directory_path() /
"file3.jpg");
Payload payload3(kPayloadId, InputFile(file3));
nearby_connections_manager_->OnPayloadReceivedForTesting(
kRemoteEndpointId, payload3);
unknown_file_paths =
nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
EXPECT_EQ(unknown_file_paths.size(), 0);
}
TEST_F(NearbyConnectionsManagerImplTest,
OnPayloadReceivedDeletePreviousFileWithSamePayloadId) {
NearbyFlags::GetInstance().OverrideBoolFlagValue(
config_package_nearby::nearby_sharing_feature::
kDeleteUnexpectedReceivedFileFix,
true);
NearbyConnectionsService::ConnectionListener connection_listener_remote;
testing::NiceMock<MockIncomingConnectionListener>
incoming_connection_listener;
StartAdvertising(connection_listener_remote, incoming_connection_listener);
NearbyConnectionsService::PayloadListener payload_listener_remote;
ASSERT_TRUE(OnIncomingConnection(
connection_listener_remote, incoming_connection_listener,
payload_listener_remote) != nullptr);
auto payload_listener =
std::make_shared<testing::NiceMock<MockPayloadStatusListener>>();
nearby_connections_manager_->RegisterPayloadStatusListener(
kPayloadId, payload_listener->GetWeakPtr());
std::filesystem::path file(std::filesystem::temp_directory_path() /
"file.jpg");
Payload payload(kPayloadId, InputFile(file));
nearby_connections_manager_->OnPayloadReceivedForTesting(
kRemoteEndpointId, payload);
auto unknown_file_paths =
nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting();
EXPECT_EQ(unknown_file_paths.size(), 0);
absl::Notification cancel_notification;
EXPECT_CALL(*nearby_connections_, CancelPayload)
.WillOnce([&](absl::string_view service_id, int64_t payload_id,
std::function<void(Status status)> callback) {
EXPECT_EQ(service_id, kServiceId);
EXPECT_EQ(payload_id, kPayloadId);
std::move(callback)(Status::kSuccess);
cancel_notification.Notify();
});
absl::Notification payload_notification;
EXPECT_CALL(*payload_listener, OnStatusUpdate)
.WillOnce([&](std::unique_ptr<PayloadTransferUpdate> update,
std::optional<Medium> upgraded_medium) {
EXPECT_EQ(update->payload_id, kPayloadId);
EXPECT_EQ(update->status, PayloadStatus::kCanceled);
EXPECT_EQ(update->total_bytes, 0u);
EXPECT_EQ(update->bytes_transferred, 0u);
EXPECT_FALSE(upgraded_medium.has_value());
payload_notification.Notify();
});
std::filesystem::path file2(std::filesystem::temp_directory_path() /
"file2.jpg");
Payload payload2(kPayloadId, InputFile(file2));
nearby_connections_manager_->OnPayloadReceivedForTesting(kRemoteEndpointId,
payload2);
unknown_file_paths =
nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete();
EXPECT_EQ(unknown_file_paths.size(), 1);
EXPECT_TRUE(payload_notification.WaitForNotificationWithTimeout(
kSynchronizationTimeOut));
EXPECT_TRUE(cancel_notification.WaitForNotificationWithTimeout(
kSynchronizationTimeOut));
}
TEST_F(NearbyConnectionsManagerImplTest, ProcessUnknownFilePathsToDelete) {
std::filesystem::path file(std::filesystem::temp_directory_path() /
"file.jpg");