Fix validation of return value for invokeCommands. (project-chip#37442)

The response array for this case can have any number of entries, not just one.
espressif · Feb 7, 2025 · e1d26a0 · e1d26a0
1 parent ce47e22
commit e1d26a0
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 29 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDeviceDataValidation.h b/src/darwin/Framework/CHIP/MTRDeviceDataValidation.h
@@ -39,4 +39,9 @@ MTR_EXTERN MTR_TESTABLE BOOL MTREventReportIsWellFormed(NSArray<MTRDeviceRespons
 // objects in the right places.
 MTR_EXTERN MTR_TESTABLE BOOL MTRInvokeResponseIsWellFormed(NSArray<MTRDeviceResponseValueDictionary> * response);
 
+// Returns whether the provided invoke reponses actually has the right sorts of
+// objects in the right places.  This differes from
+// MTRInvokeResponseIsWellFormed in not enforcing that there is only one response.
+MTR_EXTERN MTR_TESTABLE BOOL MTRInvokeResponsesAreWellFormed(NSArray<MTRDeviceResponseValueDictionary> * response);
+
 NS_ASSUME_NONNULL_END
diff --git a/src/darwin/Framework/CHIP/MTRDeviceDataValidation.mm b/src/darwin/Framework/CHIP/MTRDeviceDataValidation.mm
@@ -165,51 +165,68 @@ BOOL MTREventReportIsWellFormed(NSArray<MTRDeviceResponseValueDictionary> * repo
 
 BOOL MTRInvokeResponseIsWellFormed(NSArray<MTRDeviceResponseValueDictionary> * response)
 {
-    if (!MTR_SAFE_CAST(response, NSArray)) {
-        MTR_LOG_ERROR("Invoke response is not an array: %@", response);
+    if (!MTRInvokeResponsesAreWellFormed(response)) {
         return NO;
     }
 
-    // Input is an array with a single value that must have MTRCommandPathKey.
+    // Input is an array with a single value.
     if (response.count != 1) {
         MTR_LOG_ERROR("Invoke response is not an array with exactly one entry: %@", response);
         return NO;
     }
 
-    MTRDeviceResponseValueDictionary responseValue = response[0];
+    return YES;
+}
 
-    if (!MTR_SAFE_CAST(responseValue, NSDictionary) || !MTR_SAFE_CAST(responseValue[MTRCommandPathKey], MTRCommandPath)) {
-        MTR_LOG_ERROR("Invoke response is not an array with the right things in it: %@", response);
+BOOL MTRInvokeResponsesAreWellFormed(NSArray<MTRDeviceResponseValueDictionary> * response)
+{
+    if (!MTR_SAFE_CAST(response, NSArray)) {
+        MTR_LOG_ERROR("Invoke response is not an array: %@", response);
         return NO;
     }
 
-    MTRDeviceDataValueDictionary _Nullable data = responseValue[MTRDataKey];
-    NSError * _Nullable error = responseValue[MTRErrorKey];
+    for (MTRDeviceResponseValueDictionary responseValue in response) {
+        // Each entry must be a dictionary that has MTRCommandPathKey.
 
-    if (data != nil && error != nil) {
-        MTR_LOG_ERROR("Invoke response claims to have both data and error: %@", responseValue);
-        return NO;
-    }
+        if (!MTR_SAFE_CAST(responseValue, NSDictionary) || !MTR_SAFE_CAST(responseValue[MTRCommandPathKey], MTRCommandPath)) {
+            MTR_LOG_ERROR("Invoke response has an invalid array entry: %@", responseValue);
+            return NO;
+        }
 
-    if (error != nil) {
-        return MTR_SAFE_CAST(error, NSError) != nil;
-    }
+        MTRDeviceDataValueDictionary _Nullable data = responseValue[MTRDataKey];
+        NSError * _Nullable error = responseValue[MTRErrorKey];
 
-    if (data == nil) {
-        // This is valid: indicates a success status response.
-        return YES;
-    }
+        if (data != nil && error != nil) {
+            MTR_LOG_ERROR("Invoke response claims to have both data and error: %@", responseValue);
+            return NO;
+        }
 
-    if (!MTRDataValueDictionaryIsWellFormed(data)) {
-        MTR_LOG_ERROR("Invoke response claims to have data that is not a data-value: %@", data);
-        return NO;
-    }
+        if (error != nil) {
+            if (!MTR_SAFE_CAST(error, NSError)) {
+                MTR_LOG_ERROR("Invoke response %@ has %@ instead of an NSError", responseValue, error);
+                return NO;
+            }
 
-    // Now we know data is a dictionary (in fact a data-value).  The only thing
-    // we promise about it is that it has type MTRStructureValueType.
-    if (![MTRStructureValueType isEqual:data[MTRTypeKey]]) {
-        MTR_LOG_ERROR("Invoke response data is not of structure type: %@", data);
-        return NO;
+            // Valid error response.
+            continue;
+        }
+
+        if (data == nil) {
+            // This is valid: indicates a success status response.
+            continue;
+        }
+
+        if (!MTRDataValueDictionaryIsWellFormed(data)) {
+            MTR_LOG_ERROR("Invoke response claims to have data that is not a data-value: %@", data);
+            return NO;
+        }
+
+        // Now we know data is a dictionary (in fact a data-value).  The only thing
+        // we promise about it is that it has type MTRStructureValueType.
+        if (![MTRStructureValueType isEqual:data[MTRTypeKey]]) {
+            MTR_LOG_ERROR("Invoke response data is not of structure type: %@", data);
+            return NO;
+        }
     }
 
     return YES;

diff --git a/src/darwin/Framework/CHIP/MTRDevice_XPC.mm b/src/darwin/Framework/CHIP/MTRDevice_XPC.mm
@@ -488,7 +488,7 @@ - (void)invokeCommands:(NSArray<NSArray<MTRCommandWithRequiredResponse *> *> *)c
                              return;
                          }
 
-                         if (responses != nil && !MTRInvokeResponseIsWellFormed(responses)) {
+                         if (responses != nil && !MTRInvokeResponsesAreWellFormed(responses)) {
                              MTR_LOG_ERROR("%@ got invoke responses for %@ that has invalid data: %@", self, commands, responses);
                              completion(nil, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT]);
                              return;

diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
@@ -5752,6 +5752,7 @@ - (void)test045_MTRDeviceInvokeGroups
                 completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
                     XCTAssertNil(error);
                     XCTAssertNotNil(values);
+                    XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values));
 
                     // Successful invoke is represented as a value with the relevant
                     // command path and neither data nor error.
@@ -5781,6 +5782,7 @@ - (void)test045_MTRDeviceInvokeGroups
                 completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
                     XCTAssertNil(error);
                     XCTAssertNotNil(values);
+                    XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values));
 
                     // We should not have anything for groups after the first one
                     XCTAssertEqual(values.count, 2);
@@ -5814,6 +5816,7 @@ - (void)test045_MTRDeviceInvokeGroups
                 completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
                     XCTAssertNil(error);
                     XCTAssertNotNil(values);
+                    XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values));
 
                     // We should not have anything for groups after the first one
                     __auto_type * expectedValues = @[
@@ -5858,6 +5861,7 @@ - (void)test045_MTRDeviceInvokeGroups
                 completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
                     XCTAssertNil(error);
                     XCTAssertNotNil(values);
+                    XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values));
 
                     XCTAssertEqual(values.count, 3);
 
@@ -5902,6 +5906,7 @@ - (void)test045_MTRDeviceInvokeGroups
                 completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
                     XCTAssertNil(error);
                     XCTAssertNotNil(values);
+                    XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values));
 
                     XCTAssertEqual(values.count, 3);
 
@@ -5946,6 +5951,7 @@ - (void)test045_MTRDeviceInvokeGroups
                 completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
                     XCTAssertNil(error);
                     XCTAssertNotNil(values);
+                    XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values));
 
                     XCTAssertEqual(values.count, 2);
 
@@ -5988,6 +5994,7 @@ - (void)test045_MTRDeviceInvokeGroups
                 completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
                     XCTAssertNil(error);
                     XCTAssertNotNil(values);
+                    XCTAssertTrue(MTRInvokeResponsesAreWellFormed(values));
 
                     XCTAssertEqual(values.count, 2);