Skip to content

Commit

Permalink
Correctly return RESOURCE_EXHAUSTED when too many bindings are writte…
Browse files Browse the repository at this point in the history
…n. (#25549)

Fixes #25538
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Feb 16, 2024
1 parent 7601b1a commit 1096607
Show file tree
Hide file tree
Showing 5 changed files with 255 additions and 14 deletions.
35 changes: 24 additions & 11 deletions src/app/clusters/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
#include <app/util/attribute-storage.h>
#include <app/util/config.h>
#include <lib/support/logging/CHIPLogging.h>
#include <protocols/interaction_model/StatusCode.h>

using namespace chip;
using namespace chip::app;
using namespace chip::app::Clusters;
Expand Down Expand Up @@ -117,7 +119,7 @@ CHIP_ERROR CheckValidBindingList(const EndpointId localEndpoint, const Decodable
return CHIP_NO_ERROR;
}

void CreateBindingEntry(const TargetStructType & entry, EndpointId localEndpoint)
CHIP_ERROR CreateBindingEntry(const TargetStructType & entry, EndpointId localEndpoint)
{
EmberBindingTableEntry bindingEntry;

Expand All @@ -131,7 +133,7 @@ void CreateBindingEntry(const TargetStructType & entry, EndpointId localEndpoint
entry.cluster);
}

AddBindingEntry(bindingEntry);
return AddBindingEntry(bindingEntry);
}

CHIP_ERROR BindingTableAccess::Read(const ConcreteReadAttributePath & path, AttributeValueEncoder & encoder)
Expand Down Expand Up @@ -225,10 +227,11 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath
}

// Add new entries
auto iter = newBindingList.begin();
while (iter.Next())
auto iter = newBindingList.begin();
CHIP_ERROR err = CHIP_NO_ERROR;
while (iter.Next() && err == CHIP_NO_ERROR)
{
CreateBindingEntry(iter.GetValue(), path.mEndpointId);
err = CreateBindingEntry(iter.GetValue(), path.mEndpointId);
}

// If this was not caused by a list operation, OnListWriteEnd is not going to be triggered
Expand All @@ -238,7 +241,7 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath
// Notify binding table has changed
LogErrorOnFailure(NotifyBindingsChanged());
}
return CHIP_NO_ERROR;
return err;
}
if (path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
{
Expand All @@ -248,8 +251,7 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath
{
return CHIP_IM_GLOBAL_STATUS(ConstraintError);
}
CreateBindingEntry(target, path.mEndpointId);
return CHIP_NO_ERROR;
return CreateBindingEntry(target, path.mEndpointId);
}
return CHIP_IM_GLOBAL_STATUS(UnsupportedWrite);
}
Expand All @@ -269,11 +271,22 @@ void MatterBindingPluginServerInitCallback()
registerAttributeAccessOverride(&gAttrAccess);
}

void AddBindingEntry(const EmberBindingTableEntry & entry)
CHIP_ERROR AddBindingEntry(const EmberBindingTableEntry & entry)
{
CHIP_ERROR err = BindingTable::GetInstance().Add(entry);
if (err == CHIP_ERROR_NO_MEMORY)
{
return CHIP_IM_GLOBAL_STATUS(ResourceExhausted);
}

if (err != CHIP_NO_ERROR)
{
return err;
}

if (entry.type == EMBER_UNICAST_BINDING)
{
CHIP_ERROR err = BindingManager::GetInstance().UnicastBindingCreated(entry.fabricIndex, entry.nodeId);
err = BindingManager::GetInstance().UnicastBindingCreated(entry.fabricIndex, entry.nodeId);
if (err != CHIP_NO_ERROR)
{
// Unicast connection failure can happen if peer is offline. We'll retry connection on-demand.
Expand All @@ -283,5 +296,5 @@ void AddBindingEntry(const EmberBindingTableEntry & entry)
}
}

BindingTable::GetInstance().Add(entry);
return CHIP_NO_ERROR;
}
2 changes: 1 addition & 1 deletion src/app/clusters/bindings/bindings.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@
*
* @param entry binding to add
*/
void AddBindingEntry(const EmberBindingTableEntry & entry);
CHIP_ERROR AddBindingEntry(const EmberBindingTableEntry & entry);
21 changes: 21 additions & 0 deletions src/app/tests/suites/TestBinding.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,24 @@ tests:
{ FabricIndex: 1, Node: 1, Endpoint: 1, Cluster: 6 },
{ FabricIndex: 1, Node: 2, Endpoint: 1 },
]

- label: "Write over-long binding table on endpoint 1"
command: "writeAttribute"
attribute: "Binding"
arguments:
value:
[
{ FabricIndex: 0, Node: 1, Endpoint: 1, Cluster: 6 },
{ FabricIndex: 0, Node: 2, Endpoint: 2, Cluster: 6 },
{ FabricIndex: 0, Node: 3, Endpoint: 3, Cluster: 6 },
{ FabricIndex: 0, Node: 4, Endpoint: 4, Cluster: 6 },
{ FabricIndex: 0, Node: 5, Endpoint: 5, Cluster: 6 },
{ FabricIndex: 0, Node: 6, Endpoint: 6, Cluster: 6 },
{ FabricIndex: 0, Node: 7, Endpoint: 7, Cluster: 6 },
{ FabricIndex: 0, Node: 8, Endpoint: 8, Cluster: 6 },
{ FabricIndex: 0, Node: 9, Endpoint: 9, Cluster: 6 },
{ FabricIndex: 0, Node: 10, Endpoint: 10, Cluster: 6 },
{ FabricIndex: 0, Node: 11, Endpoint: 11, Cluster: 6 },
]
response:
error: RESOURCE_EXHAUSTED
108 changes: 107 additions & 1 deletion 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.

103 changes: 102 additions & 1 deletion 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 1096607

Please sign in to comment.