Skip to content

Commit 32679dd

Browse files
lpbeliveau-silabslpbeliveau-silabs
committed
Added check on maximal attribute value when executing add scenes, along with unit tests and refactor of AttributeValuePair->Value to uint64
1 parent 8d623ff commit 32679dd

File tree

23 files changed

+104
-28
lines changed

23 files changed

+104
-28
lines changed

‎docs/ERROR_CODES.md

+1
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ This file was **AUTOMATICALLY** generated by
8989
| 92 | 0x5C | `CHIP_ERROR_WRONG_NODE_ID` |
9090
| 100 | 0x64 | `CHIP_ERROR_RETRANS_TABLE_FULL` |
9191
| 104 | 0x68 | `CHIP_ERROR_TRANSACTION_CANCELED` |
92+
| 106 | 0x6A | `CHIP_ERROR_UNSUPPORTED_ATTRIBUTE` |
9293
| 107 | 0x6B | `CHIP_ERROR_INVALID_SUBSCRIPTION` |
9394
| 108 | 0x6C | `CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE` |
9495
| 112 | 0x70 | `CHIP_ERROR_UNSOLICITED_MSG_NO_ORIGINATOR` |

‎examples/all-clusters-app/all-clusters-common/all-clusters-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -3514,7 +3514,7 @@ provisional cluster ScenesManagement = 98 {
35143514

35153515
struct AttributeValuePair {
35163516
attrib_id attributeID = 0;
3517-
int32u attributeValue = 1;
3517+
int64u attributeValue = 1;
35183518
}
35193519

35203520
struct ExtensionFieldSet {

‎examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -2426,7 +2426,7 @@ provisional cluster ScenesManagement = 98 {
24262426

24272427
struct AttributeValuePair {
24282428
attrib_id attributeID = 0;
2429-
int32u attributeValue = 1;
2429+
int64u attributeValue = 1;
24302430
}
24312431

24322432
struct ExtensionFieldSet {

‎examples/light-switch-app/light-switch-common/light-switch-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -2037,7 +2037,7 @@ provisional cluster ScenesManagement = 98 {
20372037

20382038
struct AttributeValuePair {
20392039
attrib_id attributeID = 0;
2040-
int32u attributeValue = 1;
2040+
int64u attributeValue = 1;
20412041
}
20422042

20432043
struct ExtensionFieldSet {

‎examples/light-switch-app/qpg/zap/switch.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1834,7 +1834,7 @@ provisional cluster ScenesManagement = 98 {
18341834

18351835
struct AttributeValuePair {
18361836
attrib_id attributeID = 0;
1837-
int32u attributeValue = 1;
1837+
int64u attributeValue = 1;
18381838
}
18391839

18401840
struct ExtensionFieldSet {

‎examples/lighting-app/lighting-common/lighting-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1854,7 +1854,7 @@ provisional cluster ScenesManagement = 98 {
18541854

18551855
struct AttributeValuePair {
18561856
attrib_id attributeID = 0;
1857-
int32u attributeValue = 1;
1857+
int64u attributeValue = 1;
18581858
}
18591859

18601860
struct ExtensionFieldSet {

‎examples/lighting-app/silabs/data_model/lighting-thread-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1559,7 +1559,7 @@ provisional cluster ScenesManagement = 98 {
15591559

15601560
struct AttributeValuePair {
15611561
attrib_id attributeID = 0;
1562-
int32u attributeValue = 1;
1562+
int64u attributeValue = 1;
15631563
}
15641564

15651565
struct ExtensionFieldSet {

‎examples/lighting-app/silabs/data_model/lighting-wifi-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1850,7 +1850,7 @@ provisional cluster ScenesManagement = 98 {
18501850

18511851
struct AttributeValuePair {
18521852
attrib_id attributeID = 0;
1853-
int32u attributeValue = 1;
1853+
int64u attributeValue = 1;
18541854
}
18551855

18561856
struct ExtensionFieldSet {

‎examples/placeholder/linux/apps/app1/config.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -3020,7 +3020,7 @@ provisional cluster ScenesManagement = 98 {
30203020

30213021
struct AttributeValuePair {
30223022
attrib_id attributeID = 0;
3023-
int32u attributeValue = 1;
3023+
int64u attributeValue = 1;
30243024
}
30253025

30263026
struct ExtensionFieldSet {

‎examples/placeholder/linux/apps/app2/config.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -2977,7 +2977,7 @@ provisional cluster ScenesManagement = 98 {
29772977

29782978
struct AttributeValuePair {
29792979
attrib_id attributeID = 0;
2980-
int32u attributeValue = 1;
2980+
int64u attributeValue = 1;
29812981
}
29822982

29832983
struct ExtensionFieldSet {

‎examples/thermostat/nxp/zap/thermostat_matter_thread.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1755,7 +1755,7 @@ provisional cluster ScenesManagement = 98 {
17551755

17561756
struct AttributeValuePair {
17571757
attrib_id attributeID = 0;
1758-
int32u attributeValue = 1;
1758+
int64u attributeValue = 1;
17591759
}
17601760

17611761
struct ExtensionFieldSet {

‎examples/thermostat/nxp/zap/thermostat_matter_wifi.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1666,7 +1666,7 @@ provisional cluster ScenesManagement = 98 {
16661666

16671667
struct AttributeValuePair {
16681668
attrib_id attributeID = 0;
1669-
int32u attributeValue = 1;
1669+
int64u attributeValue = 1;
16701670
}
16711671

16721672
struct ExtensionFieldSet {

‎examples/virtual-device-app/virtual-device-common/virtual-device-app.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -1902,7 +1902,7 @@ provisional cluster ScenesManagement = 98 {
19021902

19031903
struct AttributeValuePair {
19041904
attrib_id attributeID = 0;
1905-
int32u attributeValue = 1;
1905+
int64u attributeValue = 1;
19061906
}
19071907

19081908
struct ExtensionFieldSet {

‎src/app/clusters/level-control/level-control.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ class DefaultLevelControlSceneHandler : public scenes::DefaultSceneHandlerImpl
177177
}
178178
else
179179
{
180-
chip::app::NumericAttributeTraits<uint32_t>::SetNull(pairs[0].attributeValue);
180+
chip::app::NumericAttributeTraits<uint64_t>::SetNull(pairs[0].attributeValue);
181181
}
182182
size_t attributeCount = 1;
183183
if (LevelControlHasFeature(endpoint, LevelControl::Feature::kFrequency))

‎src/app/clusters/scenes-server/SceneHandlerImpl.cpp

+34-7
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,49 @@ namespace scenes {
2222

2323
namespace {
2424

25-
/// ValideAttribute
25+
using ConcreteAttributePath = app::ConcreteAttributePath;
26+
using AttributeValuePairType = app::Clusters::ScenesManagement::Structs::AttributeValuePair::Type;
27+
28+
/// CapAttributeID
29+
/// Cap the attribute value based on the attribute type size
30+
/// @param[in] aVPair AttributeValuePairType
31+
/// @param[in] metadata EmberAfAttributeMetadata
32+
///
33+
void CapAttributeID(AttributeValuePairType & aVPair, const EmberAfAttributeMetadata * metadata)
34+
{
35+
// Calculate the maximum value that can be represented with the given number of bytes
36+
uint64_t maxValue = (1ULL << (metadata->size * 8)) - 1;
37+
38+
// If the attribute ID is greater than the maximum value, cap it
39+
if (aVPair.attributeValue > maxValue)
40+
{
41+
aVPair.attributeValue = maxValue;
42+
}
43+
}
44+
2645
/// @brief Validate the attribute exists for a given cluster
46+
/// @param[in] endpoint Endpoint ID
2747
/// @param[in] clusterID Cluster ID
28-
/// @param[in] attID Attribute ID
48+
/// @param[in] aVPair AttributeValuePairType, will be mutated to cap the value if it is out of range
2949
/// @return CHIP_ERROR_UNSUPPORTED_ATTRIBUTE if the attribute does not exist for a given cluster or is not scenable
3050
/// @note This will allways fail for global list attributes. If we do want to make them scenable someday, we will need to
3151
/// use a different validation method.
32-
// TODO: Assess if we also want to throw an error if the attribute value is out of range
52+
// TODO: Assess if (and how) we also want to throw an error if the attribute value is out of range
3353
// TODO: Add check for "S" quality to determine if the attribute is scenable once suported :
3454
// https://github.com/project-chip/connectedhomeip/issues/24177
35-
CHIP_ERROR ValidateAttributePath(EndpointId endpoint, ClusterId cluster, AttributeId attributeId)
55+
CHIP_ERROR ValidateAttributePath(EndpointId endpoint, ClusterId cluster, AttributeValuePairType & aVPair)
3656
{
37-
bool attIndex = emberAfContainsAttribute(endpoint, cluster, attributeId);
57+
bool attIndex = emberAfContainsAttribute(endpoint, cluster, aVPair.attributeID);
3858
if (!attIndex)
3959
{
4060
return CHIP_ERROR_UNSUPPORTED_ATTRIBUTE;
4161
}
62+
63+
EmberAfAttributeMetadata metadata = *emberAfLocateAttributeMetadata(endpoint, cluster, aVPair.attributeID);
64+
65+
// Cap value based on the attribute type size
66+
CapAttributeID(aVPair, &metadata);
67+
4268
return CHIP_NO_ERROR;
4369
}
4470
} // namespace
@@ -81,8 +107,9 @@ DefaultSceneHandlerImpl::SerializeAdd(EndpointId endpoint, const ExtensionFieldS
81107
auto pair_iterator = extensionFieldSet.attributeValueList.begin();
82108
while (pair_iterator.Next())
83109
{
84-
ReturnErrorOnFailure(ValidateAttributePath(endpoint, extensionFieldSet.clusterID, pair_iterator.GetValue().attributeID));
85-
aVPairs[pairCount] = pair_iterator.GetValue();
110+
AttributeValuePairType currentPair = pair_iterator.GetValue();
111+
ReturnErrorOnFailure(ValidateAttributePath(endpoint, extensionFieldSet.clusterID, currentPair));
112+
aVPairs[pairCount] = currentPair;
86113
pairCount++;
87114
}
88115
ReturnErrorOnFailure(pair_iterator.GetStatus());

‎src/app/tests/TestSceneTable.cpp

+38
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,44 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext)
722722

723723
memset(failBuffer, 0, fail_list.size());
724724
memset(buffer, 0, buff_span.size());
725+
726+
// Test Serialize Add of an attribute value that is greater than the mock attribute size (size of uint32_t)
727+
OOPairs[0].attributeValue = 0xFFFFFFFF'FFFFFFFF;
728+
729+
// EFS to test caping of value once a variable above the mock attribute size is serialize
730+
app::Clusters::ScenesManagement::Structs::ExtensionFieldSet::Type extensionFieldValueCapOut;
731+
app::Clusters::ScenesManagement::Structs::ExtensionFieldSet::DecodableType extensionFieldValueCapIn;
732+
733+
extensionFieldValueCapOut.clusterID = kOnOffClusterId;
734+
extensionFieldValueCapOut.attributeValueList = OOPairs;
735+
736+
/// Setup of input EFS (by temporary using the output one)
737+
writer.Init(buff_span);
738+
NL_TEST_ASSERT(
739+
aSuite, CHIP_NO_ERROR == app::DataModel::Encode(writer, TLV::AnonymousTag(), extensionFieldValueCapOut.attributeValueList));
740+
741+
reader.Init(buffer);
742+
extensionFieldValueCapIn.clusterID = kOnOffClusterId;
743+
NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next());
744+
NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == extensionFieldValueCapIn.attributeValueList.Decode(reader));
745+
746+
// Verify that the initial value is not capped
747+
auto pair_iterator = extensionFieldValueCapIn.attributeValueList.begin();
748+
pair_iterator.Next();
749+
app::Clusters::ScenesManagement::Structs::AttributeValuePair::Type pair = pair_iterator.GetValue();
750+
NL_TEST_ASSERT(aSuite, pair.attributeValue == OOPairs[0].attributeValue);
751+
752+
// Verify that we cap the value to the mock attribute size when serializing
753+
NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sHandler.SerializeAdd(kTestEndpoint1, extensionFieldValueCapIn, buff_span));
754+
NL_TEST_ASSERT(aSuite,
755+
CHIP_NO_ERROR == sHandler.Deserialize(kTestEndpoint1, kOnOffClusterId, buff_span, extensionFieldValueCapOut));
756+
757+
// Verify that the output value is capped
758+
NL_TEST_ASSERT(aSuite, extensionFieldValueCapOut.attributeValueList[0].attributeValue == 0x00000000'FFFFFFFF);
759+
760+
// Clear buffer
761+
memset(failBuffer, 0, fail_list.size());
762+
memset(buffer, 0, buff_span.size());
725763
};
726764

727765
void TestHandlerHelpers(nlTestSuite * aSuite, void * aContext) {}

‎src/app/util/mock/attribute-storage.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ uint8_t mockAttribute4[256] = {
7878
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
7979
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
8080
};
81+
EmberAfAttributeMetadata mockmetadata = { .defaultValue = EmberAfDefaultOrMinMaxAttributeValue(static_cast<uint32_t>(0)),
82+
.attributeId = 0,
83+
.size = sizeof(uint32_t),
84+
.attributeType = 0,
85+
.mask = 0 }; // namespace
8186

8287
} // namespace
8388

@@ -206,6 +211,11 @@ const EmberAfCluster * emberAfFindServerCluster(EndpointId endpointId, ClusterId
206211
return cluster->emberCluster();
207212
}
208213

214+
const EmberAfAttributeMetadata * emberAfLocateAttributeMetadata(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId)
215+
{
216+
return &mockmetadata;
217+
}
218+
209219
namespace chip {
210220
namespace app {
211221

‎src/app/zap-templates/zcl/data-model/chip/scene.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ limitations under the License.
3232
<struct name="AttributeValuePair">
3333
<cluster code="0x0062"/>
3434
<item name="AttributeID" type="attrib_id" optional="false"/>
35-
<item name="AttributeValue" type="int32u" optional="false"/>
35+
<item name="AttributeValue" type="int64u" optional="false"/>
3636
</struct>
3737

3838
<struct name="ExtensionFieldSet">

‎src/controller/data_model/controller-clusters.matter

+1-1
Original file line numberDiff line numberDiff line change
@@ -3750,7 +3750,7 @@ provisional cluster ScenesManagement = 98 {
37503750

37513751
struct AttributeValuePair {
37523752
attrib_id attributeID = 0;
3753-
int32u attributeValue = 1;
3753+
int64u attributeValue = 1;
37543754
}
37553755

37563756
struct ExtensionFieldSet {

‎src/controller/java/generated/java/matter/controller/cluster/structs/ScenesManagementClusterAttributeValuePair.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import matter.tlv.Tag
2222
import matter.tlv.TlvReader
2323
import matter.tlv.TlvWriter
2424

25-
class ScenesManagementClusterAttributeValuePair(val attributeID: UInt, val attributeValue: UInt) {
25+
class ScenesManagementClusterAttributeValuePair(val attributeID: UInt, val attributeValue: ULong) {
2626
override fun toString(): String = buildString {
2727
append("ScenesManagementClusterAttributeValuePair {\n")
2828
append("\tattributeID : $attributeID\n")
@@ -46,7 +46,7 @@ class ScenesManagementClusterAttributeValuePair(val attributeID: UInt, val attri
4646
fun fromTlv(tlvTag: Tag, tlvReader: TlvReader): ScenesManagementClusterAttributeValuePair {
4747
tlvReader.enterStructure(tlvTag)
4848
val attributeID = tlvReader.getUInt(ContextSpecificTag(TAG_ATTRIBUTE_I_D))
49-
val attributeValue = tlvReader.getUInt(ContextSpecificTag(TAG_ATTRIBUTE_VALUE))
49+
val attributeValue = tlvReader.getULong(ContextSpecificTag(TAG_ATTRIBUTE_VALUE))
5050

5151
tlvReader.exitContainer()
5252

‎src/darwin/Framework/CHIP/zap-generated/MTRCommandPayloadsObjc.mm

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎zzz_generated/app-common/app-common/zap-generated/cluster-objects.h

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎zzz_generated/darwin-framework-tool/zap-generated/cluster/Commands.h

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)