project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.42k stars 1.99k forks source link

Binding cluster does not enforce "min 1" constraint on the Group field of TargetStruct #34215

Open agatah2333 opened 3 months ago

agatah2333 commented 3 months ago

Reproduction steps

The value definition for the parameter min is missing in the Binding cluster.

image

the min of group_id should be 1

./chip-tool binding write binding '[{ "group":0, "cluster":6}]' 0x654325 0

This command could work incorrectly due to the missing constraints.

Bug prevalence

each time

GitHub hash of the SDK that was being used

5bb5c9e23d532cea40476fc0bd1d3008522792ba

Platform

other

Platform Version(s)

1.3

Type

Common Cluster Logic, Spec Compliance Issue, Core SDK Performance Improvement, Core SDK Interopability Issue

Anything else?

No response

bzbarsky-apple commented 3 months ago

This XML format does not have a way to express constraints on struct fields.

That said, the actual implementation in the Binding cluster needs to be enforcing the constraint, and it does not seem to do that.

agatah2333 commented 3 months ago

If you check the other XML files, for example, application-basic-cluster.xml, they all have a clear definition of min.

image

And one follow-up question: if the code needs to add the limitation, why does the XML provide the detailed limitation? Does it only affect the .matter file when using ZAP?

bzbarsky-apple commented 3 months ago

For writable attributes the min value is enforced automatically, at least if the value lives in the Ember datastore.

For struct fields there is no enforcement, because nothing uses those XML values for anything; they are not available from the ZAP database, as far as I know.