Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5c0ec3f1 authored by Yu Shan's avatar Yu Shan
Browse files

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
parent c69d81e5

automotive/vehicle/2.0/utils/UserHalHelper.cpp

+15 −4
Original line number Diff line number Diff line
@@ -60,11 +60,22 @@ Result<void> parseUserInfo(const hidl_vec<int32_t>& int32Values, size_t startPos 
                       << int32Values.size();

    }

    userInfo->userId = int32Values[startPos];

    auto userFlags = verifyAndCast<UserFlags>(int32Values[startPos + 1]);

    if (!userFlags.ok()) {

        return Error() << "Invalid user flags: " << userFlags.error();

    }

    userInfo->flags = *userFlags;

    int32_t intUserFlags = int32Values[startPos + 1];

    int32_t expectedUserFlags = 0;

    for (const auto& v : hidl_enum_range<UserFlags>()) {

        int32_t intEnumUserFlag = static_cast<int32_t>(v);

        if ((intUserFlags & intEnumUserFlag) != 0) {

            expectedUserFlags |= intEnumUserFlag;

        }

    }

    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<UserFlags>(intUserFlags);

    return {};

}

automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp

+57 −1
Original line number Diff line number Diff line
@@ -54,6 +54,10 @@ constexpr int32_t VEHICLE_REQUEST = static_cast<int32_t>(SwitchUserMessageType:: 
constexpr int32_t GUEST_USER = static_cast<int32_t>(UserFlags::GUEST);

constexpr int32_t NONE_USER = static_cast<int32_t>(UserFlags::NONE);

constexpr int32_t SYSTEM_USER = static_cast<int32_t>(UserFlags::SYSTEM);

constexpr int32_t ADMIN_USER = static_cast<int32_t>(UserFlags::ADMIN);

constexpr int32_t SYSTEM_ADMIN_USER = static_cast<int32_t>(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<int32_t>(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<UserFlags>(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,