Skip to content

Commit

Permalink
Ensure that DataModel::Encode does not encode kUnknownEnumValue for e…
Browse files Browse the repository at this point in the history
…nums. (#26299)

This should help prevent those values leaking out on the wire.

Fixes #24368
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Sep 22, 2023
1 parent 03911d1 commit 5302441
Show file tree
Hide file tree
Showing 12 changed files with 118 additions and 33 deletions.
5 changes: 4 additions & 1 deletion examples/chip-tool/include/CHIPProjectAppConfig.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* Copyright (c) 2020-2022 Project CHIP Authors
* Copyright (c) 2020-2023 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -63,4 +63,7 @@
// Enable some test-only interaction model APIs.
#define CONFIG_BUILD_FOR_HOST_UNIT_TEST 1

// Allow us, for test purposes, to encode invalid enum values.
#define CHIP_CONFIG_IM_ENABLE_ENCODING_SENTINEL_ENUM_VALUES 1

#endif /* CHIPPROJECTCONFIG_H */
6 changes: 6 additions & 0 deletions examples/darwin-framework-tool/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ action("build-darwin-framework") {
args += [ "--no-clang" ]
}

if (config_enable_yaml_tests) {
args += [ "--enable_encoding_sentinel_enum_values" ]
} else {
args += [ "--no-enable_encoding_sentinel_enum_values" ]
}

output_name = "Matter.framework"
outputs = [
"${root_out_dir}/macos_framework_output/Build/Products/${output_sdk_type}/${output_name}",
Expand Down
11 changes: 11 additions & 0 deletions examples/darwin-framework-tool/args.gni
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,15 @@ import("//build_overrides/chip.gni")

import("${chip_root}/config/standalone/args.gni")

import("${chip_root}/examples/chip-tool/chip-tool.gni")

chip_crypto = "boringssl"

if (config_enable_yaml_tests) {
chip_device_project_config_include = "<CHIPProjectAppConfig.h>"
chip_project_config_include = "<CHIPProjectAppConfig.h>"

chip_project_config_include_dirs =
[ "${chip_root}/examples/darwin-framework-tool/include" ]
chip_project_config_include_dirs += [ "${chip_root}/config/standalone" ]
}
31 changes: 31 additions & 0 deletions examples/darwin-framework-tool/include/CHIPProjectAppConfig.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright (c) 2023 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* @file
* Project configuration for Darwin Framework Tool.
*
*/
#ifndef CHIPPROJECTCONFIG_H
#define CHIPPROJECTCONFIG_H

// Enable some test-only interaction model APIs.
#define CONFIG_BUILD_FOR_HOST_UNIT_TEST 1

// Allow us, for test purposes, to encode invalid enum values.
#define CHIP_CONFIG_IM_ENABLE_ENCODING_SENTINEL_ENUM_VALUES 1

#endif /* CHIPPROJECTCONFIG_H */
5 changes: 5 additions & 0 deletions scripts/build/build_darwin_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def build_darwin_framework(args):
'CHIP_IS_ASAN': args.asan,
'CHIP_IS_BLE': args.ble,
'CHIP_IS_CLANG': args.clang,
'CHIP_ENABLE_ENCODING_SENTINEL_ENUM_VALUES': args.enable_encoding_sentinel_enum_values
}
for option in options:
command += ["{}={}".format(option, "YES" if options[option] else "NO")]
Expand Down Expand Up @@ -116,6 +117,9 @@ def build_darwin_framework(args):
get_file_from_pigweed("libclang_rt.asan_osx_dynamic.dylib")
]

if args.enable_encoding_sentinel_enum_values:
cflags += ["-DCHIP_CONFIG_IM_ENABLE_ENCODING_SENTINEL_ENUM_VALUES=1"]

command += ["OTHER_CFLAGS=" + ' '.join(cflags), "OTHER_LDFLAGS=" + ' '.join(ldflags)]
command_result = run_command(command)
print("Build Framework Result: {}".format(command_result))
Expand Down Expand Up @@ -158,6 +162,7 @@ def build_darwin_framework(args):
parser.add_argument('--asan', action=argparse.BooleanOptionalAction)
parser.add_argument('--ble', action=argparse.BooleanOptionalAction)
parser.add_argument('--clang', action=argparse.BooleanOptionalAction)
parser.add_argument('--enable_encoding_sentinel_enum_values', action=argparse.BooleanOptionalAction)

args = parser.parse_args()
build_darwin_framework(args)
27 changes: 26 additions & 1 deletion src/app/data-model/Encode.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,18 @@ namespace chip {
namespace app {
namespace DataModel {

namespace detail {
// A way to detect whether an enum has a kUnknownEnumValue value, for use in enable_if.
template <typename Enum, Enum value>
using VoidType = void;

template <typename, typename = void>
constexpr bool HasUnknownValue = false;

template <typename T>
constexpr bool HasUnknownValue<T, VoidType<T, T::kUnknownEnumValue>> = true;
} // namespace detail

/*
* @brief
* Set of overloaded encode methods that based on the type of cluster element passed in,
Expand All @@ -48,9 +60,22 @@ CHIP_ERROR Encode(TLV::TLVWriter & writer, TLV::Tag tag, X x)
return writer.Put(tag, x);
}

template <typename X, typename std::enable_if_t<std::is_enum<X>::value, int> = 0>
template <typename X, typename std::enable_if_t<std::is_enum<X>::value && !detail::HasUnknownValue<X>, int> = 0>
CHIP_ERROR Encode(TLV::TLVWriter & writer, TLV::Tag tag, X x)
{
return writer.Put(tag, x);
}

template <typename X, typename std::enable_if_t<std::is_enum<X>::value && detail::HasUnknownValue<X>, int> = 0>
CHIP_ERROR Encode(TLV::TLVWriter & writer, TLV::Tag tag, X x)
{
#if !CHIP_CONFIG_IM_ENABLE_ENCODING_SENTINEL_ENUM_VALUES
if (x == X::kUnknownEnumValue)
{
return CHIP_IM_GLOBAL_STATUS(ConstraintError);
}
#endif // !CHIP_CONFIG_IM_ENABLE_ENCODING_SENTINEL_ENUM_VALUES

return writer.Put(tag, x);
}

Expand Down
7 changes: 2 additions & 5 deletions src/app/tests/suites/TestCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1068,11 +1068,8 @@ tests:
- name: "arg2"
value: 101
response:
values:
- name: "arg1"
value: 20003
- name: "arg2"
value: 4
# Attempting to echo back the invalid enum value should fail.
error: FAILURE

# Tests for Struct

Expand Down
6 changes: 3 additions & 3 deletions src/app/tests/suites/TestEvents.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ tests:
- name: "arg1"
value: 3
- name: "arg2"
value: 4
value: 1
- name: "arg3"
value: false
response:
Expand All @@ -95,7 +95,7 @@ tests:
- values:
- value: { arg1: 1, arg2: 2, arg3: true }
- values:
- value: { arg1: 3, arg2: 4, arg3: false }
- value: { arg1: 3, arg2: 1, arg3: false }

- label: "Subscribe to the event"
command: "subscribeEvent"
Expand All @@ -106,7 +106,7 @@ tests:
- values:
- value: { arg1: 1, arg2: 2, arg3: true }
- values:
- value: { arg1: 3, arg2: 4, arg3: false }
- value: { arg1: 3, arg2: 1, arg3: false }

- label: "Generate a third event on the accessory"
command: "TestEmitTestEventRequest"
Expand Down
9 changes: 9 additions & 0 deletions src/darwin/Framework/chip_xcode_build_connector.sh
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ for define in "${defines[@]}"; do
esac
target_defines+=,\"${define//\"/\\\"}\"
done
[[ $CHIP_ENABLE_ENCODING_SENTINEL_ENUM_VALUES == YES ]] && {
target_defines+=,\"CHIP_CONFIG_IM_ENABLE_ENCODING_SENTINEL_ENUM_VALUES=1\"
}
target_defines=[${target_defines:1}]

declare target_arch=
Expand Down Expand Up @@ -142,6 +145,12 @@ declare -a args=(
)
}

[[ $CHIP_ENABLE_ENCODING_SENTINEL_ENUM_VALUES == YES ]] && {
args+=(
'enable_encoding_sentinel_enum_values=true'
)
}

# search current (or $2) and its parent directories until
# a name match is found, which is output on stdout
find_in_ancestors() {
Expand Down
11 changes: 11 additions & 0 deletions src/lib/core/CHIPConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,17 @@ extern const char CHIP_NON_PRODUCTION_MARKER[];
#define CONFIG_BUILD_FOR_HOST_UNIT_TEST 0
#endif

/**
* @def CHIP_CONFIG_IM_ENABLE_ENCODING_SENTINEL_ENUM_VALUES
*
* @brief Defines whether encoding the "not a known enum value" enum values is
* allowed. Should only be enabled in certain test applications. This
* flag must not be enabled on actual devices.
*/
#ifndef CHIP_CONFIG_IM_ENABLE_ENCODING_SENTINEL_ENUM_VALUES
#define CHIP_CONFIG_IM_ENABLE_ENCODING_SENTINEL_ENUM_VALUES 0
#endif

/**
* @def CHIP_CONFIG_LAMBDA_EVENT_SIZE
*
Expand Down
14 changes: 4 additions & 10 deletions zzz_generated/chip-tool/zap-generated/test/Commands.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 6 additions & 13 deletions zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 5302441

Please sign in to comment.