Skip to content

Commit 13bf16c

Browse files
If optional parts of the state we receive over XPC are invalid, just ignore them. (#37592)
* If optional parts of the state we receive over XPC are invalid, just ignore them. This avoids failing the things that really matter because something else got confused somehow. Also fixes incorrect handling of the DeviceCachePrimed state. * Fix incorrect variable use. Co-authored-by: Jeff Tung <100387939+jtung-apple@users.noreply.github.com> --------- Co-authored-by: Jeff Tung <100387939+jtung-apple@users.noreply.github.com>
1 parent c075fcc commit 13bf16c

File tree

1 file changed

+36
-16
lines changed

1 file changed

+36
-16
lines changed

src/darwin/Framework/CHIP/MTRDevice_XPC.mm

+36-16
Original file line numberDiff line numberDiff line change
@@ -253,15 +253,23 @@ - (oneway void)deviceConfigurationChanged:(NSNumber *)nodeID
253253
}];
254254
}
255255

256-
static const auto * requiredInternalStateKeys = @[ kMTRDeviceInternalPropertyDeviceState, kMTRDeviceInternalPropertyLastSubscriptionAttemptWait ];
257-
static const auto * optionalInternalStateKeys = @[ kMTRDeviceInternalPropertyKeyVendorID, kMTRDeviceInternalPropertyKeyProductID, kMTRDeviceInternalPropertyNetworkFeatures, kMTRDeviceInternalPropertyMostRecentReportTime, kMTRDeviceInternalPropertyLastSubscriptionFailureTime ];
258-
259-
- (BOOL)_internalState:(NSDictionary *)dictionary hasValidValuesForKeys:(const NSArray<NSString *> *)keys valueRequired:(BOOL)required
256+
static const auto * requiredInternalStateKeys = @{
257+
kMTRDeviceInternalPropertyDeviceState : NSNumber.class,
258+
kMTRDeviceInternalPropertyLastSubscriptionAttemptWait : NSNumber.class,
259+
};
260+
261+
static const auto * optionalInternalStateKeys = @{
262+
kMTRDeviceInternalPropertyKeyVendorID : NSNumber.class,
263+
kMTRDeviceInternalPropertyKeyProductID : NSNumber.class,
264+
kMTRDeviceInternalPropertyNetworkFeatures : NSNumber.class,
265+
kMTRDeviceInternalPropertyMostRecentReportTime : NSDate.class,
266+
kMTRDeviceInternalPropertyLastSubscriptionFailureTime : NSDate.class,
267+
};
268+
269+
- (BOOL)_ensureValidValuesForKeys:(const NSDictionary<NSString *, Class> *)keys inInternalState:(NSMutableDictionary *)internalState valueRequired:(BOOL)required
260270
{
261-
// At one point, all keys were NSNumber valued; now there are also NSDates.
262-
// TODO: Create a mapping between keys and their expected types and use that in the type check below.
263271
for (NSString * key in keys) {
264-
id value = dictionary[key];
272+
id value = internalState[key];
265273
if (!value) {
266274
if (required) {
267275
MTR_LOG_ERROR("%@ device:internalStateUpdated: handed state with no value for \"%@\": %@", self, key, value);
@@ -270,10 +278,17 @@ - (BOOL)_internalState:(NSDictionary *)dictionary hasValidValuesForKeys:(const N
270278

271279
continue;
272280
}
273-
if (!MTR_SAFE_CAST(value, NSNumber) && !MTR_SAFE_CAST(value, NSDate)) {
281+
if (![value isKindOfClass:keys[key]]) {
274282
MTR_LOG_ERROR("%@ device:internalStateUpdated: handed state with invalid value of type %@ for \"%@\": %@", self,
275283
NSStringFromClass([value class]), key, value);
276-
return NO;
284+
if (required) {
285+
return NO;
286+
}
287+
288+
// If an optional value is invalid, just drop it and press on with
289+
// the other parts of the state, so we don't break those pieces
290+
// just because something optional has gone awry.
291+
[internalState removeObjectForKey:key];
277292
}
278293
}
279294

@@ -293,13 +308,21 @@ - (oneway void)device:(NSNumber *)nodeID internalStateUpdated:(NSDictionary *)di
293308
return;
294309
}
295310

311+
[self _updateInternalState:[dictionary mutableCopy]];
312+
}
313+
314+
- (void)_updateInternalState:(NSMutableDictionary *)newState
315+
{
316+
VerifyOrReturn([self _ensureValidValuesForKeys:requiredInternalStateKeys inInternalState:newState valueRequired:YES]);
317+
VerifyOrReturn([self _ensureValidValuesForKeys:optionalInternalStateKeys inInternalState:newState valueRequired:NO]);
318+
296319
NSNumber * oldStateNumber = MTR_SAFE_CAST(self._internalState[kMTRDeviceInternalPropertyDeviceState], NSNumber);
297-
NSNumber * newStateNumber = MTR_SAFE_CAST(dictionary[kMTRDeviceInternalPropertyDeviceState], NSNumber);
320+
NSNumber * newStateNumber = MTR_SAFE_CAST(newState[kMTRDeviceInternalPropertyDeviceState], NSNumber);
298321

299-
VerifyOrReturn([self _internalState:dictionary hasValidValuesForKeys:requiredInternalStateKeys valueRequired:YES]);
300-
VerifyOrReturn([self _internalState:dictionary hasValidValuesForKeys:optionalInternalStateKeys valueRequired:NO]);
322+
NSNumber * oldPrimedState = MTR_SAFE_CAST(self._internalState[kMTRDeviceInternalPropertyDeviceCachePrimed], NSNumber);
323+
NSNumber * newPrimedState = MTR_SAFE_CAST(newState[kMTRDeviceInternalPropertyDeviceCachePrimed], NSNumber);
301324

302-
[self _setInternalState:dictionary];
325+
[self _setInternalState:newState];
303326

304327
if (!MTREqualObjects(oldStateNumber, newStateNumber)) {
305328
MTRDeviceState state = self.state;
@@ -308,9 +331,6 @@ - (oneway void)device:(NSNumber *)nodeID internalStateUpdated:(NSDictionary *)di
308331
}];
309332
}
310333

311-
NSNumber * oldPrimedState = MTR_SAFE_CAST(self._internalState[kMTRDeviceInternalPropertyDeviceCachePrimed], NSNumber);
312-
NSNumber * newPrimedState = MTR_SAFE_CAST(dictionary[kMTRDeviceInternalPropertyDeviceCachePrimed], NSNumber);
313-
314334
if (!MTREqualObjects(oldPrimedState, newPrimedState)) {
315335
[self _lockAndCallDelegatesWithBlock:^(id<MTRDeviceDelegate> delegate) {
316336
if ([delegate respondsToSelector:@selector(deviceCachePrimed:)]) {

0 commit comments

Comments
 (0)