[Darwin] Surface setup-payload parse failures instead of flattening to InvalidArgument

When ChipDnssdResolveNoLongerNeeded drops the consumer counter to zero we
were calling Finalize -> DNSServiceRefDeallocate immediately. Per the dnssd
contract, DNSServiceRefDeallocate discards any events already queued on
that connection but not yet read off the socket, so any inbound resolve
result that mDNSResponder had already written becomes invisible to us --
which manifests as a ~1s coldstart retry latency (the next resolve has to
start from scratch). A second observation from the mDNS owner is that
"starting and stopping queries doesn't query harder": a tight cancel-then-
restart pattern for the same instance name is strictly worse than letting
the existing query keep running.

This change introduces a deferred-teardown window (default 500ms) before
the actual DNSServiceRefDeallocate, gated by a per-ResolveContext
deferredTeardownScheduled flag. While the timer is armed:

  - A queued read indicator that fires inside the window dispatches the
    resolve result through the normal DispatchSuccess path, which
    cancels the timer and finalizes the context naturally.
  - A new ChipDnssdResolve for the same instance name reuses the
    existing ResolveContext: it cancels the deferred-teardown timer,
    bumps the consumer counter back to 1, and skips
    DNSServiceCreateConnection / DNSServiceResolve entirely. This is
    the coalescing half of the fix.
  - If neither happens before the window elapses, the timer fires
    OnResolveDeferredTeardown which calls Finalize(CHIP_ERROR_CANCELLED)
    -- preserving the existing failure-path contract upper-layer state
    machines (Discovery_ImplPlatform, MTRDevice CASE retry) rely on.

DispatchSuccess and DispatchFailure also cancel any pending teardown timer
so it can never fire against a freed context.

The window length is exposed via SetResolveDeferredTeardownDelay so the
new TestDarwinDnssdResolveCoalesce tests in TestDnssd.cpp can drive the
state machine in test time without waiting 500ms. Three tests cover:

  - Resolve -> ResolveNoLongerNeeded -> Resolve within the window
    reuses the same ResolveContext (counter 1 -> 0 -> 1).
  - Wait > teardown delay with no new Resolve actually fires the
    deferred teardown exactly once.
  - The CHIP_ERROR_CANCELLED dispatch contract is preserved when no

Author: woody-apple

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm

@@ -2383,11 +2383,8 @@ - (BOOL)pairDevice:(uint64_t)deviceID
 
 - (BOOL)pairDevice:(uint64_t)deviceID onboardingPayload:(NSString *)onboardingPayload error:(NSError * __autoreleasing *)error
 {
-    auto * setupPayload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload];
+    auto * setupPayload = [MTRSetupPayload setupPayloadWithOnboardingPayload:onboardingPayload error:error];
     if (!setupPayload) {
-        if (error) {
-            *error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
-        }
         return NO;
     }
 

src/darwin/Framework/CHIP/MTRManualSetupPayloadParser.mm

@@ -33,9 +33,16 @@ - (id)initWithDecimalStringRepresentation:(NSString *)decimalStringRepresentatio
 
 - (MTRSetupPayload *)populatePayload:(NSError * __autoreleasing *)error
 {
-    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:_decimalStringRepresentation];
-    if (!payload && error) {
-        *error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
+    NSError * underlyingError = nil;
+    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:_decimalStringRepresentation
+                                                                             error:&underlyingError];
+    if (payload == nil && error != nil) {
+        // Deprecated surface: preserve the legacy error-code contract by
+        // flattening every parse failure to MTRErrorCodeInvalidArgument.
+        // The new -[MTRSetupPayload initWithManualPairingCode:error:] entry
+        // point exposes the finer-grained underlying CHIP_ERROR codes for
+        // callers that migrate.
+        *error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:underlyingError.userInfo];
     }
     return payload;
 }

src/darwin/Framework/CHIP/MTRQRCodeSetupPayloadParser.mm

@@ -33,9 +33,25 @@ - (id)initWithBase38Representation:(NSString *)base38Representation
 
 - (MTRSetupPayload *)populatePayload:(NSError * __autoreleasing *)error
 {
-    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithQRCode:_base38Representation];
-    if (!payload && error) {
-        *error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
+    // chip::QRCodeSetupPayloadParser::ExtractPayload requires a segment beginning
+    // with the "MT:" prefix; without it the parser returns an empty payload and
+    // every input flattens to CHIP_ERROR_INVALID_ARGUMENT. Historically this
+    // surface accepted bare Base38, but production callers (notably HomeKit's
+    // HMMTRAccessorySetupPayload) pass an already-prefixed "MT:..." string. To
+    // remain compatible with both shapes, only prepend the prefix when it is
+    // not already present; otherwise we would feed "MT:MT:..." to the parser,
+    // which leaves an invalid ':' character in the Base38 alphabet.
+    NSString * qrCodeString = [_base38Representation hasPrefix:@"MT:"]
+        ? _base38Representation
+        : [@"MT:" stringByAppendingString:(_base38Representation ?: @"")];
+    NSError * underlyingError = nil;
+    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithQRCode:qrCodeString error:&underlyingError];
+    if (payload == nil && error != nil) {
+        // Deprecated surface: preserve the legacy error-code contract by
+        // flattening every parse failure to MTRErrorCodeInvalidArgument. The
+        // new -[MTRSetupPayload initWithQRCode:error:] entry point exposes the
+        // finer-grained codes for callers that migrate.
+        *error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:underlyingError.userInfo];
     }
     return payload;
 }

src/darwin/Framework/CHIP/MTRSetupPayload.h

@@ -121,6 +121,23 @@ 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 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

@@ -269,17 +269,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 +303,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 +699,17 @@ - (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];
+        // Pre-existing behavior is intentionally fault-tolerant here: we do not
+        // call -[NSCoder failWithError:] on parse failure to avoid breaking
+        // back-compat with archives written by older versions that may contain
+        // payloads we no longer parse cleanly. Log a breadcrumb so failures are
+        // observable in diagnostics; the rest of -initWithCoder: then falls
+        // through and the object is returned with whatever fields were
+        // separately 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,9 +812,23 @@ + (instancetype)new
 + (MTRSetupPayload * _Nullable)setupPayloadWithOnboardingPayload:(NSString *)onboardingPayload
                                                            error:(NSError * __autoreleasing *)error
 {
-    MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload];
-    if (!payload && error) {
-        *error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
+    // Deprecated surface: preserve the legacy error-code contract by
+    // flattening every parse failure to MTRErrorCodeInvalidArgument. The
+    // new -initWithQRCode:error: / -initWithManualPairingCode:error: entry
+    // points expose the finer-grained CHIP_ERROR codes (integrity-check
+    // failed, invalid string length, invalid integer value, etc.) for
+    // callers that migrate; existing callers switching on
+    // error.code == MTRErrorCodeInvalidArgument would silently miss every
+    // parse failure if we returned the fine-grained codes here.
+    NSError * underlyingError = nil;
+    MTRSetupPayload * payload = nil;
+    if ([onboardingPayload hasPrefix:@"MT:"]) {
+        payload = [[MTRSetupPayload alloc] initWithQRCode:onboardingPayload error:&underlyingError];
+    } else {
+        payload = [[MTRSetupPayload alloc] initWithManualPairingCode:onboardingPayload error:&underlyingError];
+    }
+    if (payload == nil && error != nil) {
+        *error = [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeInvalidArgument userInfo:underlyingError.userInfo];
     }
     return payload;
 }

src/darwin/Framework/CHIP/MTRSetupPayload_Internal.h

@@ -28,7 +28,38 @@ 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;
+/**
+ * Initializes the payload object from a Manual Pairing Code.
+ *
+ * Returns nil and populates `error` (if non-nil) on parse failure. The
+ * returned error is in MTRErrorDomain; per-failure codes from the underlying
+ * decimal-string decode (e.g. MTRErrorCodeIntegrityCheckFailed for a bad
+ * Verhoeff check digit, MTRErrorCodeInvalidStringLength for a code with the
+ * wrong number of digits) are preserved so callers can distinguish typo-class
+ * failures from structural rejection.
+ *
+ * @note MTRErrorCodeInvalidArgument may still be returned for structurally
+ *       valid decimal payloads whose contents fail semantic validation
+ *       (the !isValidManualCode branch).
+ */
+- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode error:(NSError * __autoreleasing *)error;
 
 @end
 

src/darwin/Framework/CHIPTests/MTRPairingBackwardsCompatTests.m

@@ -118,4 +118,25 @@ - (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. No server app is needed -- this fails at
+    // parse time before any PASE work is attempted.
+    __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