Skip to content

Commit b3b41c8

Browse files
committed
Surface integrity-check failure when parsing a bad manual pairing code
A Manual Pairing Code with a wrong Verhoeff check digit causes ManualSetupPayloadParser::populatePayload to return CHIP_ERROR_INTEGRITY_CHECK_FAILED, but the MTR layer was discarding the specific code: -[MTRSetupPayload initWithManualPairingCode:] dropped it on the floor (no error out-param), and downstream call sites (+setupPayloadWithOnboardingPayload:error:, -[MTRManualSetupPayloadParser populatePayload:], -[MTRDeviceController pairDevice:onboardingPayload:error:]) all flattened parse failures to MTRErrorCodeInvalidArgument. Callers had no way to tell "the user typed a bad setup code" apart from any other parse error. Add an NSError**-bearing -initWithManualPairingCode:error: variant that maps the underlying CHIP_ERROR via [MTRError errorForCHIPErrorCode:], and route the three downstream call sites through it. The existing no-error initWithManualPairingCode: is preserved as a thin wrapper. The CHIP_ERROR_INTEGRITY_CHECK_FAILED -> MTRErrorCodeIntegrityCheckFailed mapping already exists in MTRError.mm; no new error code is needed. Adds three XCTests against MTRSetupPayloadTests.m pinning the new error shape on both APIs plus the happy path. The existing C++ regression guard (TestManualCode.TestPayloadParser_InvalidEntry) is left intact.
1 parent adba29b commit b3b41c8

6 files changed

Lines changed: 176 additions & 9 deletions

File tree

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.mm

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2383,11 +2383,8 @@ - (BOOL)pairDevice:(uint64_t)deviceID
23832383

23842384
- (BOOL)pairDevice:(uint64_t)deviceID onboardingPayload:(NSString *)onboardingPayload error:(NSError * __autoreleasing *)error
23852385
{
2386-
auto * setupPayload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload];
2386+
auto * setupPayload = [MTRSetupPayload setupPayloadWithOnboardingPayload:onboardingPayload error:error];
23872387
if (!setupPayload) {
2388-
if (error) {
2389-
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
2390-
}
23912388
return NO;
23922389
}
23932390

src/darwin/Framework/CHIP/MTRManualSetupPayloadParser.mm

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,7 @@ - (id)initWithDecimalStringRepresentation:(NSString *)decimalStringRepresentatio
3333

3434
- (MTRSetupPayload *)populatePayload:(NSError * __autoreleasing *)error
3535
{
36-
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:_decimalStringRepresentation];
37-
if (!payload && error) {
38-
*error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];
39-
}
40-
return payload;
36+
return [[MTRSetupPayload alloc] initWithManualPairingCode:_decimalStringRepresentation error:error];
4137
}
4238

4339
@end

src/darwin/Framework/CHIP/MTRSetupPayload.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
121121
*/
122122
- (nullable instancetype)initWithPayload:(NSString *)payload MTR_AVAILABLE(ios(17.6), macos(14.6), watchos(10.6), tvos(17.6));
123123

124+
/**
125+
* Initializes the payload object from the provided Manual Pairing Code string.
126+
*
127+
* Returns nil and populates `error` (if non-nil) if the string is not a valid
128+
* Manual Pairing Code. The returned error is in MTRErrorDomain; in particular,
129+
* a Manual Pairing Code with an incorrect Verhoeff check digit will report
130+
* `MTRErrorCodeIntegrityCheckFailed`, allowing callers to distinguish a
131+
* mistyped setup code from other parse failures.
132+
*/
133+
- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
134+
error:(NSError * __autoreleasing *)error MTR_PROVISIONALLY_AVAILABLE;
135+
124136
/**
125137
* Whether this object represents a concatenated QR Code payload consisting
126138
* of two or more underlying payloads. If YES, then:

src/darwin/Framework/CHIP/MTRSetupPayload.mm

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,12 @@ - (nullable instancetype)initWithQRCode:(NSString *)qrCodePayload
280280
}
281281

282282
- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
283+
{
284+
return [self initWithManualPairingCode:manualCode error:nil];
285+
}
286+
287+
- (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
288+
error:(NSError * __autoreleasing *)error
283289
{
284290
self = [super init];
285291

@@ -288,10 +294,16 @@ - (nullable instancetype)initWithManualPairingCode:(NSString *)manualCode
288294
CHIP_ERROR err = parser.populatePayload(_payload);
289295
if (err != CHIP_NO_ERROR) {
290296
MTR_LOG_ERROR("Failed to parse Manual Pairing Code: %" CHIP_ERROR_FORMAT, err.Format());
297+
if (error) {
298+
*error = [MTRError errorForCHIPErrorCode:err];
299+
}
291300
return nil;
292301
}
293302
if (!_payload.isValidManualCode(chip::PayloadContents::ValidationMode::kConsume)) {
294303
MTR_LOG_ERROR("Invalid Manual Pairing Code");
304+
if (error) {
305+
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
306+
}
295307
return nil;
296308
}
297309

@@ -781,6 +793,13 @@ + (instancetype)new
781793
+ (MTRSetupPayload * _Nullable)setupPayloadWithOnboardingPayload:(NSString *)onboardingPayload
782794
error:(NSError * __autoreleasing *)error
783795
{
796+
// For Manual Pairing Codes use the error-bearing init so we can surface
797+
// the underlying CHIP_ERROR (e.g. CHIP_ERROR_INTEGRITY_CHECK_FAILED for a
798+
// bad Verhoeff check digit) rather than flattening to InvalidArgument.
799+
if (![onboardingPayload hasPrefix:@"MT:"]) {
800+
return [[MTRSetupPayload alloc] initWithManualPairingCode:onboardingPayload error:error];
801+
}
802+
784803
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithPayload:onboardingPayload];
785804
if (!payload && error) {
786805
*error = [MTRError errorWithCode:MTRErrorCodeInvalidArgument];

src/darwin/Framework/CHIPTests/MTRPairingBackwardsCompatTests.m

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,25 @@ - (void)test002_PairDeviceWithString
118118
[self waitForExpectations:@[ expectation ] timeout:kPairingTimeoutInSeconds];
119119
}
120120

121+
- (void)test003_PairDeviceWithBadManualPairingCode_SurfacesIntegrityCheckFailed
122+
{
123+
// Integration regression pin: pairing through MTRDeviceController with a
124+
// manual pairing code whose Verhoeff check digit is wrong must surface
125+
// MTRErrorCodeIntegrityCheckFailed end-to-end, not the pre-fix generic
126+
// MTRErrorCodeInvalidArgument. No server app is needed -- this fails at
127+
// parse time before any PASE work is attempted.
128+
__auto_type * controller = [self createControllerOnTestFabric];
129+
XCTAssertNotNil(controller);
130+
131+
NSError * error;
132+
BOOL ok = [controller pairDevice:sDeviceId
133+
onboardingPayload:@"02684354589"
134+
error:&error];
135+
XCTAssertFalse(ok);
136+
XCTAssertNotNil(error);
137+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
138+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
139+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
140+
}
141+
121142
@end

src/darwin/Framework/CHIPTests/MTRSetupPayloadTests.m

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
#import <Matter/Matter.h>
1919
#import <XCTest/XCTest.h>
2020

21+
#import "MTRSetupPayload_Internal.h"
22+
2123
@interface MTRSetupPayloadTests : XCTestCase
2224

2325
@end
@@ -562,4 +564,124 @@ - (void)testValidSetupPasscode
562564
XCTAssertTrue([MTRSetupPayload isValidSetupPasscode:[MTRSetupPayload generateRandomSetupPasscode]]);
563565
}
564566

567+
- (void)testManualParser_BadCheckDigit_ReturnsIntegrityCheckFailed
568+
{
569+
// "02684354589" is a manual pairing code with a wrong Verhoeff check digit.
570+
// It must surface as MTRErrorCodeIntegrityCheckFailed, not be flattened to
571+
// MTRErrorCodeInvalidArgument (which loses the user-actionable distinction
572+
// between "you typed a bad code" and other parse failures).
573+
NSError * error;
574+
MTRSetupPayload * payload = [MTRSetupPayload setupPayloadWithOnboardingPayload:@"02684354589" error:&error];
575+
576+
XCTAssertNil(payload);
577+
XCTAssertNotNil(error);
578+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
579+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
580+
}
581+
582+
- (void)testManualParser_InitWithManualPairingCodeError_BadCheckDigit
583+
{
584+
NSError * error;
585+
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:@"02684354589" error:&error];
586+
587+
XCTAssertNil(payload);
588+
XCTAssertNotNil(error);
589+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
590+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
591+
}
592+
593+
- (void)testManualParser_InitWithManualPairingCodeError_Valid
594+
{
595+
NSError * error;
596+
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:@"641286075300001000016" error:&error];
597+
598+
XCTAssertNotNil(payload);
599+
XCTAssertNil(error);
600+
XCTAssertTrue(payload.hasShortDiscriminator);
601+
XCTAssertEqual(payload.discriminator.unsignedIntegerValue, 10);
602+
XCTAssertEqual(payload.setupPasscode.unsignedIntegerValue, 12345670);
603+
}
604+
605+
- (void)testManualParser_PopulatePayload_BadCheckDigit_ReturnsIntegrityCheckFailed
606+
{
607+
// Cover the MTRManualSetupPayloadParser entry point too, so a regression
608+
// that bypasses MTRSetupPayload's new init still gets caught.
609+
MTRManualSetupPayloadParser * parser =
610+
[[MTRManualSetupPayloadParser alloc] initWithDecimalStringRepresentation:@"02684354589"];
611+
NSError * error;
612+
MTRSetupPayload * payload = [parser populatePayload:&error];
613+
614+
XCTAssertNil(payload);
615+
XCTAssertNotNil(error);
616+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
617+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
618+
}
619+
620+
- (void)testOnboardingPayloadParser_Manual_BadCheckDigit_DistinguishesFromInvalidArgument
621+
{
622+
// Regression pin: +setupPayloadWithOnboardingPayload:error: must surface
623+
// MTRErrorCodeIntegrityCheckFailed for a bad-Verhoeff manual pairing code,
624+
// and explicitly NOT flatten to MTRErrorCodeInvalidArgument (the pre-fix
625+
// behavior). Guards against re-introduction of the flattening in any
626+
// downstream call site routed through this factory.
627+
NSError * error;
628+
MTRSetupPayload * payload = [MTRSetupPayload setupPayloadWithOnboardingPayload:@"02684354589" error:&error];
629+
630+
XCTAssertNil(payload);
631+
XCTAssertNotNil(error);
632+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
633+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
634+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
635+
}
636+
637+
- (void)testManualParser_BadCheckDigit_LegacyInitStillReturnsNil_AndNewInitNotFlattenedToInvalidArgument
638+
{
639+
// Regression pin for both shapes of the API on a bad-Verhoeff code:
640+
// - Legacy -initWithManualPairingCode: must still return nil
641+
// (source-compat for callers that relied on nil-on-failure).
642+
// - New -initWithManualPairingCode:error: must surface
643+
// MTRErrorCodeIntegrityCheckFailed and explicitly NOT
644+
// MTRErrorCodeInvalidArgument (pre-fix flattening).
645+
NSString * badCode = @"02684354589";
646+
647+
MTRSetupPayload * legacyPayload = [[MTRSetupPayload alloc] initWithManualPairingCode:badCode];
648+
XCTAssertNil(legacyPayload);
649+
650+
NSError * error;
651+
MTRSetupPayload * newPayload = [[MTRSetupPayload alloc] initWithManualPairingCode:badCode error:&error];
652+
XCTAssertNil(newPayload);
653+
XCTAssertNotNil(error);
654+
XCTAssertEqualObjects(error.domain, MTRErrorDomain);
655+
XCTAssertEqual(error.code, MTRErrorCodeIntegrityCheckFailed);
656+
XCTAssertNotEqual(error.code, MTRErrorCodeInvalidArgument);
657+
}
658+
659+
- (void)testManualParser_InitWithManualPairingCodeError_BoundaryInputs
660+
{
661+
// Boundary-input pin for -initWithManualPairingCode:error:. None of these
662+
// should crash, all should yield a nil payload, and any reported error
663+
// must be in MTRErrorDomain. Both &error and NULL out-param shapes are
664+
// exercised so a regression that null-derefs the out-param is caught.
665+
NSArray<NSString *> * badInputs = @[
666+
@"", // zero-length
667+
@"0", // sub-minimum
668+
@"1234567890", // exact-length-but-invalid (10 digits, not a valid manual code)
669+
@"abcdefghijk", // non-numeric
670+
@" 02684354589 ", // whitespace-padded
671+
@"026843545890268435458902684354589026843545890", // oversized
672+
];
673+
674+
for (NSString * input in badInputs) {
675+
NSError * error;
676+
MTRSetupPayload * payload = [[MTRSetupPayload alloc] initWithManualPairingCode:input error:&error];
677+
XCTAssertNil(payload, @"input=%@ unexpectedly produced a payload", input);
678+
XCTAssertNotNil(error, @"input=%@ produced nil payload but no error", input);
679+
XCTAssertEqualObjects(error.domain, MTRErrorDomain, @"input=%@ wrong error domain", input);
680+
681+
// NULL error out-param must not crash.
682+
MTRSetupPayload * payloadNoError = [[MTRSetupPayload alloc] initWithManualPairingCode:input error:NULL];
683+
XCTAssertNil(payloadNoError, @"input=%@ unexpectedly produced a payload (NULL error)", input);
684+
}
685+
}
686+
565687
@end

0 commit comments

Comments
 (0)