rdar://177309700 (Setup payload parse failures (Manual + QR) silently flattened to InvalidArgument)

Add NSError**-bearing inits on MTRSetupPayload (-initWithManualPairingCode:error:
public, -initWithQRCode:error: internal) that preserve the underlying CHIP_ERROR
via MTRError, so callers can distinguish typo/integrity-check failures from generic
parse rejection. Route MTRManualSetupPayloadParser, MTRQRCodeSetupPayloadParser,
+setupPayloadWithOnboardingPayload:error:, and -pairDevice:onboardingPayload:error:
through the new variants. No-error inits preserved as thin wrappers.

The error-bearing -initWithManualPairingCode:error: is declared once, on the public
MTRSetupPayload.h; the redundant MTR_DIRECT_MEMBERS re-declaration in
MTRSetupPayload_Internal.h is removed to avoid the duplicate/direct-conflict
declaration that broke the Darwin/tvOS framework builds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: woody-apple

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm

@@ -2383,11 +2383,12 @@ - (BOOL)pairDevice:(uint64_t)deviceID
 
 - (BOOL)pairDevice:(uint64_t)deviceID onboardingPayload:(NSString *)onboardingPayload error:(NSError * __autoreleasing *)error
 {
-    auto * setupPayload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload];
+    // Use the fine-grained -initWithPayload:error: (NOT the deprecated
+    // +setupPayloadWithOnboardingPayload:error:, which flattens to
+    // MTRErrorCodeInvalidArgument) so a mistyped manual pairing code surfaces
+    // as MTRErrorCodeIntegrityCheckFailed to the commissioning UI.
+    auto * setupPayload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload error:error];
     if (!setupPayload) {
-        if (error) {
-            *error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
-        }
         return NO;
     }
 

src/darwin/Framework/CHIP/MTRManualSetupPayloadParser.mm

@@ -33,8 +33,12 @@ - (id)initWithDecimalStringRepresentation:(NSString *)decimalStringRepresentatio
 
 - (MTRSetupPayload *)populatePayload:(NSError * __autoreleasing *)error
 {
-    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:_decimalStringRepresentation];
-    if (!payload && error) {
+    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:_decimalStringRepresentation
+                                                                             error:nil];
+    if (payload == nil && error != nil) {
+        // Deprecated surface: flatten to MTRErrorCodeInvalidArgument for the
+        // legacy contract. Callers wanting the fine-grained code should use
+        // -[MTRSetupPayload initWithManualPairingCode:error:].
         *error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
     }
     return payload;

src/darwin/Framework/CHIP/MTRQRCodeSetupPayloadParser.mm

@@ -33,8 +33,14 @@ - (id)initWithBase38Representation:(NSString *)base38Representation
 
 - (MTRSetupPayload *)populatePayload:(NSError * __autoreleasing *)error
 {
-    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithQRCode:_base38Representation];
-    if (!payload && error) {
+    // Pass _base38Representation through unchanged so the set of inputs this
+    // surface accepts/rejects stays identical to the prior implementation; we
+    // only route through the new -initWithQRCode:error: as a thin shim.
+    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithQRCode:_base38Representation error:nil];
+    if (payload == nil && error != nil) {
+        // Deprecated surface: flatten to MTRErrorCodeInvalidArgument for the
+        // legacy contract. Callers wanting the fine-grained code should use
+        // -[MTRSetupPayload initWithQRCode:error:].
         *error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
     }
     return payload;

src/darwin/Framework/CHIP/MTRSetupPayload.h

@@ -121,6 +121,44 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
  */
 - (nullable instancetype)initWithPayload:(NSString *)payload MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6));
 
+/**
+ * Initializes the payload object from the provided QR Code or Manual Pairing
+ * Code string, reporting a fine-grained error on failure.
+ *
+ * Returns nil and populates `error` (if non-nil) if the string is not a valid
+ * payload. The returned error is in MTRErrorDomain and PRESERVES the underlying
+ * parse-failure code rather than flattening it: in particular, a Manual Pairing
+ * Code with an incorrect Verhoeff check digit reports
+ * `MTRErrorCodeIntegrityCheckFailed`, letting a commissioning UI distinguish a
+ * mistyped setup code from other parse failures.
+ *
+ * This is the error-bearing counterpart of -initWithPayload:. Unlike the
+ * deprecated +setupPayloadWithOnboardingPayload:error:, it does not flatten all
+ * failures to MTRErrorCodeInvalidArgument.
+ *
+ * @note MTRErrorCodeInvalidArgument may still be returned for structurally
+ *       valid payloads whose contents fail semantic validation.
+ */
+- (nullable instancetype)initWithPayload:(NSString *)payload
+                                   error:(NSError * __autoreleasing *)error MTR_PROVISIONALLY_AVAILABLE;
+
+/**
+ * Initializes the payload object from the provided Manual Pairing Code string.
+ *
+ * Returns nil and populates `error` (if non-nil) if the string is not a valid
+ * Manual Pairing Code. The returned error is in MTRErrorDomain; in particular,
+ * a Manual Pairing Code with an incorrect Verhoeff check digit will report
+ * `MTRErrorCodeIntegrityCheckFailed`, allowing callers to distinguish a
+ * mistyped setup code from other parse failures.
+ *
+ * @note MTRErrorCodeInvalidArgument may still be returned for structurally
+ *       valid manual codes (e.g. correct Verhoeff digit) whose contents fail
+ *       semantic validation, distinct from MTRErrorCodeIntegrityCheckFailed
+ *       which indicates a check-digit failure.
+ */
+- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
+                                             error:(NSError * __autoreleasing *)error MTR_PROVISIONALLY_AVAILABLE;
+
 /**
  * Whether this object represents a concatenated QR Code payload consisting
  * of two or more underlying payloads. If YES, then:

src/darwin/Framework/CHIP/MTRSetupPayload.mm

@@ -225,7 +225,16 @@ + (void)initialize
 
 - (nullable instancetype)initWithPayload:(NSString *)payload
 {
-    return ([payload hasPrefix:@"MT:"]) ? [self initWithQRCode:payload] : [self initWithManualPairingCode:payload];
+    return [self initWithPayload:payload error:nil];
+}
+
+- (nullable instancetype)initWithPayload:(NSString *)payload error:(NSError * __autoreleasing *)error
+{
+    // Dispatch by the "MT:" prefix and preserve the fine-grained underlying
+    // error code (unlike +setupPayloadWithOnboardingPayload:error:, which
+    // flattens to MTRErrorCodeInvalidArgument).
+    return ([payload hasPrefix:@"MT:"]) ? [self initWithQRCode:payload error:error]
+                                        : [self initWithManualPairingCode:payload error:error];
 }
 
 - (CHIP_ERROR)initializeFromQRCode:(NSString *)qrCode validate:(BOOL)validate
@@ -269,17 +278,32 @@ - (CHIP_ERROR)initializeFromQRCode:(NSString *)qrCode validate:(BOOL)validate
 }
 
 - (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload
+{
+    return [self initWithQRCode:qrCodePayload error:nil];
+}
+
+- (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload
+                                  error:(NSError * __autoreleasing *)error
 {
     self = [super init];
     CHIP_ERROR err = [self initializeFromQRCode:qrCodePayload validate:YES];
     if (err != CHIP_NO_ERROR) {
+        if (error) {
+            *error = [MTRError errorForCHIPErrorCode:err];
+        }
         return nil;
     }
 
     return self;
 }
 
 - (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
+{
+    return [self initWithManualPairingCode:manualCode error:nil];
+}
+
+- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
+                                             error:(NSError * __autoreleasing *)error
 {
     self = [super init];
 
@@ -288,10 +312,16 @@ - (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
     CHIP_ERROR err = parser.populatePayload(_payload);
     if (err != CHIP_NO_ERROR) {
         MTR_LOG_ERROR("Failed to parse Manual Pairing Code: %" CHIP_ERROR_FORMAT, err.Format());
+        if (error) {
+            *error = [MTRError errorForCHIPErrorCode:err];
+        }
         return nil;
     }
     if (!_payload.isValidManualCode(chip::PayloadContents::ValidationMode::kConsume)) {
         MTR_LOG_ERROR("Invalid Manual Pairing Code");
+        if (error) {
+            *error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
+        }
         return nil;
     }
 
@@ -678,7 +708,13 @@ - (instancetype)initWithCoder:(NSCoder *)coder
     // did not encode it. When present, it carries almost the entire state of the object.
     NSString * qrCode = [coder decodeObjectOfClass:NSString.class forKey:MTRSetupPayloadCodingKeyQRCode];
     if (qrCode != nil) {
-        [self initializeFromQRCode:qrCode validate:NO];
+        // Intentionally fault-tolerant: don't -failWithError: on parse failure
+        // (older archives may hold payloads we no longer parse cleanly). Just
+        // log a breadcrumb and fall through to the fields decoded below.
+        CHIP_ERROR err = [self initializeFromQRCode:qrCode validate:NO];
+        if (err != CHIP_NO_ERROR) {
+            MTR_LOG_ERROR("initWithCoder: failed to restore QR code from archive: %" CHIP_ERROR_FORMAT, err.Format());
+        }
     } else {
         self.version = [coder decodeObjectOfClass:NSNumber.class forKey:MTRSetupPayloadCodingKeyVersion];
         self.vendorID = [coder decodeObjectOfClass:NSNumber.class forKey:MTRSetupPayloadCodingKeyVendorID];
@@ -781,8 +817,13 @@ + (instancetype)new
 + (MTRSetupPayload * _Nullable)setupPayloadWithOnboardingPayload:(NSString *)onboardingPayload
                                                            error:(NSError * __autoreleasing *)error
 {
-    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload];
-    if (!payload && error) {
+    // Deprecated surface: flatten every parse failure to
+    // MTRErrorCodeInvalidArgument, preserving the legacy contract for callers
+    // that switch on it. Callers wanting the fine-grained code should migrate
+    // to -initWithPayload:error:. (We pass nil for the underlying error so its
+    // localized description can't contradict the flattened code we report.)
+    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload error:nil];
+    if (payload == nil && error != nil) {
         *error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
     }
     return payload;

src/darwin/Framework/CHIP/MTRSetupPayload_Internal.h

@@ -28,6 +28,22 @@ MTR_DIRECT_MEMBERS
 
 - (instancetype)initWithSetupPayload:(const chip::SetupPayload &)setupPayload;
 - (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload;
+/**
+ * Initializes the payload object from a "MT:"-prefixed QR Code string.
+ *
+ * Returns nil and populates `error` (if non-nil) on parse failure. The
+ * returned error is in MTRErrorDomain; per-failure codes from the underlying
+ * Base38 decode (e.g. MTRErrorCodeInvalidIntegerValue for an out-of-alphabet
+ * character, MTRErrorCodeInvalidStringLength for a malformed chunk) are
+ * preserved so callers can distinguish typo-class failures from structural
+ * rejection.
+ *
+ * @note MTRErrorCodeInvalidArgument may still be returned for structurally
+ *       valid Base38 payloads whose contents fail semantic QR Code validation
+ *       (the !isValidQRCodePayload branch). This is intentionally identical
+ *       to the corresponding branch in -initWithManualPairingCode:error:.
+ */
+- (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload error:(NSError * __autoreleasing *)error;
 - (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode;
 
 @end

src/darwin/Framework/CHIPTests/MTRPairingBackwardsCompatTests.m

@@ -118,4 +118,33 @@ - (void)test002_PairDeviceWithString
     [self waitForExpectations:@[ expectation ] timeout:kPairingTimeoutInSeconds];
 }
 
+- (void)test003_PairDeviceWithBadManualPairingCode_SurfacesIntegrityCheckFailed
+{
+    // Integration regression pin: pairing through MTRDeviceController with a
+    // manual pairing code whose Verhoeff check digit is wrong must surface
+    // MTRErrorCodeIntegrityCheckFailed end-to-end, not the pre-fix generic
+    // MTRErrorCodeInvalidArgument.
+    //
+    // Deliberately does NOT call -startServerApp (unlike test001/test002): the
+    // bad check digit is rejected synchronously in
+    // -[MTRDeviceController pairDevice:onboardingPayload:error:] when it parses
+    // the payload via -[MTRSetupPayload initWithPayload:error:], which fails
+    // before any commissionable-node discovery or PASE handshake is attempted.
+    // The call returns NO inline, so no server, network, or paired device is
+    // needed and there is no asynchronous delegate callback to await. Adding a
+    // server app here would be incorrect -- it would never be contacted.
+    __auto_type * controller = [self createControllerOnTestFabric];
+    XCTAssertNotNil(controller);
+
+    NSError * error;
+    BOOL ok = [controller pairDevice:sDeviceId
+                   onboardingPayload:@"02684354589"
+                               error:&error];
+    XCTAssertFalse(ok);
+    XCTAssertNotNil(error);
+    XCTAssertEqualObjects(error.domain, MTRErrorDomain);
+    XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
+    XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
+}
+
 @end