Skip to content

Commit

Permalink
Add validation handling to Invoke processing (#580)
Browse files Browse the repository at this point in the history
* Add validation handling to Invoke processing

This PR is adding two things:
* Validation of request data for invoke request
* thrown ValidationError's are converted into a ConstraintError StatusCode like required in specs

* Add FabricIndex injection because missing/implicit in the logic

* Adjust tests

* Make ValidationError a StatusResponseError

* Remove unneeded Error conversion

* Adjust gitignore

* Fix tests
  • Loading branch information
Apollon77 authored Dec 14, 2023
1 parent 4d55a2c commit b41f44f
Show file tree
Hide file tree
Showing 43 changed files with 378 additions and 250 deletions.
2 changes: 1 addition & 1 deletion packages/matter-node.js/test/IntegrationTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ describe("Integration Test", () => {
assert.ok(basicInfoCluster);

await assert.rejects(async () => await basicInfoCluster.attributes.location.set("XXX"), {
message: "String is too long: 3, max 2.",
message: "(Validation/135) String is too long: 3, max 2.",
});
});

Expand Down
24 changes: 12 additions & 12 deletions packages/matter-node.js/test/cluster/GroupsServerTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
Identify,
} from "@project-chip/matter.js/cluster";
import { Message, SessionType } from "@project-chip/matter.js/codec";
import { ValidationError } from "@project-chip/matter.js/common";
import { EndpointNumber, GroupId } from "@project-chip/matter.js/datatype";
import { DeviceTypes, Endpoint } from "@project-chip/matter.js/device";
import { Fabric, FabricJsonObject } from "@project-chip/matter.js/fabric";
Expand Down Expand Up @@ -343,19 +344,18 @@ describe("Groups Server test", () => {
});

it("error on adding group with too long name", async () => {
const result = await callCommandOnClusterServer(
groupsServer!,
"addGroup",
{ groupId: GroupId(1), groupName: "12345678901234567" },
endpoint!,
testSession,
{ packetHeader: { sessionType: SessionType.Unicast } } as Message,
await assert.rejects(
async () =>
callCommandOnClusterServer(
groupsServer!,
"addGroup",
{ groupId: GroupId(1), groupName: "12345678901234567" },
endpoint!,
testSession,
{ packetHeader: { sessionType: SessionType.Unicast } } as Message,
),
new ValidationError("String is too long: 17, max 16."),
);

assert.ok(result);
assert.equal(result.code, StatusCode.Success);
assert.equal(result.response.status, StatusCode.ConstraintError);
assert.deepEqual(result.response.groupId, GroupId(1));
});
});

Expand Down
37 changes: 18 additions & 19 deletions packages/matter-node.js/test/cluster/ScenesServerTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
ScenesClusterHandler,
} from "@project-chip/matter.js/cluster";
import { Message, SessionType } from "@project-chip/matter.js/codec";
import { ValidationError } from "@project-chip/matter.js/common";
import { AttributeId, ClusterId, EndpointNumber, GroupId } from "@project-chip/matter.js/datatype";
import { DeviceTypes, Endpoint } from "@project-chip/matter.js/device";
import { Fabric, FabricJsonObject } from "@project-chip/matter.js/fabric";
Expand Down Expand Up @@ -735,26 +736,24 @@ describe("Scenes Server test", () => {
});

it("error on adding scene with too long name", async () => {
const result = await callCommandOnClusterServer(
scenesServer!,
"addScene",
{
groupId: GroupId(1),
sceneId: 1,
sceneName: "12345678901234567",
transitionTime: 2,
extensionFieldSets: [],
},
endpoint!,
testSession,
{ packetHeader: { sessionType: SessionType.Unicast } } as Message,
await assert.rejects(
async () =>
callCommandOnClusterServer(
scenesServer!,
"addScene",
{
groupId: GroupId(1),
sceneId: 1,
sceneName: "12345678901234567",
transitionTime: 2,
extensionFieldSets: [],
},
endpoint!,
testSession,
{ packetHeader: { sessionType: SessionType.Unicast } } as Message,
),
new ValidationError("String is too long: 17, max 16."),
);

assert.ok(result);
assert.equal(result.code, StatusCode.Success);
assert.equal(result.response.status, StatusCode.ConstraintError);
assert.deepEqual(result.response.groupId, GroupId(1));
assert.deepEqual(result.response.sceneId, 1);
});

it("recallScene on not existing group id", async () => {
Expand Down
119 changes: 119 additions & 0 deletions packages/matter-node.js/test/interaction/InteractionProtocolTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import {
BasicInformationCluster,
ClusterServer,
ClusterServerObjForCluster,
GeneralCommissioning,
OnOffCluster,
WritableAttribute,
} from "@project-chip/matter.js/cluster";
import { Message, SessionType } from "@project-chip/matter.js/codec";
import { ValidationError } from "@project-chip/matter.js/common";
import {
AttributeId,
ClusterId,
Expand Down Expand Up @@ -562,6 +564,33 @@ const INVOKE_COMMAND_REQUEST_INVALID: InvokeRequest = {
],
};

const INVOKE_COMMAND_REQUEST_VALIDATION_ERROR: InvokeRequest = {
interactionModelRevision: 10,
suppressResponse: false,
timedRequest: false,
invokeRequests: [
{
commandPath: { endpointId: EndpointNumber(0), clusterId: ClusterId(6), commandId: CommandId(2) },
},
],
};

const INVOKE_COMMAND_REQUEST_VALIDATION_ERROR_DATA: InvokeRequest = {
interactionModelRevision: 10,
suppressResponse: false,
timedRequest: false,
invokeRequests: [
{
commandPath: { endpointId: EndpointNumber(0), clusterId: ClusterId(0x30), commandId: CommandId(0x2) },
commandFields: GeneralCommissioning.TlvSetRegulatoryConfigRequest.encodeTlv({
newRegulatoryConfig: 1,
countryCode: "XXX",
breadcrumb: 0,
}),
},
],
};

const INVOKE_COMMAND_RESPONSE: InvokeResponse = {
interactionModelRevision: 10,
suppressResponse: false,
Expand All @@ -588,6 +617,32 @@ const INVOKE_COMMAND_RESPONSE_BUSY: InvokeResponse = {
],
};

const INVOKE_COMMAND_RESPONSE_VALIDATION_ERROR: InvokeResponse = {
interactionModelRevision: 10,
suppressResponse: false,
invokeResponses: [
{
status: {
commandPath: { clusterId: ClusterId(6), commandId: CommandId(2), endpointId: EndpointNumber(0) },
status: { status: 0x87, clusterStatus: undefined },
},
},
],
};

const INVOKE_COMMAND_RESPONSE_VALIDATION_ERROR_DATA: InvokeResponse = {
interactionModelRevision: 10,
suppressResponse: false,
invokeResponses: [
{
status: {
commandPath: { clusterId: ClusterId(0x30), commandId: CommandId(0x2), endpointId: EndpointNumber(0) },
status: { status: 0x87, clusterStatus: undefined },
},
},
],
};

const INVOKE_COMMAND_RESPONSE_INVALID: InvokeResponse = {
interactionModelRevision: 10,
suppressResponse: false,
Expand Down Expand Up @@ -1253,6 +1308,70 @@ describe("InteractionProtocol", () => {
assert.equal(onOffState, false);
});

it("invoke command with thrown validation error", async () => {
const onOffCluster = ClusterServer(
OnOffCluster,
{
onOff: onOffState,
},
{
on: async () => {
onOffState = true;
},
off: async () => {
onOffState = false;
},
toggle: async () => {
throw new ValidationError("test");
},
},
);
endpoint.addClusterServer(onOffCluster);
interactionProtocol.setRootEndpoint(endpoint);
const result = await interactionProtocol.handleInvokeRequest(
await getDummyMessageExchange(),
INVOKE_COMMAND_REQUEST_VALIDATION_ERROR,
{ packetHeader: { sessionType: SessionType.Unicast } } as Message,
);

assert.deepEqual(result, INVOKE_COMMAND_RESPONSE_VALIDATION_ERROR);
assert.equal(onOffState, false);
});

it("invoke command with data validation error", async () => {
const generalCommissioningCluster = ClusterServer(
GeneralCommissioning.Cluster,
{
breadcrumb: 0,
basicCommissioningInfo: { failSafeExpiryLengthSeconds: 1, maxCumulativeFailsafeSeconds: 1 },
regulatoryConfig: 1,
locationCapability: 1,
supportsConcurrentConnection: false,
},
{
armFailSafe: async () => {
throw new Error("should never be called");
},
setRegulatoryConfig: async () => {
throw new Error("should never be called");
},
commissioningComplete: async () => {
throw new Error("should never be called");
},
},
);
endpoint.addClusterServer(generalCommissioningCluster);
interactionProtocol.setRootEndpoint(endpoint);
const result = await interactionProtocol.handleInvokeRequest(
await getDummyMessageExchange(),
INVOKE_COMMAND_REQUEST_VALIDATION_ERROR_DATA,
{ packetHeader: { sessionType: SessionType.Unicast } } as Message,
);

assert.deepEqual(result, INVOKE_COMMAND_RESPONSE_VALIDATION_ERROR_DATA);
assert.equal(onOffState, false);
});

it("multi invoke commands", async () => {
onOffState = false;
let triggeredOn = false;
Expand Down
1 change: 1 addition & 0 deletions packages/matter.js-tools/.gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/build
/dist
/node_modules
7 changes: 3 additions & 4 deletions packages/matter.js/src/MatterDevice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,17 @@ import { Endpoint } from "./device/Endpoint.js";
import { Fabric } from "./fabric/Fabric.js";
import { FabricManager } from "./fabric/FabricManager.js";
import { Logger } from "./log/Logger.js";
import { isNetworkInterface, NetInterface } from "./net/NetInterface.js";
import { NetInterface, isNetworkInterface } from "./net/NetInterface.js";
import { NetworkError } from "./net/Network.js";
import { ChannelManager } from "./protocol/ChannelManager.js";
import { ExchangeManager } from "./protocol/ExchangeManager.js";
import { StatusResponseError } from "./protocol/interaction/InteractionMessenger.js";
import { StatusCode } from "./protocol/interaction/InteractionProtocol.js";
import { ProtocolHandler } from "./protocol/ProtocolHandler.js";
import { StatusCode, StatusResponseError } from "./protocol/interaction/StatusCode.js";
import { SecureChannelProtocol } from "./protocol/securechannel/SecureChannelProtocol.js";
import { PaseServer } from "./session/pase/PaseServer.js";
import { SecureSession } from "./session/SecureSession.js";
import { Session } from "./session/Session.js";
import { ResumptionRecord, SessionManager } from "./session/SessionManager.js";
import { PaseServer } from "./session/pase/PaseServer.js";
import { StorageContext } from "./storage/StorageContext.js";
import { Time, Timer } from "./time/Time.js";
import { ByteArray } from "./util/ByteArray.js";
Expand Down
4 changes: 2 additions & 2 deletions packages/matter.js/src/cluster/client/ClusterClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import { EventId } from "../../datatype/EventId.js";
import { Logger } from "../../log/Logger.js";
import { DecodedEventData } from "../../protocol/interaction/EventDataDecoder.js";
import { InteractionClient } from "../../protocol/interaction/InteractionClient.js";
import { StatusResponseError } from "../../protocol/interaction/InteractionMessenger.js";
import { StatusCode, TlvEventFilter } from "../../protocol/interaction/InteractionProtocol.js";
import { TlvEventFilter } from "../../protocol/interaction/InteractionProtocol.js";
import { StatusCode, StatusResponseError } from "../../protocol/interaction/StatusCode.js";
import { BitSchema, TypeFromPartialBitSchema } from "../../schema/BitmapSchema.js";
import { TypeFromSchema } from "../../tlv/TlvSchema.js";
import { toHexString } from "../../util/Number.js";
Expand Down
Loading

0 comments on commit b41f44f

Please sign in to comment.