From 68cb8c04e83036df26612e444b0801b8c20d3e0f Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Wed, 20 Oct 2021 14:01:42 -0700 Subject: [PATCH] Parse user flags as flags instead of enum. The flags field in UserInfo is a int32_t that contains multiple UserFlags 'or'ed together. We should not parse it as enum. Test: atest android.hardware.automotive.vehicle@2.0-utils-unit-tests Bug: 202520478 Change-Id: Ie7e81a8a5f39f6070e35f2e77bce88a211fd526b Merged-In: Ie7e81a8a5f39f6070e35f2e77bce88a211fd526b (cherry picked from commit 5c0ec3f1debcf5c395609c951cf748315b52219a) --- .../vehicle/2.0/utils/UserHalHelper.cpp | 19 ++++-- .../2.0/utils/tests/UserHalHelper_test.cpp | 58 ++++++++++++++++++- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/automotive/vehicle/2.0/utils/UserHalHelper.cpp b/automotive/vehicle/2.0/utils/UserHalHelper.cpp index abf59b74e3..dccdb2b845 100644 --- a/automotive/vehicle/2.0/utils/UserHalHelper.cpp +++ b/automotive/vehicle/2.0/utils/UserHalHelper.cpp @@ -60,11 +60,22 @@ Result parseUserInfo(const hidl_vec& int32Values, size_t startPos << int32Values.size(); } userInfo->userId = int32Values[startPos]; - auto userFlags = verifyAndCast(int32Values[startPos + 1]); - if (!userFlags.ok()) { - return Error() << "Invalid user flags: " << userFlags.error(); + int32_t intUserFlags = int32Values[startPos + 1]; + int32_t expectedUserFlags = 0; + for (const auto& v : hidl_enum_range()) { + int32_t intEnumUserFlag = static_cast(v); + if ((intUserFlags & intEnumUserFlag) != 0) { + expectedUserFlags |= intEnumUserFlag; + } } - userInfo->flags = *userFlags; + if (intUserFlags != expectedUserFlags) { + return Error() << "Invalid user flags: " << intUserFlags << ", must be '|' of UserFlags"; + } + // intUserFlags is actually not a valid UserFlags enum, instead, it is a 'bit or' of possible + // multiple UserFlags. However, because the HAL interface was defined incorrectly, we have to + // cast it to UserFlags here, which is defined behavior because the underlying type for + // UserFlags is int32_t and our intUserFlags is within the range of int32_t. + userInfo->flags = static_cast(intUserFlags); return {}; } diff --git a/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp b/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp index 7da87a23c8..0562a54cec 100644 --- a/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp +++ b/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp @@ -54,6 +54,10 @@ constexpr int32_t VEHICLE_REQUEST = static_cast(SwitchUserMessageType:: constexpr int32_t GUEST_USER = static_cast(UserFlags::GUEST); constexpr int32_t NONE_USER = static_cast(UserFlags::NONE); constexpr int32_t SYSTEM_USER = static_cast(UserFlags::SYSTEM); +constexpr int32_t ADMIN_USER = static_cast(UserFlags::ADMIN); +constexpr int32_t SYSTEM_ADMIN_USER = static_cast(UserFlags::SYSTEM | UserFlags::ADMIN); +// 0x1111 is not a valid UserFlags combination. +constexpr int32_t INVALID_USER_FLAG = 0x1111; constexpr int32_t USER_ID_ASSOC_KEY_FOB = static_cast(UserIdentificationAssociationType::KEY_FOB); @@ -72,7 +76,7 @@ constexpr int32_t USER_ID_ASSOC_NO_USER = } // namespace -TEST(UserHalHelperTest, TestToInitialUserInfoRequest) { +TEST(UserHalHelperTest, TestToInitialUserInfoRequestSystemUser) { VehiclePropValue propValue{ .prop = INITIAL_USER_INFO, .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, SYSTEM_USER, @@ -92,6 +96,58 @@ TEST(UserHalHelperTest, TestToInitialUserInfoRequest) { EXPECT_THAT(actual.value(), Eq(expected)); } +TEST(UserHalHelperTest, TestToInitialUserInfoRequestAdminUser) { + VehiclePropValue propValue{ + .prop = INITIAL_USER_INFO, + .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, ADMIN_USER, 10, + NONE_USER}}, + }; + InitialUserInfoRequest expected{ + .requestId = 23, + .requestType = InitialUserInfoRequestType::FIRST_BOOT_AFTER_OTA, + .usersInfo = {{10, UserFlags::NONE}, 2, {{0, UserFlags::ADMIN}, {10, UserFlags::NONE}}}, + }; + + auto actual = toInitialUserInfoRequest(propValue); + + ASSERT_TRUE(actual.ok()) << actual.error().message(); + EXPECT_THAT(actual.value(), Eq(expected)); +} + +TEST(UserHalHelperTest, TestToInitialUserInfoRequestUserFlagsBitCombination) { + // SYSTEM_ADMIN_USER is two UserFlags combined and is itself not a defined UserFlags enum. + VehiclePropValue propValue{ + .prop = INITIAL_USER_INFO, + .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, + SYSTEM_ADMIN_USER, 10, NONE_USER}}, + }; + InitialUserInfoRequest expected{ + .requestId = 23, + .requestType = InitialUserInfoRequestType::FIRST_BOOT_AFTER_OTA, + .usersInfo = {{10, UserFlags::NONE}, + 2, + {{0, static_cast(SYSTEM_ADMIN_USER)}, {10, UserFlags::NONE}}}, + }; + + auto actual = toInitialUserInfoRequest(propValue); + + ASSERT_TRUE(actual.ok()) << actual.error().message(); + EXPECT_THAT(actual.value(), Eq(expected)); +} + +TEST(UserHalHelperTest, TestToInitialUserInfoRequestUserInvalidUserFlag) { + // 0x1111 is not a valid UserFlags flag combination. + VehiclePropValue propValue{ + .prop = INITIAL_USER_INFO, + .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, + INVALID_USER_FLAG, 10, NONE_USER}}, + }; + + auto actual = toInitialUserInfoRequest(propValue); + + EXPECT_FALSE(actual.ok()) << "No error returned on invalid user flags"; +} + TEST(UserHalHelperTest, TestFailsToInitialUserInfoRequestWithMismatchingPropType) { VehiclePropValue propValue{ .prop = INT32_MAX,