Added type checks and proper size check based on min max if they are existing

Author: lpbeliveau-silabs

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

@@ -26,17 +26,92 @@ namespace {
 using ConcreteAttributePath  = app::ConcreteAttributePath;
 using AttributeValuePairType = app::Clusters::ScenesManagement::Structs::AttributeValuePair::Type;
 
+/// IsAttributeTypeValid
+/// @brief Check if the attribute type is valid for a scene extension field value according to the spec.
+/// @param type Type of the attribute's metadata
+/// @return
+bool IsSceneValidAttributeType(EmberAfAttributeType type)
+{
+    return (type == ZCL_INT8U_ATTRIBUTE_TYPE || type == ZCL_INT16U_ATTRIBUTE_TYPE || type == ZCL_INT32U_ATTRIBUTE_TYPE ||
+            type == ZCL_INT64U_ATTRIBUTE_TYPE || type == ZCL_INT8S_ATTRIBUTE_TYPE || type == ZCL_INT16S_ATTRIBUTE_TYPE ||
+            type == ZCL_INT32S_ATTRIBUTE_TYPE || type == ZCL_INT64S_ATTRIBUTE_TYPE);
+}
+
+/// IsSignedAttribute
+/// @brief Compare the attribute type to the signed attribute types in ZCL and return true if it is signed.
+/// @param attributeType metadata's attribute type
+/// @return
+bool IsSignedAttribute(EmberAfAttributeType attributeType)
+{
+    return (attributeType >= ZCL_INT8S_ATTRIBUTE_TYPE && attributeType <= ZCL_INT64S_ATTRIBUTE_TYPE);
+}
+
+/// ConvertByteArrayToUInt64
+/// @brief Helper function to convert a byte array to a uint64_t value
+/// @param EmberAfDefaultAttributeValue & defaultValue
+/// @param len Length of the byte array
+/// @return uint64_t Value
+/// @note The attribute table supports 8 bytes only for unsigned integers, 4 bytes is the maximum for signed integers
+uint64_t ConvertByteArrayToUInt64(const EmberAfDefaultAttributeValue & defaultValue, uint16_t len)
+{
+    if (len <= 2)
+    {
+        return static_cast<uint64_t>(defaultValue.defaultValue);
+    }
+    else
+    {
+        uint64_t result     = 0;
+        const uint8_t * val = defaultValue.ptrToDefaultValue;
+        for (size_t i = 0; i < len; i++)
+        {
+            result = (result << 8) | val[(CHIP_CONFIG_BIG_ENDIAN_TARGET ? i : (len - 1) - i)];
+        }
+        return result;
+    }
+}
+
 /// CapAttributeID
-/// Cap the attribute value based on the attribute type size
+/// Cap the attribute value based on the attribute's min and max if they are defined,
+/// or based on the attribute's size if they are not.
 /// @param[in] aVPair   AttributeValuePairType
 /// @param[in] metadata  EmberAfAttributeMetadata
 ///
 void CapAttributeID(AttributeValuePairType & aVPair, const EmberAfAttributeMetadata * metadata)
 {
     // Calculate the maximum value that can be represented with the given number of bytes
-    uint64_t maxValue = (1ULL << (metadata->size * 8)) - 1;
+    uint64_t maxValue = 0;
 
-    // If the attribute ID is greater than the maximum value, cap it
+    // Check if the attribute type is signed
+    if (IsSignedAttribute(metadata->attributeType))
+    {
+        maxValue = (1ULL << ((emberAfAttributeSize(metadata) * 8) - 1)) - 1;
+    }
+    else
+    {
+        maxValue = (1ULL << (emberAfAttributeSize(metadata) * 8)) - 1;
+    }
+
+    // Check metadata for min and max values
+    if ((metadata->mask & ATTRIBUTE_MASK_MIN_MAX) && metadata->defaultValue.ptrToMinMaxValue)
+    {
+        const EmberAfAttributeMinMaxValue * minMaxValue = metadata->defaultValue.ptrToMinMaxValue;
+        uint64_t minVal = ConvertByteArrayToUInt64(minMaxValue->minValue, emberAfAttributeSize(metadata));
+        uint64_t maxVal = ConvertByteArrayToUInt64(minMaxValue->maxValue, emberAfAttributeSize(metadata));
+
+        // Cap based on minValue
+        if (minVal > aVPair.attributeValue)
+        {
+            aVPair.attributeValue = minVal;
+        }
+
+        // Adjust maxValue if greater than the meta data's max value
+        if (maxVal < maxValue)
+        {
+            maxValue = maxVal;
+        }
+    }
+
+    // Cap based on maximum value
     if (aVPair.attributeValue > maxValue)
     {
         aVPair.attributeValue = maxValue;
@@ -50,7 +125,6 @@ void CapAttributeID(AttributeValuePairType & aVPair, const EmberAfAttributeMetad
 /// @return CHIP_ERROR_UNSUPPORTED_ATTRIBUTE if the attribute does not exist for a given cluster or is not scenable
 /// @note This will allways fail for global list attributes. If we do want to make them scenable someday, we will need to
 ///       use a different validation method.
-// TODO: Assess if (and how) we also want to throw an error if the attribute value is out of range
 // TODO: Add check for "S" quality to determine if the attribute is scenable once suported :
 // https://github.com/project-chip/connectedhomeip/issues/24177
 CHIP_ERROR ValidateAttributePath(EndpointId endpoint, ClusterId cluster, AttributeValuePairType & aVPair)
@@ -62,6 +136,11 @@ CHIP_ERROR ValidateAttributePath(EndpointId endpoint, ClusterId cluster, Attribu
         return CHIP_ERROR_UNSUPPORTED_ATTRIBUTE;
     }
 
+    if (!IsSceneValidAttributeType(metadata->attributeType))
+    {
+        return CHIP_ERROR_UNSUPPORTED_ATTRIBUTE;
+    }
+
     // Cap value based on the attribute type size
     CapAttributeID(aVPair, metadata);
 

src/app/tests/TestSceneTable.cpp

@@ -754,8 +754,11 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext)
     NL_TEST_ASSERT(aSuite,
                    CHIP_NO_ERROR == sHandler.Deserialize(kTestEndpoint1, kOnOffClusterId, buff_span, extensionFieldValueCapOut));
 
+    printf("extensionFieldValueCapOut.attributeValueList[0].attributeValue = 0x%016" PRIX64 "\n",
+           extensionFieldValueCapOut.attributeValueList[0].attributeValue);
+
     // Verify that the output value is capped
-    NL_TEST_ASSERT(aSuite, extensionFieldValueCapOut.attributeValueList[0].attributeValue == 0x00000000'FFFFFFFF);
+    NL_TEST_ASSERT(aSuite, extensionFieldValueCapOut.attributeValueList[0].attributeValue == 0x00FFFFFF);
 
     // Clear buffer
     memset(failBuffer, 0, fail_list.size());

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

@@ -78,11 +78,22 @@ uint8_t mockAttribute4[256]  = {
     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,
     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,
 };
-EmberAfAttributeMetadata mockmetadata = { .defaultValue  = EmberAfDefaultOrMinMaxAttributeValue(static_cast<uint32_t>(0)),
+
+const uint8_t defaultValueData[] = { 0x01, 0x02, 0x03, 0x04 };
+const uint8_t minValueData[]     = { 0x00, 0x00, 0x00, 0x00 };
+#if CHIP_CONFIG_BIG_ENDIAN_TARGET
+const uint8_t maxValueData[] = { 0x00, 0xFF, 0xFF, 0xFF }; // Equivalent, in big-endian, to 0x00FFFFFF
+#else
+const uint8_t maxValueData[] = { 0xFF, 0xFF, 0xFF, 0x00 }; // Equivalent, in little-endian, to 0x00FFFFFF
+#endif
+
+EmberAfAttributeMinMaxValue minMaxValue = { defaultValueData, minValueData, maxValueData };
+
+EmberAfAttributeMetadata mockmetadata = { .defaultValue  = EmberAfDefaultOrMinMaxAttributeValue(&minMaxValue),
                                           .attributeId   = 0,
                                           .size          = sizeof(uint32_t),
-                                          .attributeType = 0,
-                                          .mask          = 0 }; // namespace
+                                          .attributeType = ZCL_INT32U_ATTRIBUTE_TYPE,
+                                          .mask          = ATTRIBUTE_MASK_MIN_MAX }; // namespace
 
 } // namespace