Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't call into AttributeAccessInterface for attributes that don't exist #11997

Merged
merged 1 commit into from
Nov 24, 2021
Merged

Don't call into AttributeAccessInterface for attributes that don't exist #11997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/app/tests/suites/TestCluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2033,3 +2033,10 @@ tests:
attribute: "nullable_char_string"
response:
value: ""

- label: "Read nonexistent attribute."
endpoint: 200
command: "readAttribute"
attribute: "list_int8u"
response:
error: 0x86 # UNSUPPORTED_ATTRIBUTE
46 changes: 33 additions & 13 deletions src/app/util/ember-compatibility-functions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,28 @@ bool ServerClusterCommandExists(const ConcreteCommandPath & aCommandPath)
return emberAfContainsServer(aCommandPath.mEndpointId, aCommandPath.mClusterId);
}

namespace {
CHIP_ERROR SendFailureStatus(const ConcreteAttributePath & aPath, AttributeReportIB::Builder & aAttributeReport,
Protocols::InteractionModel::Status aStatus, const TLV::TLVWriter & aReportCheckpoint)
{
aAttributeReport.Rollback(aReportCheckpoint);
AttributeStatusIB::Builder attributeStatusIBBuilder = aAttributeReport.CreateAttributeStatus();
AttributePathIB::Builder attributePathIBBuilder = attributeStatusIBBuilder.CreatePath();
attributePathIBBuilder.Endpoint(aPath.mEndpointId)
.Cluster(aPath.mClusterId)
.Attribute(aPath.mAttributeId)
.EndOfAttributePathIB();
ReturnErrorOnFailure(attributePathIBBuilder.GetError());
StatusIB::Builder statusIBBuilder = attributeStatusIBBuilder.CreateErrorStatus();
statusIBBuilder.EncodeStatusIB(StatusIB(aStatus));
ReturnErrorOnFailure(statusIBBuilder.GetError());
attributeStatusIBBuilder.EndOfAttributeStatusIB();
ReturnErrorOnFailure(attributeStatusIBBuilder.GetError());
return CHIP_NO_ERROR;
}

} // anonymous namespace

CHIP_ERROR ReadSingleClusterData(FabricIndex aAccessingFabricIndex, const ConcreteReadAttributePath & aPath,
AttributeReportIB::Builder & aAttributeReport)
{
Expand All @@ -234,6 +256,16 @@ CHIP_ERROR ReadSingleClusterData(FabricIndex aAccessingFabricIndex, const Concre
AttributeAccessInterface * attrOverride = findAttributeAccessOverride(aPath.mEndpointId, aPath.mClusterId);
if (attrOverride != nullptr)
{
const EmberAfAttributeMetadata * attributeMetadata =
emberAfLocateAttributeMetadata(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, CLUSTER_MASK_SERVER, 0);

if (attributeMetadata == nullptr)
{
// This attribute (or even this cluster) is not actually supported
// on this endpoint.
return SendFailureStatus(aPath, aAttributeReport, Protocols::InteractionModel::Status::UnsupportedAttribute, backup);
}

// TODO: We should probably clone the writer and convert failures here
// into status responses, unless our caller already does that.
writer = attributeDataIBBuilder.GetWriter();
Expand Down Expand Up @@ -433,19 +465,7 @@ CHIP_ERROR ReadSingleClusterData(FabricIndex aAccessingFabricIndex, const Concre
}
else
{
aAttributeReport.Rollback(backup);
attributeStatusIBBuilder = aAttributeReport.CreateAttributeStatus();
attributePathIBBuilder = attributeStatusIBBuilder.CreatePath();
attributePathIBBuilder.Endpoint(aPath.mEndpointId)
.Cluster(aPath.mClusterId)
.Attribute(aPath.mAttributeId)
.EndOfAttributePathIB();
ReturnErrorOnFailure(attributePathIBBuilder.GetError());
StatusIB::Builder statusIBBuilder = attributeStatusIBBuilder.CreateErrorStatus();
statusIBBuilder.EncodeStatusIB(StatusIB(imStatus));
ReturnErrorOnFailure(statusIBBuilder.GetError());
attributeStatusIBBuilder.EndOfAttributeStatusIB();
ReturnErrorOnFailure(attributeStatusIBBuilder.GetError());
ReturnErrorOnFailure(SendFailureStatus(aPath, aAttributeReport, imStatus, backup));
}
return CHIP_NO_ERROR;
}
Expand Down
5 changes: 3 additions & 2 deletions src/app/zap-templates/common/ClusterTestGeneration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ function setDefaultResponse(test)
setDefault(test[kResponseName], kConstraintsName, defaultResponseConstraints);

const hasResponseValue = 'value' in test[kResponseName];
const hasResponseError = 'error' in test[kResponseName];
const hasResponseConstraints = 'constraints' in test[kResponseName] && Object.keys(test[kResponseName].constraints).length;
const hasResponseValueOrConstraints = hasResponseValue || hasResponseConstraints;

Expand Down Expand Up @@ -233,10 +234,10 @@ function setDefaultResponse(test)
return;
}

if (!hasResponseValueOrConstraints) {
if (!hasResponseValueOrConstraints && !hasResponseError) {
console.log(test);
console.log(test[kResponseName]);
const errorStr = 'Test does not have a "value" or a "constraints" defined.';
const errorStr = 'Test does not have a "value" or a "constraints" defined and is not expecting an error.';
throwError(test, errorStr);
}

Expand Down
6 changes: 6 additions & 0 deletions src/darwin/Framework/CHIP.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
51B22C222740CB1D008D5055 /* CHIPCommandPayloadsObjc.h in Headers */ = {isa = PBXBuildFile; fileRef = 51B22C212740CB1D008D5055 /* CHIPCommandPayloadsObjc.h */; settings = {ATTRIBUTES = (Public, ); }; };
51B22C262740CB32008D5055 /* CHIPStructsObjc.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51B22C252740CB32008D5055 /* CHIPStructsObjc.mm */; };
51B22C2A2740CB47008D5055 /* CHIPCommandPayloadsObjc.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51B22C292740CB47008D5055 /* CHIPCommandPayloadsObjc.mm */; };
51E24E73274E0DAC007CCF6E /* CHIPErrorTestUtils.mm in Sources */ = {isa = PBXBuildFile; fileRef = 51E24E72274E0DAC007CCF6E /* CHIPErrorTestUtils.mm */; };
991DC0842475F45400C13860 /* CHIPDeviceController.h in Headers */ = {isa = PBXBuildFile; fileRef = 991DC0822475F45400C13860 /* CHIPDeviceController.h */; settings = {ATTRIBUTES = (Public, ); }; };
991DC0892475F47D00C13860 /* CHIPDeviceController.mm in Sources */ = {isa = PBXBuildFile; fileRef = 991DC0872475F47D00C13860 /* CHIPDeviceController.mm */; };
991DC08B247704DC00C13860 /* CHIPLogging.h in Headers */ = {isa = PBXBuildFile; fileRef = 991DC08A247704DC00C13860 /* CHIPLogging.h */; };
Expand Down Expand Up @@ -151,6 +152,7 @@
51B22C212740CB1D008D5055 /* CHIPCommandPayloadsObjc.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = CHIPCommandPayloadsObjc.h; path = "zap-generated/CHIPCommandPayloadsObjc.h"; sourceTree = "<group>"; };
51B22C252740CB32008D5055 /* CHIPStructsObjc.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = CHIPStructsObjc.mm; path = "zap-generated/CHIPStructsObjc.mm"; sourceTree = "<group>"; };
51B22C292740CB47008D5055 /* CHIPCommandPayloadsObjc.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = CHIPCommandPayloadsObjc.mm; path = "zap-generated/CHIPCommandPayloadsObjc.mm"; sourceTree = "<group>"; };
51E24E72274E0DAC007CCF6E /* CHIPErrorTestUtils.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CHIPErrorTestUtils.mm; sourceTree = "<group>"; };
991DC0822475F45400C13860 /* CHIPDeviceController.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CHIPDeviceController.h; sourceTree = "<group>"; };
991DC0872475F47D00C13860 /* CHIPDeviceController.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CHIPDeviceController.mm; sourceTree = "<group>"; };
991DC08A247704DC00C13860 /* CHIPLogging.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CHIPLogging.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -321,6 +323,7 @@
B202529A2459E34F00F97062 /* CHIPTests */ = {
isa = PBXGroup;
children = (
51E24E72274E0DAC007CCF6E /* CHIPErrorTestUtils.mm */,
1EB41B7A263C4CC60048E4C1 /* CHIPClustersTests.m */,
99C65E0F267282F1003402F6 /* CHIPControllerTests.m */,
B2F53AF1245B0DCF0010745E /* CHIPSetupPayloadParserTests.m */,
Expand Down Expand Up @@ -551,6 +554,7 @@
997DED1A26955D0200975E97 /* CHIPThreadOperationalDatasetTests.mm in Sources */,
99C65E10267282F1003402F6 /* CHIPControllerTests.m in Sources */,
B2F53AF2245B0DCF0010745E /* CHIPSetupPayloadParserTests.m in Sources */,
51E24E73274E0DAC007CCF6E /* CHIPErrorTestUtils.mm in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down Expand Up @@ -691,6 +695,7 @@
isa = XCBuildConfiguration;
buildSettings = {
CODE_SIGN_STYLE = Automatic;
"HEADER_SEARCH_PATHS[arch=*]" = "$(PROJECT_DIR)/../../../src";
INFOPLIST_FILE = CHIPTests/Info.plist;
LD_RUNPATH_SEARCH_PATHS = (
"$(inherited)",
Expand Down Expand Up @@ -814,6 +819,7 @@
buildSettings = {
CODE_SIGN_STYLE = Automatic;
DEVELOPMENT_TEAM = "";
"HEADER_SEARCH_PATHS[arch=*]" = "$(PROJECT_DIR)/../../../src";
INFOPLIST_FILE = CHIPTests/Info.plist;
LD_RUNPATH_SEARCH_PATHS = (
"$(inherited)",
Expand Down
23 changes: 23 additions & 0 deletions src/darwin/Framework/CHIP/CHIPError.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,27 @@ + (CHIP_ERROR)errorToCHIPErrorCode:(NSError *)error
return CHIP_ERROR_INTERNAL;
}
}

+ (uint8_t)errorToZCLErrorCode:(NSError * _Nullable)error
{
// If this is changed, change CHIPErrorTestUtils' version of
// errorToZCLErrorCode too.
if (error == nil) {
return EMBER_ZCL_STATUS_SUCCESS;
}
if (error.domain != CHIPErrorDomain) {
return EMBER_ZCL_STATUS_FAILURE;
}

switch (error.code) {
case CHIPErrorCodeDuplicateExists:
return EMBER_ZCL_STATUS_DUPLICATE_EXISTS;
case CHIPErrorCodeUnsupportedAttribute:
return EMBER_ZCL_STATUS_UNSUPPORTED_ATTRIBUTE;
case CHIPSuccess:
return EMBER_ZCL_STATUS_SUCCESS;
default:
return EMBER_ZCL_STATUS_FAILURE;
}
}
@end
3 changes: 2 additions & 1 deletion src/darwin/Framework/CHIP/CHIPError_Internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ NS_ASSUME_NONNULL_BEGIN
@interface CHIPError : NSObject
+ (nullable NSError *)errorForCHIPErrorCode:(CHIP_ERROR)errorCode;
+ (nullable NSError *)errorForZCLErrorCode:(uint8_t)errorCode;
+ (CHIP_ERROR)errorToCHIPErrorCode:(NSError *)errorCode;
+ (CHIP_ERROR)errorToCHIPErrorCode:(NSError *)error;
+ (uint8_t)errorToZCLErrorCode:(NSError * _Nullable)error;
@end

NS_ASSUME_NONNULL_END
2 changes: 2 additions & 0 deletions src/darwin/Framework/CHIP/templates/clusters-tests.zapt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
#import <CHIP/CHIP.h>
#import <CHIP/CHIPTestClustersObjc.h>

#import "CHIPErrorTestUtils.h"

// system dependencies
#import <XCTest/XCTest.h>

Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/templates/partials/test_cluster.zapt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ bool testSendCluster{{parent.filename}}_{{asTestIndex index}}_{{asUpperCamelCase
}
{{/if}}

XCTAssertEqual(err.code, {{response.error}});
XCTAssertEqual([CHIPErrorTestUtils errorToZCLErrorCode:err], {{response.error}});
{{#unless (isStrEqual "0" response.error)}}
[expectation fulfill];
{{else}}
Expand Down
Loading