From a882f5816d6caf26c0df810d6323eead82cfc41c Mon Sep 17 00:00:00 2001 From: Anay Wadhera Date: Mon, 3 Jun 2024 17:48:58 -0700 Subject: [PATCH] Block targets with the same vendor ID This CL does many things: 1) Adds a new API for RegisterSendSurface to specify vendor ID and whether we should block targets with the same vendor ID. 2) Refactors the sending surface callback observer lists to maps, mirroring Android. 3) Uses WrappedShareTargetDiscoveredCallback, which wraps the passed-in `ShareTargetDiscoveredCallback` and adds blocking on vendor ID (the actual point of this CL). PiperOrigin-RevId: 639969139 --- sharing/BUILD | 1 + sharing/advertisement_test.cc | 23 ++ sharing/nearby_sharing_service.h | 13 ++ sharing/nearby_sharing_service_impl.cc | 219 ++++++++++-------- sharing/nearby_sharing_service_impl.h | 45 ++-- sharing/nearby_sharing_service_impl_test.cc | 74 +++++- ...rapped_share_target_discovered_callback.cc | 8 +- ...wrapped_share_target_discovered_callback.h | 9 +- ...d_share_target_discovered_callback_test.cc | 20 +- 9 files changed, 276 insertions(+), 136 deletions(-) diff --git a/sharing/BUILD b/sharing/BUILD index 63a69169..19b868e9 100644 --- a/sharing/BUILD +++ b/sharing/BUILD @@ -282,6 +282,7 @@ cc_test( ":types", "//sharing/common:enum", "@com_github_protobuf_matchers//protobuf-matchers", + "@com_google_absl//absl/types:span", "@com_google_googletest//:gtest_main", ] + select({ "@platforms//os:windows": [ diff --git a/sharing/advertisement_test.cc b/sharing/advertisement_test.cc index 577aa15c..6631085b 100644 --- a/sharing/advertisement_test.cc +++ b/sharing/advertisement_test.cc @@ -20,12 +20,23 @@ #include #include "gtest/gtest.h" +#include "absl/types/span.h" #include "sharing/common/nearby_share_enums.h" namespace nearby { namespace sharing { namespace { +constexpr uint8_t kQrCodeTlvData[]{ + 0x01, // QrCode TLV type. + 0x0f, // QrCode data length. + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, // QR Code data. +}; + +constexpr absl::Span kQrCodeTlvBytes = + absl::MakeConstSpan(kQrCodeTlvData); + struct TestParameters { std::vector salt; std::vector encrypted_metadata_key; @@ -46,6 +57,18 @@ TEST_P(AdvertisementTest, TestAdvertisementRoundTrip) { EXPECT_EQ(*advertisement_from_bytes, *advertisement); } +TEST(BadAdvertisementTest, TestTlvParsingOnAdvertisement) { + auto advertisement = Advertisement::NewInstance( + std::vector(Advertisement::kSaltSize), + std::vector(Advertisement::kMetadataEncryptionKeyHashByteSize), + ShareTargetType::kLaptop, std::nullopt, /*vendor_id=*/1); + auto bytes = advertisement->ToEndpointInfo(); + // Add a TLV field for QR code. + bytes.insert(bytes.end(), kQrCodeTlvBytes.begin(), kQrCodeTlvBytes.end()); + auto advertisement_from_bytes = Advertisement::FromEndpointInfo(bytes); + EXPECT_EQ(*advertisement_from_bytes, *advertisement); +} + INSTANTIATE_TEST_SUITE_P( ShareTargetTypes, AdvertisementTest, testing::Values( diff --git a/sharing/nearby_sharing_service.h b/sharing/nearby_sharing_service.h index 1867c410..ed9abdf0 100644 --- a/sharing/nearby_sharing_service.h +++ b/sharing/nearby_sharing_service.h @@ -132,11 +132,24 @@ class NearbySharingService { // Registers a send surface for handling payload transfer status and device // discovery. + ABSL_DEPRECATED("Use the variant with vendor ID/blocking request instead.") virtual void RegisterSendSurface( TransferUpdateCallback* transfer_callback, ShareTargetDiscoveredCallback* discovery_callback, SendSurfaceState state, std::function status_codes_callback) = 0; + // Registers a send surface for handling payload transfer status and device + // discovery, with optional blocking on a specified vendor ID. + // |transfer_callback| is used as the main identity for the surface, so trying + // to re-register the same transfer callback with a different + // |discovery_callback| will result in an error delivered via the status + // callback. + virtual void RegisterSendSurface( + TransferUpdateCallback* transfer_callback, + ShareTargetDiscoveredCallback* discovery_callback, SendSurfaceState state, + Advertisement::BlockedVendorId blocked_vendor_id, + std::function status_codes_callback) = 0; + // Unregisters the current send surface. virtual void UnregisterSendSurface( TransferUpdateCallback* transfer_callback, diff --git a/sharing/nearby_sharing_service_impl.cc b/sharing/nearby_sharing_service_impl.cc index a4d9b499..788e149c 100644 --- a/sharing/nearby_sharing_service_impl.cc +++ b/sharing/nearby_sharing_service_impl.cc @@ -109,6 +109,7 @@ #include "sharing/transfer_metadata_builder.h" #include "sharing/transfer_update_callback.h" #include "sharing/wifi_credentials_attachment.h" +#include "sharing/wrapped_share_target_discovered_callback.h" namespace nearby { namespace sharing { @@ -321,10 +322,8 @@ void NearbySharingServiceImpl::Cleanup() { discovered_advertisements_to_retry_map_.clear(); discovered_advertisements_retried_set_.clear(); - foreground_send_transfer_callbacks_.Clear(); - background_send_transfer_callbacks_.Clear(); - foreground_send_discovery_callbacks_.Clear(); - background_send_discovery_callbacks_.Clear(); + foreground_send_surface_map_.clear(); + background_send_surface_map_.clear(); last_incoming_metadata_.reset(); last_outgoing_metadata_.reset(); @@ -364,9 +363,17 @@ void NearbySharingServiceImpl::RegisterSendSurface( TransferUpdateCallback* transfer_callback, ShareTargetDiscoveredCallback* discovery_callback, SendSurfaceState state, std::function status_codes_callback) { + RegisterSendSurface(transfer_callback, discovery_callback, state, + BlockedVendorId::kNone, std::move(status_codes_callback)); +} +void NearbySharingServiceImpl::RegisterSendSurface( + TransferUpdateCallback* transfer_callback, + ShareTargetDiscoveredCallback* discovery_callback, SendSurfaceState state, + BlockedVendorId blocked_vendor_id, + std::function status_codes_callback) { RunOnNearbySharingServiceThread( "api_register_send_surface", - [this, transfer_callback, discovery_callback, state, + [this, transfer_callback, discovery_callback, state, blocked_vendor_id, status_codes_callback = std::move(status_codes_callback)]() { NL_DCHECK(transfer_callback); NL_DCHECK(discovery_callback); @@ -378,10 +385,8 @@ void NearbySharingServiceImpl::RegisterSendSurface( : "Background") << ", transfer_callback: " << transfer_callback; - if (foreground_send_transfer_callbacks_.HasObserver( - transfer_callback) || - background_send_transfer_callbacks_.HasObserver( - transfer_callback)) { + if (foreground_send_surface_map_.contains(transfer_callback) || + background_send_surface_map_.contains(transfer_callback)) { NL_VLOG(1) << __func__ << ": RegisterSendSurface failed. Already registered for a " @@ -389,6 +394,17 @@ void NearbySharingServiceImpl::RegisterSendSurface( std::move(status_codes_callback)(StatusCodes::kInvalidArgument); return; } + BlockedVendorId sending_id = GetSendingVendorId(); + if (ShouldBlockSurfaceRegistration(blocked_vendor_id, sending_id)) { + NL_LOG(INFO) << __func__ + << ": RegisterSendSurface failed. Already registered to " + "block a different vendor ID " + << static_cast(sending_id); + std::move(status_codes_callback)(StatusCodes::kInvalidArgument); + return; + } + WrappedShareTargetDiscoveredCallback wrapped_callback( + discovery_callback, blocked_vendor_id); if (state == SendSurfaceState::kForeground) { // Only check this error case for foreground senders @@ -399,15 +415,15 @@ void NearbySharingServiceImpl::RegisterSendSurface( return; } - foreground_send_transfer_callbacks_.AddObserver(transfer_callback); - foreground_send_discovery_callbacks_.AddObserver(discovery_callback); + foreground_send_surface_map_.insert( + {transfer_callback, wrapped_callback}); } else { - background_send_transfer_callbacks_.AddObserver(transfer_callback); - background_send_discovery_callbacks_.AddObserver(discovery_callback); + background_send_surface_map_.insert( + {transfer_callback, wrapped_callback}); } if (is_receiving_files_) { - InternalUnregisterSendSurface(transfer_callback, discovery_callback); + InternalUnregisterSendSurface(transfer_callback); NL_VLOG(1) << __func__ << ": Ignore registering (and unregistering if registered) send " @@ -423,7 +439,10 @@ void NearbySharingServiceImpl::RegisterSendSurface( last_outgoing_metadata_.has_value()) { // When a new share sheet is registered, we want to immediately show // the in-progress bar. - discovery_callback->OnShareTargetDiscovered( + // TODO(b/341740930): Make sure we absolutely do not deliver updates + // to blocked targets, and block any interaction with them if the + // request comes from a surface with the blocked vendor ID. + wrapped_callback.OnShareTargetDiscovered( last_outgoing_metadata_->first); transfer_callback->OnTransferUpdate( last_outgoing_metadata_->first, @@ -451,7 +470,10 @@ void NearbySharingServiceImpl::RegisterSendSurface( // targets from current scanning session. if (is_scanning_) { for (const auto& item : outgoing_share_target_map_) { - discovery_callback->OnShareTargetDiscovered(item.second); + NL_LOG(INFO) << "Reporting discovered target " + << item.second.ToString() + << " when registering send surface"; + wrapped_callback.OnShareTargetDiscovered(item.second); } } @@ -465,15 +487,10 @@ void NearbySharingServiceImpl::RegisterSendSurface( << ": A SendSurface has been registered for state: " << SendSurfaceStateToString(state); - NL_VLOG(1) - << "RegisterSendSurface: foreground_send_transfer_callbacks_:" - << foreground_send_transfer_callbacks_.size() - << ", foreground_send_discovery_callbacks_:" - << foreground_send_discovery_callbacks_.size() - << ", background_send_transfer_callbacks_:" - << background_send_transfer_callbacks_.size() - << ", background_send_discovery_callbacks_" - << background_send_discovery_callbacks_.size(); + NL_VLOG(1) << "RegisterSendSurface: foreground_send_surface_map_:" + << foreground_send_surface_map_.size() + << ", background_send_surface_map_:" + << background_send_surface_map_.size(); InvalidateSendSurfaceState(); std::move(status_codes_callback)(StatusCodes::kOk); @@ -488,18 +505,13 @@ void NearbySharingServiceImpl::UnregisterSendSurface( "api_unregister_send_surface", [this, transfer_callback, discovery_callback, status_codes_callback = std::move(status_codes_callback)]() { - StatusCodes status_codes = InternalUnregisterSendSurface( - transfer_callback, discovery_callback); + StatusCodes status_codes = + InternalUnregisterSendSurface(transfer_callback); - NL_VLOG(1) - << "UnregisterSendSurface: foreground_send_transfer_callbacks_:" - << foreground_send_transfer_callbacks_.size() - << ", foreground_send_discovery_callbacks_:" - << foreground_send_discovery_callbacks_.size() - << ", background_send_transfer_callbacks_:" - << background_send_transfer_callbacks_.size() - << ", background_send_discovery_callbacks_" - << background_send_discovery_callbacks_.size(); + NL_VLOG(1) << "UnregisterSendSurface: foreground_send_surface_map_:" + << foreground_send_surface_map_.size() + << ", background_send_surface_map_:" + << background_send_surface_map_.size(); std::move(status_codes_callback)(status_codes); }); @@ -531,7 +543,7 @@ void NearbySharingServiceImpl::RegisterReceiveSurface( StatusCodes::kNoAvailableConnectionMedium); return; } - BlockedVendorId before_registration_vendor_id = GetVendorId(); + BlockedVendorId before_registration_vendor_id = GetReceivingVendorId(); // We specifically allow re-registering without error, so it is clear to // caller that the transfer_callback is currently registered. @@ -558,7 +570,7 @@ void NearbySharingServiceImpl::RegisterReceiveSurface( "that has vendor_id " << static_cast(vendor_id) << " because the current vendor_id is " - << static_cast(GetVendorId()); + << static_cast(GetReceivingVendorId()); std::move(status_codes_callback)(StatusCodes::kInvalidArgument); return; } @@ -680,8 +692,8 @@ void NearbySharingServiceImpl::SendAttachments( } // |is_scanning_| means at least one send transfer callback. - NL_DCHECK(!foreground_send_transfer_callbacks_.empty() || - !background_send_transfer_callbacks_.empty()); + NL_DCHECK(!foreground_send_surface_map_.empty() || + !background_send_surface_map_.empty()); // |is_scanning_| and |is_transferring_| are mutually exclusive. NL_DCHECK(!is_transferring_); @@ -1089,22 +1101,20 @@ void NearbySharingServiceImpl::OnIncomingConnection( NearbySharingService::StatusCodes NearbySharingServiceImpl::InternalUnregisterSendSurface( - TransferUpdateCallback* transfer_callback, - ShareTargetDiscoveredCallback* discovery_callback) { + TransferUpdateCallback* transfer_callback) { NL_DCHECK(transfer_callback); - NL_DCHECK(discovery_callback); NL_LOG(INFO) << __func__ << ": UnregisterSendSurface is called" << ", transfer_callback: " << transfer_callback; - if (!foreground_send_transfer_callbacks_.HasObserver(transfer_callback) && - !background_send_transfer_callbacks_.HasObserver(transfer_callback)) { + if (!foreground_send_surface_map_.contains(transfer_callback) && + !background_send_surface_map_.contains(transfer_callback)) { NL_VLOG(1) << __func__ << ": unregisterSendSurface failed. Unknown TransferUpdateCallback"; return StatusCodes::kError; } - if (!foreground_send_transfer_callbacks_.empty() && last_outgoing_metadata_ && + if (!foreground_send_surface_map_.empty() && last_outgoing_metadata_ && last_outgoing_metadata_->second.is_final_status()) { // We already saw the final status in the foreground // Nullify it so the next time the user opens sharing, it starts the UI from @@ -1113,22 +1123,19 @@ NearbySharingServiceImpl::InternalUnregisterSendSurface( } SendSurfaceState state = SendSurfaceState::kUnknown; - if (foreground_send_transfer_callbacks_.HasObserver(transfer_callback)) { - foreground_send_transfer_callbacks_.RemoveObserver(transfer_callback); - foreground_send_discovery_callbacks_.RemoveObserver(discovery_callback); + if (foreground_send_surface_map_.contains(transfer_callback)) { + foreground_send_surface_map_.erase(transfer_callback); state = SendSurfaceState::kForeground; } else { - background_send_transfer_callbacks_.RemoveObserver(transfer_callback); - background_send_discovery_callbacks_.RemoveObserver(discovery_callback); + background_send_surface_map_.erase(transfer_callback); state = SendSurfaceState::kBackground; } // Displays the most recent payload status processed by foreground surfaces on // background surfaces. - if (foreground_send_transfer_callbacks_.empty() && last_outgoing_metadata_) { - for (auto& background_transfer_callback : - background_send_transfer_callbacks_.GetObservers()) { - background_transfer_callback->OnTransferUpdate( + if (foreground_send_surface_map_.empty() && last_outgoing_metadata_) { + for (auto& background_transfer_callback : background_send_surface_map_) { + background_transfer_callback.first->OnTransferUpdate( last_outgoing_metadata_->first, last_outgoing_metadata_->first.attachment_container, last_outgoing_metadata_->second); @@ -1538,7 +1545,7 @@ void NearbySharingServiceImpl::SetupBluetoothAdapter() { InvalidateSurfaceState(); } -BlockedVendorId NearbySharingServiceImpl::GetVendorId() const { +BlockedVendorId NearbySharingServiceImpl::GetReceivingVendorId() const { // Prefer a vendor ID provided by a foreground surface. auto fg_vendor_it = std::find_if(foreground_receive_callbacks_map_.begin(), @@ -1562,6 +1569,28 @@ BlockedVendorId NearbySharingServiceImpl::GetVendorId() const { return BlockedVendorId::kNone; } +BlockedVendorId NearbySharingServiceImpl::GetSendingVendorId() const { + // Prefer a vendor ID provided by a foreground surface. + auto fg_vendor_it = std::find_if( + foreground_send_surface_map_.begin(), foreground_send_surface_map_.end(), + [](const auto& send_surface) { + return send_surface.second.BlockedVendorId() != BlockedVendorId::kNone; + }); + if (fg_vendor_it != foreground_send_surface_map_.end()) { + return fg_vendor_it->second.BlockedVendorId(); + } + // Look through background surfaces. + auto bg_vendor_it = std::find_if( + background_send_surface_map_.begin(), background_send_surface_map_.end(), + [](const auto& send_surface) { + return send_surface.second.BlockedVendorId() != BlockedVendorId::kNone; + }); + if (bg_vendor_it != background_send_surface_map_.end()) { + return bg_vendor_it->second.BlockedVendorId(); + } + return BlockedVendorId::kNone; +} + absl::flat_hash_map& NearbySharingServiceImpl::GetReceiveCallbacksMapFromState( ReceiveSurfaceState state) { @@ -1617,7 +1646,7 @@ NearbySharingServiceImpl::CreateEndpointInfo( std::unique_ptr advertisement = Advertisement::NewInstance( std::move(salt), std::move(encrypted_key), device_type, device_name, - static_cast(GetVendorId())); + static_cast(GetReceivingVendorId())); if (advertisement) { return advertisement->ToEndpointInfo(); } else { @@ -1763,6 +1792,7 @@ void NearbySharingServiceImpl::OnOutgoingAdvertisementDecoded( advertisement_copy = *advertisement](std::optional decrypted_public_certificate) { + NL_LOG(INFO) << __func__ << ": Decrypted public certificate"; OnOutgoingDecryptedCertificate(endpoint_id_copy, endpoint_info_copy, advertisement_copy, decrypted_public_certificate); @@ -1825,17 +1855,15 @@ void NearbySharingServiceImpl::OnOutgoingDecryptedCertificate( // Notifies the user that we discovered a device. NL_VLOG(1) << __func__ << ": There are " - << (foreground_send_discovery_callbacks_.size() + - background_send_discovery_callbacks_.size()) + << (foreground_send_surface_map_.size() + + background_send_surface_map_.size()) << " discovery callbacks be called."; - for (ShareTargetDiscoveredCallback* discovery_callback : - foreground_send_discovery_callbacks_.GetObservers()) { - discovery_callback->OnShareTargetDiscovered(*share_target); + for (auto& entry : foreground_send_surface_map_) { + entry.second.OnShareTargetDiscovered(*share_target); } - for (ShareTargetDiscoveredCallback* discovery_callback : - background_send_discovery_callbacks_.GetObservers()) { - discovery_callback->OnShareTargetDiscovered(*share_target); + for (auto& entry : background_send_surface_map_) { + entry.second.OnShareTargetDiscovered(*share_target); } NL_VLOG(1) << __func__ << ": Reported OnShareTargetDiscovered " @@ -1964,7 +1992,7 @@ void NearbySharingServiceImpl::InvalidateScanningState() { return; } - if (foreground_send_transfer_callbacks_.empty()) { + if (foreground_send_surface_map_.empty()) { StopScanning(); NL_VLOG(1) << __func__ << ": Stopping discovery because no scanning surface has been " @@ -2013,11 +2041,12 @@ void NearbySharingServiceImpl::InvalidateFastInitiationAdvertising() { return; } - if (foreground_send_transfer_callbacks_.empty()) { + if (foreground_send_surface_map_.empty()) { StopFastInitiationAdvertising(); - NL_VLOG(1) << __func__ - << ": Stopping fast initiation advertising because no send " - "surface is registered."; + NL_VLOG(1) + << __func__ + << ": Stopping fast initiation advertising because no foreground send " + "surface is registered."; return; } @@ -2205,7 +2234,7 @@ void NearbySharingServiceImpl::StartScanning() { NL_DCHECK(settings_->GetEnabled()); NL_DCHECK(!is_screen_locked_); NL_DCHECK(HasAvailableConnectionMediums()); - NL_DCHECK(!foreground_send_transfer_callbacks_.empty()); + NL_DCHECK(!foreground_send_surface_map_.empty()); if (is_scanning_) { NL_VLOG(1) << __func__ << ": We're currently scanning, ignoring."; @@ -2228,7 +2257,7 @@ void NearbySharingServiceImpl::StartScanning() { // Log analytics event of starting discovery. analytics::AnalyticsInformation analytics_information; analytics_information.send_surface_state = - foreground_send_discovery_callbacks_.empty() + foreground_send_surface_map_.empty() ? analytics::SendSurfaceState::kBackground : analytics::SendSurfaceState::kForeground; analytics_recorder_->NewScanForShareTargetsStart( @@ -2467,23 +2496,11 @@ void NearbySharingServiceImpl::RemoveOutgoingShareTargetWithEndpointId( return; } - for (ShareTargetDiscoveredCallback* discovery_callback : - foreground_send_discovery_callbacks_.GetObservers()) { - if (discovery_callback != nullptr) { - discovery_callback->OnShareTargetLost(share_target); - } else { - NL_LOG(WARNING) << __func__ - << "Foreground Discovery Callback is not exist"; - } + for (auto& entry : foreground_send_surface_map_) { + entry.second.OnShareTargetLost(share_target); } - for (ShareTargetDiscoveredCallback* discovery_callback : - background_send_discovery_callbacks_.GetObservers()) { - if (discovery_callback != nullptr) { - discovery_callback->OnShareTargetLost(share_target); - } else { - NL_LOG(WARNING) << __func__ - << "Background Discovery Callback is not exist"; - } + for (auto& entry : background_send_surface_map_) { + entry.second.OnShareTargetLost(share_target); } NL_VLOG(1) << __func__ << ": Reported OnShareTargetLost"; @@ -2726,8 +2743,8 @@ void NearbySharingServiceImpl::SendIntroduction( NearbyConnection* connection = info->connection(); - if (foreground_send_transfer_callbacks_.empty() && - background_send_transfer_callbacks_.empty()) { + if (foreground_send_surface_map_.empty() && + background_send_surface_map_.empty()) { NL_LOG(WARNING) << __func__ << ": No transfer callbacks, disconnecting."; connection->Close(); return; @@ -3230,18 +3247,22 @@ void NearbySharingServiceImpl::OnOutgoingTransferUpdate( OnTransferStarted(/*is_incoming=*/false); } - bool has_foreground_send_surface = - !foreground_send_transfer_callbacks_.empty(); - ObserverList& transfer_callbacks = - has_foreground_send_surface ? foreground_send_transfer_callbacks_ - : background_send_transfer_callbacks_; + bool has_foreground_send_surface = !foreground_send_surface_map_.empty(); if (info) { ShareTarget cached_share_target = info->share_target(); // only call transfer update when having share target info. - for (TransferUpdateCallback* callback : transfer_callbacks.GetObservers()) { - callback->OnTransferUpdate(cached_share_target, - cached_share_target.attachment_container, - metadata); + if (has_foreground_send_surface) { + for (auto& entry : foreground_send_surface_map_) { + entry.first->OnTransferUpdate(cached_share_target, + cached_share_target.attachment_container, + metadata); + } + } else { + for (auto& entry : background_send_surface_map_) { + entry.first->OnTransferUpdate(cached_share_target, + cached_share_target.attachment_container, + metadata); + } } // check whether need to send next payload. diff --git a/sharing/nearby_sharing_service_impl.h b/sharing/nearby_sharing_service_impl.h index 3151178f..043404a4 100644 --- a/sharing/nearby_sharing_service_impl.h +++ b/sharing/nearby_sharing_service_impl.h @@ -27,6 +27,7 @@ #include #include +#include "absl/base/attributes.h" #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "absl/functional/any_invocable.h" @@ -82,6 +83,7 @@ #include "sharing/transfer_metadata.h" #include "sharing/transfer_update_callback.h" #include "sharing/wifi_credentials_attachment.h" +#include "sharing/wrapped_share_target_discovered_callback.h" namespace nearby { namespace sharing { @@ -124,10 +126,16 @@ class NearbySharingServiceImpl bool HasObserver(NearbySharingService::Observer* observer) override; void Shutdown( std::function status_codes_callback) override; + ABSL_DEPRECATED("Use the variant with vendor ID instead.") void RegisterSendSurface( TransferUpdateCallback* transfer_callback, ShareTargetDiscoveredCallback* discovery_callback, SendSurfaceState state, std::function status_codes_callback) override; + void RegisterSendSurface( + TransferUpdateCallback* transfer_callback, + ShareTargetDiscoveredCallback* discovery_callback, SendSurfaceState state, + Advertisement::BlockedVendorId blocked_vendor_id, + std::function status_codes_callback) override; void UnregisterSendSurface( TransferUpdateCallback* transfer_callback, ShareTargetDiscoveredCallback* discovery_callback, @@ -195,8 +203,7 @@ class NearbySharingServiceImpl private: // Internal implementation of methods to avoid using recursive mutex. StatusCodes InternalUnregisterSendSurface( - TransferUpdateCallback* transfer_callback, - ShareTargetDiscoveredCallback* discovery_callback); + TransferUpdateCallback* transfer_callback); StatusCodes InternalUnregisterReceiveSurface( TransferUpdateCallback* transfer_callback); @@ -250,10 +257,16 @@ class NearbySharingServiceImpl absl::flat_hash_map& GetReceiveCallbacksMapFromState(ReceiveSurfaceState state); - // Retrieves the current vendor ID, defaulting to kNone if no surface has been - // registered with a different vendor ID. This function will always return one - // vendor ID, preferring a foreground surface vendor ID if available. - Advertisement::BlockedVendorId GetVendorId() const; + // Retrieves the current receiving vendor ID, defaulting to kNone if no + // surface has been registered with a different vendor ID. This function will + // always return one vendor ID, preferring a foreground surface vendor ID if + // available. + Advertisement::BlockedVendorId GetReceivingVendorId() const; + // Retrieves the current sending vendor ID, defaulting to kNone if no + // surface has been registered with a different vendor ID. This function will + // always return one vendor ID, preferring a foreground surface vendor ID if + // available. + Advertisement::BlockedVendorId GetSendingVendorId() const; bool IsVisibleInBackground(proto::DeviceVisibility visibility); std::optional> CreateEndpointInfo( proto::DeviceVisibility visibility, @@ -530,18 +543,14 @@ class NearbySharingServiceImpl // A map of background receiver callbacks -> vendor ID. absl::flat_hash_map background_receive_callbacks_map_; - // A list of foreground receivers for transfer updates on the send surface. - ObserverList foreground_send_transfer_callbacks_; - // A list of foreground receivers for discovered device updates on the send - // surface. - ObserverList - foreground_send_discovery_callbacks_; - // A list of background receivers for transfer updates on the send surface. - ObserverList background_send_transfer_callbacks_; - // A list of background receivers for discovered device updates on the send - // surface. - ObserverList - background_send_discovery_callbacks_; + // A mapping of foreground transfer callbacks to foreground send surface data. + absl::flat_hash_map + foreground_send_surface_map_; + // A mapping of background transfer callbacks to background send surface data. + absl::flat_hash_map + background_send_surface_map_; // Registers the most recent TransferMetadata and ShareTarget used for // transitioning notifications between foreground surfaces and background diff --git a/sharing/nearby_sharing_service_impl_test.cc b/sharing/nearby_sharing_service_impl_test.cc index 0dc50661..3308af8a 100644 --- a/sharing/nearby_sharing_service_impl_test.cc +++ b/sharing/nearby_sharing_service_impl_test.cc @@ -176,7 +176,7 @@ constexpr char kEndpointId[] = "test_endpoint_id"; constexpr char kTextPayload[] = "Test text payload"; constexpr char kFourDigitToken[] = "1953"; constexpr absl::string_view kTestAccountId = "test_account"; -constexpr int32_t kVendorId = 0; +constexpr uint8_t kVendorId = 0; constexpr int64_t kFreeDiskSpace = 10000; @@ -493,13 +493,14 @@ class NearbySharingServiceImplTest : public testing::Test { NearbySharingService::StatusCodes RegisterSendSurface( TransferUpdateCallback* transfer_callback, - ShareTargetDiscoveredCallback* discovery_callback, - SendSurfaceState state) { + ShareTargetDiscoveredCallback* discovery_callback, SendSurfaceState state, + Advertisement::BlockedVendorId vendor_id = + static_cast(kVendorId)) { NearbySharingService::StatusCodes result = NearbySharingService::StatusCodes::kError; absl::Notification notification; service_->RegisterSendSurface( - transfer_callback, discovery_callback, state, + transfer_callback, discovery_callback, state, vendor_id, [&](NearbySharingService::StatusCodes status_codes) { result = status_codes; notification.Notify(); @@ -1083,9 +1084,15 @@ class NearbySharingServiceImplTest : public testing::Test { } void FindEndpoint(absl::string_view endpoint_id) { + FindEndpointWithVendorId(endpoint_id, kVendorId); + } + + void FindEndpointWithVendorId(absl::string_view endpoint_id, + uint8_t vendor_id) { fake_nearby_connections_manager_->OnEndpointFound( - endpoint_id, std::make_unique( - GetValidV1EndpointInfo(), kServiceId)); + endpoint_id, + std::make_unique( + GetValidV1EndpointInfoWithVendor(vendor_id), kServiceId)); FlushTesting(); } @@ -4511,6 +4518,61 @@ TEST_F(NearbySharingServiceImplTest, EXPECT_FALSE(fake_nearby_connections_manager_->is_shutdown()); } +TEST_F(NearbySharingServiceImplTest, BlockTargetWithSameVendorId) { + InSequence s; + // Set up advertisement decoder. + EXPECT_CALL(fake_decoder_, DecodeAdvertisement(testing::_)) + .WillRepeatedly(testing::Invoke([=](absl::Span data) { + return Advertisement::NewInstance( + GetNearbyShareTestEncryptedMetadataKey().salt(), + GetNearbyShareTestEncryptedMetadataKey().encrypted_key(), + kDeviceType, kDeviceName, + static_cast(Advertisement::BlockedVendorId::kSamsung)); + })); + // Register send surface with vendor ID 1 that requests blocking. + MockTransferUpdateCallback callback; + MockShareTargetDiscoveredCallback discovery_callback; + ASSERT_EQ(RegisterSendSurface(&callback, &discovery_callback, + SendSurfaceState::kForeground, + Advertisement::BlockedVendorId::kSamsung), + NearbySharingService::StatusCodes::kOk); + // Verify service will not report discovered. + EXPECT_CALL(discovery_callback, OnShareTargetDiscovered(testing::_)).Times(0); + // Find endpoint with vendor ID 1. + FindEndpointWithVendorId( + /*endpoint_id=*/"1", + static_cast(Advertisement::BlockedVendorId::kSamsung)); + ProcessLatestPublicCertificateDecryption(/*expected_num_calls=*/1, + /*success=*/false); +} + +TEST_F(NearbySharingServiceImplTest, + RegisterSendSurfaceWithDifferentVendorIdIsBlocked) { + SetConnectionType(ConnectionType::kWifi); + preference_manager().SetInteger( + prefs::kNearbySharingBackgroundVisibilityName, + static_cast(DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS)); + FlushTesting(); + + // Register background send surface with vendor ID 1. + MockShareTargetDiscoveredCallback background_discovered_callback; + MockTransferUpdateCallback background_transfer_callback; + NearbySharingService::StatusCodes result = RegisterSendSurface( + &background_transfer_callback, &background_discovered_callback, + NearbySharingService::SendSurfaceState::kBackground, + Advertisement::BlockedVendorId::kSamsung); + ASSERT_EQ(result, NearbySharingService::StatusCodes::kOk); + // Register foreground send surface with different vendor ID. + MockShareTargetDiscoveredCallback foreground_discovered_callback; + MockTransferUpdateCallback foreground_transfer_callback; + result = RegisterSendSurface( + &foreground_transfer_callback, &foreground_discovered_callback, + NearbySharingService::SendSurfaceState::kForeground, + static_cast(2)); + EXPECT_EQ(result, NearbySharingService::StatusCodes::kInvalidArgument); + EXPECT_FALSE(fake_nearby_connections_manager_->IsDiscovering()); +} + TEST_F(NearbySharingServiceImplTest, ScreenLocksDuringAdvertising) { SetConnectionType(ConnectionType::kWifi); MockTransferUpdateCallback callback; diff --git a/sharing/wrapped_share_target_discovered_callback.cc b/sharing/wrapped_share_target_discovered_callback.cc index 5823e80a..4a98539e 100644 --- a/sharing/wrapped_share_target_discovered_callback.cc +++ b/sharing/wrapped_share_target_discovered_callback.cc @@ -16,19 +16,17 @@ #include +#include "sharing/advertisement.h" #include "sharing/internal/public/logging.h" #include "sharing/share_target.h" namespace nearby { namespace sharing { -namespace { -constexpr uint8_t kNoVendorId = 0; -} bool WrappedShareTargetDiscoveredCallback::ShouldBlockShareTarget( const ShareTarget& share_target) const { - return blocked_vendor_id_ != kNoVendorId && - share_target.vendor_id == blocked_vendor_id_; + return blocked_vendor_id_ != Advertisement::BlockedVendorId::kNone && + share_target.vendor_id == static_cast(blocked_vendor_id_); } void WrappedShareTargetDiscoveredCallback::OnShareTargetDiscovered( diff --git a/sharing/wrapped_share_target_discovered_callback.h b/sharing/wrapped_share_target_discovered_callback.h index 6929321f..c1d2952a 100644 --- a/sharing/wrapped_share_target_discovered_callback.h +++ b/sharing/wrapped_share_target_discovered_callback.h @@ -17,6 +17,7 @@ #include +#include "sharing/advertisement.h" #include "sharing/share_target.h" #include "sharing/share_target_discovered_callback.h" @@ -28,17 +29,21 @@ class WrappedShareTargetDiscoveredCallback : public ShareTargetDiscoveredCallback { public: explicit WrappedShareTargetDiscoveredCallback( - ShareTargetDiscoveredCallback* callback, uint8_t blocked_vendor_id) + ShareTargetDiscoveredCallback* callback, + Advertisement::BlockedVendorId blocked_vendor_id) : callback_(callback), blocked_vendor_id_(blocked_vendor_id) {} void OnShareTargetDiscovered(const ShareTarget& target) override; void OnShareTargetUpdated(const ShareTarget& target) override; void OnShareTargetLost(const ShareTarget& target) override; + const Advertisement::BlockedVendorId& BlockedVendorId() const { + return blocked_vendor_id_; + } private: bool ShouldBlockShareTarget(const ShareTarget& target) const; ShareTargetDiscoveredCallback* callback_; - const uint8_t blocked_vendor_id_; + const Advertisement::BlockedVendorId blocked_vendor_id_; }; } // namespace sharing } // namespace nearby diff --git a/sharing/wrapped_share_target_discovered_callback_test.cc b/sharing/wrapped_share_target_discovered_callback_test.cc index 5efe3ccd..d187e953 100644 --- a/sharing/wrapped_share_target_discovered_callback_test.cc +++ b/sharing/wrapped_share_target_discovered_callback_test.cc @@ -19,6 +19,7 @@ #include "gmock/gmock.h" #include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" +#include "sharing/advertisement.h" #include "sharing/share_target.h" #include "sharing/share_target_discovered_callback.h" @@ -27,6 +28,7 @@ namespace sharing { namespace { using ::testing::_; +using BlockedVendorId = Advertisement::BlockedVendorId; class MockShareTargetDiscoveredCallback : public ShareTargetDiscoveredCallback { public: @@ -44,7 +46,8 @@ ShareTarget GetShareTarget(uint8_t vendor_id) { TEST(WrappedShareTargetDiscoveredCallbackTest, BlocksDiscoveryForSameVendorId) { MockShareTargetDiscoveredCallback callback; ShareTarget share_target = GetShareTarget(/*vendor_id=*/1); - WrappedShareTargetDiscoveredCallback wrapped(&callback, /*vendor_id=*/1); + WrappedShareTargetDiscoveredCallback wrapped(&callback, + BlockedVendorId::kSamsung); EXPECT_CALL(callback, OnShareTargetDiscovered(_)).Times(0); wrapped.OnShareTargetDiscovered(share_target); } @@ -52,7 +55,8 @@ TEST(WrappedShareTargetDiscoveredCallbackTest, BlocksDiscoveryForSameVendorId) { TEST(WrappedShareTargetDiscoveredCallbackTest, BlocksUpdatedForSameVendorId) { MockShareTargetDiscoveredCallback callback; ShareTarget share_target = GetShareTarget(/*vendor_id=*/1); - WrappedShareTargetDiscoveredCallback wrapped(&callback, /*vendor_id=*/1); + WrappedShareTargetDiscoveredCallback wrapped(&callback, + BlockedVendorId::kSamsung); EXPECT_CALL(callback, OnShareTargetUpdated(_)).Times(0); wrapped.OnShareTargetUpdated(share_target); } @@ -60,7 +64,8 @@ TEST(WrappedShareTargetDiscoveredCallbackTest, BlocksUpdatedForSameVendorId) { TEST(WrappedShareTargetDiscoveredCallbackTest, BlocksLostForSameVendorId) { MockShareTargetDiscoveredCallback callback; ShareTarget share_target = GetShareTarget(/*vendor_id=*/1); - WrappedShareTargetDiscoveredCallback wrapped(&callback, /*vendor_id=*/1); + WrappedShareTargetDiscoveredCallback wrapped(&callback, + BlockedVendorId::kSamsung); EXPECT_CALL(callback, OnShareTargetLost(_)).Times(0); wrapped.OnShareTargetLost(share_target); } @@ -69,7 +74,8 @@ TEST(WrappedShareTargetDiscoveredCallbackTest, DoesNotBlockDiscoveryForDifferentVendorId) { MockShareTargetDiscoveredCallback callback; ShareTarget share_target = GetShareTarget(/*vendor_id=*/0); - WrappedShareTargetDiscoveredCallback wrapped(&callback, /*vendor_id=*/1); + WrappedShareTargetDiscoveredCallback wrapped(&callback, + BlockedVendorId::kSamsung); EXPECT_CALL(callback, OnShareTargetLost(_)); wrapped.OnShareTargetLost(share_target); } @@ -78,7 +84,8 @@ TEST(WrappedShareTargetDiscoveredCallbackTest, DoesNotBlockUpdatedForDifferentVendorId) { MockShareTargetDiscoveredCallback callback; ShareTarget share_target = GetShareTarget(/*vendor_id=*/0); - WrappedShareTargetDiscoveredCallback wrapped(&callback, /*vendor_id=*/1); + WrappedShareTargetDiscoveredCallback wrapped(&callback, + BlockedVendorId::kSamsung); EXPECT_CALL(callback, OnShareTargetLost(_)); wrapped.OnShareTargetLost(share_target); } @@ -87,7 +94,8 @@ TEST(WrappedShareTargetDiscoveredCallbackTest, DoesNotBlockLostForDifferentVendorId) { MockShareTargetDiscoveredCallback callback; ShareTarget share_target = GetShareTarget(/*vendor_id=*/0); - WrappedShareTargetDiscoveredCallback wrapped(&callback, /*vendor_id=*/1); + WrappedShareTargetDiscoveredCallback wrapped(&callback, + BlockedVendorId::kSamsung); EXPECT_CALL(callback, OnShareTargetLost(_)); wrapped.OnShareTargetLost(share_target); }