From 92fbb50b631d0febf76b46735696c552b57b850b Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Thu, 11 Aug 2022 17:48:33 -0400
Subject: [PATCH] Fix command path checking in Darwin NSObjectCommandCallback.
 (#21817)

Fixes https://github.com/project-chip/connectedhomeip/issues/21814
---
 src/darwin/Framework/CHIP/MTRBaseDevice.mm    |   8 +-
 .../Framework/CHIPTests/MTRDeviceTests.m      | 103 ++++++++++++++----
 2 files changed, 88 insertions(+), 23 deletions(-)

diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
index c3e060d2751077..44995e909d5e21 100644
--- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
@@ -1039,6 +1039,7 @@ void OnResponse(app::CommandSender * apCommandSender, const app::ConcreteCommand
     OnErrorCallbackType mOnError;
     OnDoneCallbackType mOnDone;
     chip::ClusterId mClusterId;
+    // Id of the command we send.
     chip::CommandId mCommandId;
 };
 
@@ -1051,7 +1052,12 @@ void OnResponse(app::CommandSender * apCommandSender, const app::ConcreteCommand
     //
     // Validate that the data response we received matches what we expect in terms of its cluster and command IDs.
     //
-    VerifyOrExit(aCommandPath.mClusterId == mClusterId && aCommandPath.mCommandId == mCommandId, err = CHIP_ERROR_SCHEMA_MISMATCH);
+    VerifyOrExit(aCommandPath.mClusterId == mClusterId, err = CHIP_ERROR_SCHEMA_MISMATCH);
+
+    // If aReader is null, we got a status response and the command id in the
+    // path should match our command id.  If aReader is not null, we got a data
+    // response, which will have its own command id, which we don't know.
+    VerifyOrExit(aCommandPath.mCommandId == mCommandId || aReader != nullptr, err = CHIP_ERROR_SCHEMA_MISMATCH);
 
     if (aReader != nullptr) {
         err = app::DataModel::Decode(*aReader, response);
diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
index 84782d21d90b7b..b2aaf80d9753ad 100644
--- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
+++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
@@ -596,7 +596,6 @@ - (void)test007_WriteAttributeFailure
     [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kTimeoutInSeconds];
 }
 
-#if 0 // Re-enable test if the crash bug in CHIP stack is fixed to handle bad command Id
 - (void)test008_InvokeCommandFailure
 {
 #if MANUAL_INDIVIDUAL_TEST
@@ -608,34 +607,31 @@ - (void)test008_InvokeCommandFailure
     MTRBaseDevice * device = GetConnectedDevice();
     dispatch_queue_t queue = dispatch_get_main_queue();
 
-    NSDictionary * fields = @ {
-@"type" :
-        @"Structure",
-@"value" :
-        @[
-@{ @"contextTag" : @0, @"data" : @ { @"type" : @"UnsignedInteger", @"value" : @0 } },
-@{ @"contextTag" : @1, @"data" : @ { @"type" : @"UnsignedInteger", @"value" : @10 } }
+    NSDictionary * fields = @{
+        @"type" : @"Structure",
+        @"value" : @[
+            @{ @"contextTag" : @0, @"data" : @ { @"type" : @"UnsignedInteger", @"value" : @0 } },
+            @{ @"contextTag" : @1, @"data" : @ { @"type" : @"UnsignedInteger", @"value" : @10 } }
         ]
     };
     [device
-     invokeCommandWithEndpointId:@1
-     clusterId:@8
-     commandId:@40000
-     commandFields:fields
-     timedInvokeTimeout:nil
-     clientQueue:queue
-    completion:^(id _Nullable values, NSError * _Nullable error) {
-        NSLog(@"invoke command: MoveToLevelWithOnOff values: %@, error: %@", values, error);
-
-        XCTAssertNil(values);
-        XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], EMBER_ZCL_STATUS_UNSUPPORTED_COMMAND);
+        invokeCommandWithEndpointId:@1
+                          clusterId:@8
+                          commandId:@40000
+                      commandFields:fields
+                 timedInvokeTimeout:nil
+                        clientQueue:queue
+                         completion:^(id _Nullable values, NSError * _Nullable error) {
+                             NSLog(@"invoke command: MoveToLevelWithOnOff values: %@, error: %@", values, error);
 
-        [expectation fulfill];
-    }];
+                             XCTAssertNil(values);
+                             XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], EMBER_ZCL_STATUS_UNSUPPORTED_COMMAND);
+
+                             [expectation fulfill];
+                         }];
 
     [self waitForExpectations:[NSArray arrayWithObject:expectation] timeout:kTimeoutInSeconds];
 }
-#endif
 
 - (void)test009_SubscribeFailure
 {
@@ -1142,6 +1138,69 @@ - (void)test013_ReuseChipClusterObject
     [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil];
 }
 
+- (void)test014_InvokeCommandWithDifferentIdResponse
+{
+#if MANUAL_INDIVIDUAL_TEST
+    [self initStack];
+    [self waitForCommissionee];
+#endif
+    XCTestExpectation * expectation = [self expectationWithDescription:@"invoke Off command"];
+
+    MTRBaseDevice * device = GetConnectedDevice();
+    dispatch_queue_t queue = dispatch_get_main_queue();
+
+    NSDictionary * fields = @{
+        @"type" : @"Structure",
+        @"value" : @[],
+    };
+    // KeySetReadAllIndices in the Group Key Management has id 4 and a data response with id 5
+    [device
+        invokeCommandWithEndpointId:@0
+                          clusterId:@(0x003F)
+                          commandId:@4
+                      commandFields:fields
+                 timedInvokeTimeout:nil
+                        clientQueue:queue
+                         completion:^(id _Nullable values, NSError * _Nullable error) {
+                             NSLog(@"invoke command: KeySetReadAllIndices values: %@, error: %@", values, error);
+
+                             XCTAssertNil(error);
+
+                             {
+                                 XCTAssertTrue([values isKindOfClass:[NSArray class]]);
+                                 NSArray * resultArray = values;
+                                 for (NSDictionary * result in resultArray) {
+                                     MTRCommandPath * path = result[MTRCommandPathKey];
+                                     XCTAssertEqual([path.endpoint unsignedIntegerValue], 0);
+                                     XCTAssertEqual([path.cluster unsignedIntegerValue], 0x003F);
+                                     XCTAssertEqual([path.command unsignedIntegerValue], 5);
+                                     // We expect a KeySetReadAllIndicesResponse struct,
+                                     // which has context tag 0 pointing to a list with one
+                                     // item: 0 (the IPK's keyset id).
+                                     NSDictionary * expectedResult = @{
+                                         MTRTypeKey : MTRStructureValueType,
+                                         MTRValueKey : @[ @{
+                                             MTRContextTagKey : @0,
+                                             MTRDataKey : @ {
+                                                 MTRTypeKey : MTRArrayValueType,
+                                                 MTRValueKey : @[ @{
+                                                     MTRDataKey : @ { MTRTypeKey : MTRUnsignedIntegerValueType, MTRValueKey : @0 }
+                                                 } ]
+                                             }
+                                         } ],
+                                     };
+                                     XCTAssertEqualObjects(result[MTRDataKey], expectedResult);
+                                     XCTAssertNil(result[MTRErrorKey]);
+                                 }
+                                 XCTAssertEqual([resultArray count], 1);
+                             }
+
+                             [expectation fulfill];
+                         }];
+
+    [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil];
+}
+
 - (void)test900_SubscribeAllAttributes
 {
 #if MANUAL_INDIVIDUAL_TEST