Make sure we propagate TLV decode errors to consumers in Darwin frame…

…work. (#26890) In particular, in MTRDevice we should cache that we had a decode failure instead of leaving the cache un-filled.
project-chip · Aug 11, 2023 · 1620434 · 1620434
1 parent e9745f6
commit 1620434
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 8 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
@@ -574,7 +574,7 @@ id _Nullable MTRDecodeDataValueDictionaryFromCHIPTLV(chip::TLV::TLVReader * data
             chip::TLV::Tag tag = data->GetTag();
             id value = MTRDecodeDataValueDictionaryFromCHIPTLV(data);
             if (value == nullptr) {
-                MTR_LOG_ERROR("Error when decoding TLV container");
+                MTR_LOG_ERROR("Error when decoding TLV container of type %s", typeName.UTF8String);
                 return nil;
             }
             NSMutableDictionary * arrayElement = [NSMutableDictionary dictionary];

diff --git a/src/darwin/Framework/CHIP/MTRDevice.mm b/src/darwin/Framework/CHIP/MTRDevice.mm
@@ -1356,14 +1356,16 @@ - (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
             MTRErrorKey : [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT]
         }];
     } else {
-        id value;
-        if (apData == nullptr) {
-            value = nil;
+        id value = MTRDecodeDataValueDictionaryFromCHIPTLV(apData);
+        if (value == nil) {
+            MTR_LOG_ERROR("Failed to decode event data for path %@", eventPath);
+            [mEventReports addObject:@ {
+                MTREventPathKey : eventPath,
+                MTRErrorKey : [MTRError errorForCHIPErrorCode:CHIP_ERROR_DECODE_FAILED],
+            }];
         } else {
-            value = MTRDecodeDataValueDictionaryFromCHIPTLV(apData);
+            [mEventReports addObject:[MTRBaseDevice eventReportForHeader:aEventHeader andData:value]];
         }
-
-        [mEventReports addObject:[MTRBaseDevice eventReportForHeader:aEventHeader andData:value]];
     }
 }
 
@@ -1391,7 +1393,13 @@ - (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
         }];
     } else {
         id value = MTRDecodeDataValueDictionaryFromCHIPTLV(apData);
-        if (value) {
+        if (value == nil) {
+            MTR_LOG_ERROR("Failed to decode attribute data for path %@", attributePath);
+            [mAttributeReports addObject:@ {
+                MTRAttributePathKey : attributePath,
+                MTRErrorKey : [MTRError errorForCHIPErrorCode:CHIP_ERROR_DECODE_FAILED],
+            }];
+        } else {
             [mAttributeReports addObject:@ { MTRAttributePathKey : attributePath, MTRDataKey : value }];
         }
     }

diff --git a/src/darwin/Framework/CHIP/MTRError.h b/src/darwin/Framework/CHIP/MTRError.h
@@ -69,6 +69,13 @@ typedef NS_ERROR_ENUM(MTRErrorDomain, MTRErrorCode){
      * expected schema.
      */
     MTRErrorCodeSchemaMismatch MTR_NEWLY_AVAILABLE = 13,
+    /**
+     * MTRErrorCodeTLVDecodeFailed means that the TLV being decoded was malformed in
+     * some way.  This can include things like lengths running past the end of
+     * the buffer, strings that are not actually UTF-8, and various other
+     * TLV-level failures.
+     */
+    MTRErrorCodeTLVDecodeFailed MTR_NEWLY_AVAILABLE = 14,
 };
 // clang-format on
 

diff --git a/src/darwin/Framework/CHIP/MTRError.mm b/src/darwin/Framework/CHIP/MTRError.mm
@@ -89,6 +89,9 @@ + (NSError *)errorForCHIPErrorCode:(CHIP_ERROR)errorCode
         [userInfo addEntriesFromDictionary:@{
             NSLocalizedDescriptionKey : NSLocalizedString(@"The device is already a member of this fabric.", nil)
         }];
+    } else if (errorCode == CHIP_ERROR_DECODE_FAILED) {
+        code = MTRErrorCodeTLVDecodeFailed;
+        [userInfo addEntriesFromDictionary:@{ NSLocalizedDescriptionKey : NSLocalizedString(@"TLV decoding failed.", nil) }];
     } else {
         code = MTRErrorCodeGeneralError;
         [userInfo addEntriesFromDictionary:@{