diff --git a/connections/implementation/BUILD b/connections/implementation/BUILD index 8ddec3bf..2c0e503d 100644 --- a/connections/implementation/BUILD +++ b/connections/implementation/BUILD @@ -144,6 +144,8 @@ cc_library( "@com_google_absl//absl/functional:any_invocable", "@com_google_absl//absl/functional:bind_front", "@com_google_absl//absl/log:check", + "@com_google_absl//absl/status", + "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/time", @@ -255,6 +257,7 @@ cc_test( "@com_github_protobuf_matchers//protobuf-matchers", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/synchronization", diff --git a/connections/implementation/ble_advertisement.cc b/connections/implementation/ble_advertisement.cc index 08861b4d..83f7fea6 100644 --- a/connections/implementation/ble_advertisement.cc +++ b/connections/implementation/ble_advertisement.cc @@ -16,9 +16,15 @@ #include +#include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/strings/escaping.h" +#include "absl/strings/str_cat.h" #include "connections/implementation/base_pcp_handler.h" +#include "connections/implementation/pcp.h" #include "internal/platform/base_input_stream.h" +#include "internal/platform/bluetooth_utils.h" +#include "internal/platform/byte_array.h" #include "internal/platform/logging.h" namespace nearby { @@ -87,26 +93,22 @@ void BleAdvertisement::DoInitialize(bool fast_advertisement, Version version, } } -BleAdvertisement::BleAdvertisement(bool fast_advertisement, - const ByteArray& ble_advertisement_bytes) { - fast_advertisement_ = fast_advertisement; - +absl::StatusOr BleAdvertisement::CreateBleAdvertisement( + bool fast_advertisement, const ByteArray& ble_advertisement_bytes) { if (ble_advertisement_bytes.Empty()) { - NEARBY_LOG(ERROR, - "Cannot deserialize BleAdvertisement: null bytes passed in."); - return; + return absl::InvalidArgumentError( + "Cannot deserialize BleAdvertisement: null bytes passed in."); } - int min_advertisement_length = fast_advertisement_ + int min_advertisement_length = fast_advertisement ? kMinFastAdvertisementLength : kMinAdvertisementLength; if (ble_advertisement_bytes.size() < min_advertisement_length) { - NEARBY_LOG(ERROR, - "Cannot deserialize BleAdvertisement: expecting min %d raw " - "bytes, got %" PRIu64, - kMinAdvertisementLength, ble_advertisement_bytes.size()); - return; + return absl::InvalidArgumentError( + absl::StrCat("Cannot deserialize BleAdvertisement: expecting min ", + min_advertisement_length, " raw bytes, got ", + ble_advertisement_bytes.size())); } ByteArray advertisement_bytes{ble_advertisement_bytes}; @@ -114,98 +116,105 @@ BleAdvertisement::BleAdvertisement(bool fast_advertisement, // The first 1 byte is supposed to be the version and pcp. auto version_and_pcp_byte = static_cast(base_input_stream.ReadUint8()); // The upper 3 bits are supposed to be the version. - version_ = + Version version = static_cast((version_and_pcp_byte & kVersionBitmask) >> 5); - if (version_ != Version::kV1) { - NEARBY_LOG(INFO, - "Cannot deserialize BleAdvertisement: unsupported Version %d", - version_); - return; + if (version != Version::kV1) { + return absl::InvalidArgumentError(absl::StrCat( + "Cannot deserialize BleAdvertisement: unsupported Version: ", version)); } + // The lower 5 bits are supposed to be the Pcp. - pcp_ = static_cast(version_and_pcp_byte & kPcpBitmask); - switch (pcp_) { + Pcp pcp = static_cast(version_and_pcp_byte & kPcpBitmask); + switch (pcp) { case Pcp::kP2pCluster: // Fall through case Pcp::kP2pStar: // Fall through case Pcp::kP2pPointToPoint: break; default: - NEARBY_LOG(INFO, - "Cannot deserialize BleAdvertisement: unsupported V1 PCP %d", - pcp_); + return absl::InvalidArgumentError(absl::StrCat( + "Cannot deserialize BleAdvertisement: unsupported V1 PCP ", pcp)); } // The next 3 bytes are supposed to be the service_id_hash if not fast // advertisement. - if (!fast_advertisement_) - service_id_hash_ = base_input_stream.ReadBytes(kServiceIdHashLength); + ByteArray service_id_hash; + if (!fast_advertisement) { + service_id_hash = base_input_stream.ReadBytes(kServiceIdHashLength); + } // The next 4 bytes are supposed to be the endpoint_id. - endpoint_id_ = std::string{base_input_stream.ReadBytes(kEndpointIdLength)}; + std::string endpoint_id = + std::string{base_input_stream.ReadBytes(kEndpointIdLength)}; // The next 1 byte is supposed to be the length of the endpoint_info. - std::uint32_t expected_endpoint_info_length = base_input_stream.ReadUint8(); + auto expected_endpoint_info_length = base_input_stream.ReadUint8(); // The next x bytes are the endpoint info. (Max length is 131 bytes or 17 // bytes as fast_advertisement being true). - endpoint_info_ = base_input_stream.ReadBytes(expected_endpoint_info_length); + auto endpoint_info = + base_input_stream.ReadBytes(expected_endpoint_info_length); const int max_endpoint_info_length = - fast_advertisement_ ? kMaxFastEndpointInfoLength : kMaxEndpointInfoLength; - if (endpoint_info_.Empty() || - endpoint_info_.size() != expected_endpoint_info_length || - endpoint_info_.size() > max_endpoint_info_length) { - NEARBY_LOG(INFO, - "Cannot deserialize BleAdvertisement(fast advertisement=%d): " - "expected endpointInfo to be %d bytes, got %" PRIu64, - fast_advertisement_, expected_endpoint_info_length, - endpoint_info_.size()); - - // Clear endpoint_id for validity. - endpoint_id_.clear(); - return; + fast_advertisement ? kMaxFastEndpointInfoLength : kMaxEndpointInfoLength; + if (endpoint_info.Empty() || + endpoint_info.size() != expected_endpoint_info_length || + endpoint_info.size() > max_endpoint_info_length) { + return absl::InvalidArgumentError(absl::StrCat( + "Cannot deserialize BleAdvertisement(fast advertisement=", + fast_advertisement, "): expected endpointInfo to be ", + expected_endpoint_info_length, " bytes, got ", endpoint_info.size())); } // The next 6 bytes are the bluetooth mac address if not fast advertisement. - if (!fast_advertisement_) { + std::string bluetooth_mac_address; + if (!fast_advertisement) { auto bluetooth_mac_address_bytes = base_input_stream.ReadBytes(BluetoothUtils::kBluetoothMacAddressLength); - bluetooth_mac_address_ = + bluetooth_mac_address = BluetoothUtils::ToString(bluetooth_mac_address_bytes); } // The next 1 byte is supposed to be the length of the uwb_address. If the // next byte is not available then it should be a fast advertisement and skip // it for remaining bytes. + ByteArray uwb_address; + BleAdvertisement ble_advertisement; if (base_input_stream.IsAvailable(1)) { - std::uint32_t expected_uwb_address_length = base_input_stream.ReadUint8(); + auto expected_uwb_address_length = base_input_stream.ReadUint8(); // If the length of uwb_address is not zero, then retrieve it. if (expected_uwb_address_length != 0) { - uwb_address_ = base_input_stream.ReadBytes(expected_uwb_address_length); - if (uwb_address_.Empty() || - uwb_address_.size() != expected_uwb_address_length) { - NEARBY_LOG(INFO, - "Cannot deserialize BleAdvertisement: " - "expected uwbAddress size to be %d bytes, got %" PRIu64, - expected_uwb_address_length, uwb_address_.size()); - - // Clear endpoint_id for validity. - endpoint_id_.clear(); - return; + uwb_address = base_input_stream.ReadBytes(expected_uwb_address_length); + if (uwb_address.Empty() || + uwb_address.size() != expected_uwb_address_length) { + return absl::InvalidArgumentError(absl::StrCat( + "Cannot deserialize BleAdvertisement: expected uwbAddress size to " + "be ", + expected_uwb_address_length, " bytes, got ", uwb_address.size())); } } // The next 1 byte is extra field. - if (!fast_advertisement_) { + if (!fast_advertisement) { if (base_input_stream.IsAvailable(kExtraFieldLength)) { auto extra_field = static_cast(base_input_stream.ReadUint8()); - web_rtc_state_ = (extra_field & kWebRtcConnectableFlagBitmask) == 1 - ? WebRtcState::kConnectable - : WebRtcState::kUnconnectable; + ble_advertisement.web_rtc_state_ = + (extra_field & kWebRtcConnectableFlagBitmask) == 1 + ? WebRtcState::kConnectable + : WebRtcState::kUnconnectable; } } } base_input_stream.Close(); + + ble_advertisement.fast_advertisement_ = fast_advertisement; + ble_advertisement.version_ = version; + ble_advertisement.pcp_ = pcp; + ble_advertisement.service_id_hash_ = service_id_hash; + ble_advertisement.endpoint_id_ = endpoint_id; + ble_advertisement.endpoint_info_ = endpoint_info; + ble_advertisement.bluetooth_mac_address_ = bluetooth_mac_address; + ble_advertisement.uwb_address_ = uwb_address; + return ble_advertisement; } BleAdvertisement::operator ByteArray() const { diff --git a/connections/implementation/ble_advertisement.h b/connections/implementation/ble_advertisement.h index 6ba4cd5b..d50ebf94 100644 --- a/connections/implementation/ble_advertisement.h +++ b/connections/implementation/ble_advertisement.h @@ -15,6 +15,7 @@ #ifndef CORE_INTERNAL_BLE_ADVERTISEMENT_H_ #define CORE_INTERNAL_BLE_ADVERTISEMENT_H_ +#include "absl/status/statusor.h" #include "connections/implementation/base_pcp_handler.h" #include "connections/implementation/pcp.h" #include "internal/platform/bluetooth_utils.h" @@ -77,8 +78,8 @@ class BleAdvertisement { const ByteArray& endpoint_info, const std::string& bluetooth_mac_address, const ByteArray& uwb_address, WebRtcState web_rtc_state); - BleAdvertisement(bool fast_advertisement, - const ByteArray& ble_advertisement_bytes); + static absl::StatusOr CreateBleAdvertisement( + bool fast_advertisement, const ByteArray& ble_advertisement_bytes); BleAdvertisement(const BleAdvertisement&) = default; BleAdvertisement& operator=(const BleAdvertisement&) = default; BleAdvertisement(BleAdvertisement&&) = default; diff --git a/connections/implementation/ble_advertisement_test.cc b/connections/implementation/ble_advertisement_test.cc index d73df08c..db6a021c 100644 --- a/connections/implementation/ble_advertisement_test.cc +++ b/connections/implementation/ble_advertisement_test.cc @@ -14,8 +14,12 @@ #include "connections/implementation/ble_advertisement.h" +#include "gmock/gmock.h" +#include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" +#include "absl/status/status.h" #include "connections/implementation/base_pcp_handler.h" +#include "internal/platform/byte_array.h" namespace nearby { namespace connections { @@ -31,6 +35,9 @@ constexpr absl::string_view kFastAdvertisementEndpointName{"Fast Advertise"}; constexpr absl::string_view kBluetoothMacAddress{"00:00:E6:88:64:13"}; constexpr WebRtcState kWebRtcState = WebRtcState::kConnectable; +using ::absl::StatusCode::kInvalidArgument; +using ::testing::status::StatusIs; + // TODO(b/169550050): Implement UWBAddress. TEST(BleAdvertisementTest, ConstructionWorks) { ByteArray service_id_hash{std::string(kServiceIdHashBytes)}; @@ -269,7 +276,10 @@ TEST(BleAdvertisementTest, ConstructionFromBytesWorks) { ByteArray{}, kWebRtcState}; ByteArray ble_advertisement_bytes(org_ble_advertisement); - BleAdvertisement ble_advertisement{false, ble_advertisement_bytes}; + auto ble_status_or = + BleAdvertisement::CreateBleAdvertisement(false, ble_advertisement_bytes); + ASSERT_OK(ble_status_or.status()); + auto ble_advertisement = ble_status_or.value(); EXPECT_TRUE(ble_advertisement.IsValid()); EXPECT_FALSE(ble_advertisement.IsFastAdvertisement()); @@ -290,7 +300,10 @@ TEST(BleAdvertisementTest, ConstructionFromBytesWorksForFastAdvertisement) { fast_endpoint_info, ByteArray{}}; ByteArray ble_advertisement_bytes(org_ble_advertisement); - BleAdvertisement ble_advertisement{true, ble_advertisement_bytes}; + auto ble_status_or = + BleAdvertisement::CreateBleAdvertisement(true, ble_advertisement_bytes); + ASSERT_OK(ble_status_or.status()); + auto ble_advertisement = ble_status_or.value(); EXPECT_TRUE(ble_advertisement.IsValid()); EXPECT_TRUE(ble_advertisement.IsFastAdvertisement()); @@ -322,7 +335,10 @@ TEST(BleAdvertisementTest, ConstructionFromLongLengthBytesWorks) { memcpy(long_ble_advertisement_bytes.data(), ble_advertisement_bytes.data(), ble_advertisement_bytes.size()); - BleAdvertisement long_ble_advertisement{false, long_ble_advertisement_bytes}; + auto ble_status_or = BleAdvertisement::CreateBleAdvertisement( + false, long_ble_advertisement_bytes); + ASSERT_OK(ble_status_or.status()); + auto long_ble_advertisement = ble_status_or.value(); EXPECT_TRUE(long_ble_advertisement.IsValid()); EXPECT_EQ(kVersion, long_ble_advertisement.GetVersion()); @@ -336,15 +352,13 @@ TEST(BleAdvertisementTest, ConstructionFromLongLengthBytesWorks) { } TEST(BleAdvertisementTest, ConstructionFromNullBytesFails) { - BleAdvertisement ble_advertisement{false, ByteArray{}}; - - EXPECT_FALSE(ble_advertisement.IsValid()); + EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement(false, ByteArray()), + StatusIs(kInvalidArgument)); } TEST(BleAdvertisementTest, ConstructionFromNullBytesFailsForFastAdvertisement) { - BleAdvertisement ble_advertisement{true, ByteArray{}}; - - EXPECT_FALSE(ble_advertisement.IsValid()); + EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement(true, ByteArray()), + StatusIs(kInvalidArgument)); } TEST(BleAdvertisementTest, ConstructionFromShortLengthBytesFails) { @@ -363,10 +377,9 @@ TEST(BleAdvertisementTest, ConstructionFromShortLengthBytesFails) { ble_advertisement_bytes.data(), BleAdvertisement::kMinAdvertisementLength - 1}; - BleAdvertisement short_ble_advertisement{false, - short_ble_advertisement_bytes}; - - EXPECT_FALSE(short_ble_advertisement.IsValid()); + EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement( + false, short_ble_advertisement_bytes), + StatusIs(kInvalidArgument)); } TEST(BleAdvertisementTest, @@ -382,9 +395,9 @@ TEST(BleAdvertisementTest, ble_advertisement_bytes.data(), BleAdvertisement::kMinAdvertisementLength - 1}; - BleAdvertisement short_ble_advertisement{true, short_ble_advertisement_bytes}; - - EXPECT_FALSE(short_ble_advertisement.IsValid()); + EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement( + true, short_ble_advertisement_bytes), + StatusIs(kInvalidArgument)); } TEST(BleAdvertisementTest, @@ -404,10 +417,9 @@ TEST(BleAdvertisementTest, corrupt_ble_advertisement_string[8] ^= 0x0FF; ByteArray corrupt_ble_advertisement_bytes(corrupt_ble_advertisement_string); - BleAdvertisement corrupt_ble_advertisement{false, - corrupt_ble_advertisement_bytes}; - - EXPECT_FALSE(corrupt_ble_advertisement.IsValid()); + EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement( + false, corrupt_ble_advertisement_bytes), + StatusIs(kInvalidArgument)); } TEST(BleAdvertisementTest, @@ -423,10 +435,9 @@ TEST(BleAdvertisementTest, corrupt_ble_advertisement_string[5] ^= 0x0FF; ByteArray corrupt_ble_advertisement_bytes(corrupt_ble_advertisement_string); - BleAdvertisement corrupt_ble_advertisement{true, - corrupt_ble_advertisement_bytes}; - - EXPECT_FALSE(corrupt_ble_advertisement.IsValid()); + EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement( + true, corrupt_ble_advertisement_bytes), + StatusIs(kInvalidArgument)); } } // namespace diff --git a/connections/implementation/p2p_cluster_pcp_handler.cc b/connections/implementation/p2p_cluster_pcp_handler.cc index eefeed08..f437b2b1 100644 --- a/connections/implementation/p2p_cluster_pcp_handler.cc +++ b/connections/implementation/p2p_cluster_pcp_handler.cc @@ -441,19 +441,6 @@ void P2pClusterPcpHandler::BluetoothDeviceLostHandler( bool P2pClusterPcpHandler::IsRecognizedBleEndpoint( const std::string& service_id, const BleAdvertisement& advertisement) const { - if (!advertisement.IsValid()) { - NEARBY_LOGS(INFO) - << "BleAdvertisement doesn't conform to the format, discarding."; - return false; - } - - if (advertisement.GetVersion() != kBleAdvertisementVersion) { - NEARBY_LOGS(INFO) << "BleAdvertisement has an unknown version; expected " - << static_cast(kBleAdvertisementVersion) - << ", found " - << static_cast(advertisement.GetVersion()); - return false; - } if (advertisement.GetPcp() != GetPcp()) { NEARBY_LOGS(INFO) << "BleAdvertisement doesn't match on Pcp; expected " @@ -499,8 +486,13 @@ void P2pClusterPcpHandler::BlePeripheralDiscoveredHandler( return; } - // Parse the BLE advertisement bytes. - BleAdvertisement advertisement(fast_advertisement, advertisement_bytes); + auto ble_status_or = BleAdvertisement::CreateBleAdvertisement( + fast_advertisement, advertisement_bytes); + if (!ble_status_or.ok()) { + NEARBY_LOGS(ERROR) << ble_status_or.status().ToString(); + return; + } + auto advertisement = ble_status_or.value(); // Make sure the BLE advertisement points to a valid // endpoint we're discovering. @@ -671,8 +663,13 @@ void P2pClusterPcpHandler::BleV2PeripheralDiscoveredHandler( return; } - // Parse the BLE advertisement bytes. - BleAdvertisement advertisement(fast_advertisement, advertisement_bytes); + auto ble_status_or = BleAdvertisement::CreateBleAdvertisement( + fast_advertisement, advertisement_bytes); + if (!ble_status_or.ok()) { + NEARBY_LOGS(ERROR) << ble_status_or.status(); + return; + } + auto advertisement = ble_status_or.value(); // Make sure the BLE advertisement points to a valid // endpoint we're discovering. @@ -761,8 +758,13 @@ void P2pClusterPcpHandler::BleV2PeripheralLostHandler( return; } - // Parse the BLE advertisement bytes. - BleAdvertisement advertisement(fast_advertisement, advertisement_bytes); + auto ble_status_or = BleAdvertisement::CreateBleAdvertisement( + fast_advertisement, advertisement_bytes); + if (!ble_status_or.ok()) { + NEARBY_LOGS(ERROR) << ble_status_or.status(); + return; + } + auto advertisement = ble_status_or.value(); // Make sure the BLE advertisement points to a valid // endpoint we're discovering.