diff --git a/UnitTests/ObjCTests/MPBase_Attribute_Event_ProjectionTests.m b/UnitTests/ObjCTests/MPBase_Attribute_Event_ProjectionTests.m index 4da03f5a5..3cd496686 100644 --- a/UnitTests/ObjCTests/MPBase_Attribute_Event_ProjectionTests.m +++ b/UnitTests/ObjCTests/MPBase_Attribute_Event_ProjectionTests.m @@ -152,6 +152,30 @@ - (void)testAttributeInstance { XCTAssertNil(attributeProjection, @"Should have been nil."); } +- (void)testAttributeInstanceWithNullAttributeMap { + // The ObjC subscripted attribute_maps[attributeIndex] with no guard on the + // element, so a null entry crashed with -[NSNull objectForKeyedSubscript:]. + NSDictionary *configuration = @{@"action":@{@"attribute_maps":@[[NSNull null]]}}; + + MPAttributeProjection *attributeProjection = [[MPAttributeProjection alloc] initWithConfiguration:configuration + projectionType:MPProjectionTypeAttribute + attributeIndex:0]; + + XCTAssertNotNil(attributeProjection, @"Should not have been nil."); + XCTAssertEqual(attributeProjection.dataType, MPDataTypeString, @"Should have defaulted."); + XCTAssertFalse(attributeProjection.required, @"Should have defaulted to NO."); +} + +- (void)testAttributeInstanceWithANonArrayAttributeMaps { + NSDictionary *configuration = @{@"action":@{@"attribute_maps":@"not an array"}}; + + MPAttributeProjection *attributeProjection = [[MPAttributeProjection alloc] initWithConfiguration:configuration + projectionType:MPProjectionTypeAttribute + attributeIndex:0]; + + XCTAssertNil(attributeProjection, @"Should have been nil."); +} + - (void)testAttributeEquality { NSDictionary *configuration = @{@"action":@{@"attribute_maps":@[@{@"value":@"original attribute", @"projected_attribute_name":@"projected attribute", diff --git a/UnitTests/ObjCTests/MPKitConfigurationTests.m b/UnitTests/ObjCTests/MPKitConfigurationTests.m index bd1a69e67..b04ccad5b 100644 --- a/UnitTests/ObjCTests/MPKitConfigurationTests.m +++ b/UnitTests/ObjCTests/MPKitConfigurationTests.m @@ -1,6 +1,7 @@ #import #import "MPKitConfiguration.h" #import "MPBaseTestCase.h" +#import "MPEnums.h" @interface MPKitConfigurationTests : MPBaseTestCase { MPKitConfiguration *kitConfiguration; @@ -237,4 +238,78 @@ - (void)testStripNullsInConfig { XCTAssertNil(value); } +// message_type indexes two arrays holding +[MPEnum messageTypeSize] (20) entries. +// -setObject:atIndexedSubscript: allows index == count and appends, so +// MPMessageTypeMedia (20) has always been registered into a 21st slot. This +// pins that, so the out-of-range guard cannot regress it. +- (void)testMediaMessageTypeProjectionIsStillConfigured { + NSDictionary *configuration = @{ + @"id":@42, + @"pr":@[@{ + @"id":@1, + @"action":@{@"projected_event_name":@"Media Event"}, + @"matches":@[@{@"message_type":@(MPMessageTypeMedia)}] + }] + }; + + MPKitConfiguration *kitConfiguration = [[MPKitConfiguration alloc] initWithDictionary:configuration]; + + XCTAssertNotNil(kitConfiguration); + XCTAssertEqual(kitConfiguration.configuredMessageTypeProjections.count, (NSUInteger)(MPMessageTypeMedia + 1)); + XCTAssertTrue([kitConfiguration.configuredMessageTypeProjections[MPMessageTypeMedia] boolValue]); +} + +// Beyond index == count the subscript assignment really does raise +// NSRangeException, and message_type is unvalidated remote configuration. +- (void)testOutOfRangeMessageTypeProjectionIsIgnored { + NSDictionary *configuration = @{ + @"id":@42, + @"pr":@[@{ + @"id":@1, + @"action":@{@"projected_event_name":@"Bogus Event"}, + @"matches":@[@{@"message_type":@99}] + }] + }; + + MPKitConfiguration *kitConfiguration = [[MPKitConfiguration alloc] initWithDictionary:configuration]; + + XCTAssertNotNil(kitConfiguration); + XCTAssertEqual(kitConfiguration.configuredMessageTypeProjections.count, [MPEnum messageTypeSize]); + for (NSNumber *configured in kitConfiguration.configuredMessageTypeProjections) { + XCTAssertFalse([configured boolValue], @"No message type slot should have been configured."); + } +} + +// A negative message_type wraps to a huge NSUInteger on the way through. +- (void)testNegativeMessageTypeProjectionIsIgnored { + NSDictionary *configuration = @{ + @"id":@42, + @"pr":@[@{ + @"id":@1, + @"action":@{@"projected_event_name":@"Negative Event"}, + @"matches":@[@{@"message_type":@"-1"}] + }] + }; + + MPKitConfiguration *kitConfiguration = [[MPKitConfiguration alloc] initWithDictionary:configuration]; + + XCTAssertNotNil(kitConfiguration); + XCTAssertNil(kitConfiguration.projections); +} + +- (void)testInRangeMessageTypeProjectionIsStillConfigured { + NSDictionary *configuration = @{ + @"id":@42, + @"pr":@[@{ + @"id":@1, + @"action":@{@"projected_event_name":@"Renamed Event"}, + @"matches":@[@{@"message_type":@(MPMessageTypeEvent)}] + }] + }; + + MPKitConfiguration *kitConfiguration = [[MPKitConfiguration alloc] initWithDictionary:configuration]; + + XCTAssertTrue([kitConfiguration.configuredMessageTypeProjections[MPMessageTypeEvent] boolValue]); +} + @end diff --git a/mParticle-Apple-SDK-Swift/Sources/Kits/MPProjectionFieldParser.swift b/mParticle-Apple-SDK-Swift/Sources/Kits/MPProjectionFieldParser.swift index f7bc1fd49..6c07aaebf 100644 --- a/mParticle-Apple-SDK-Swift/Sources/Kits/MPProjectionFieldParser.swift +++ b/mParticle-Apple-SDK-Swift/Sources/Kits/MPProjectionFieldParser.swift @@ -40,8 +40,26 @@ public final class MPProjectionFields: NSObject { } } +@objc(MPAttributeProjectionFields) +public final class MPAttributeProjectionFields: NSObject { + /// Raw `MPDataType`. Objective-C defaulted to `MPDataTypeString` when the + /// key was absent or null; the ObjC setter still owns the range clamp. + @objc public let dataType: Int + @objc public let isRequired: Bool + + init(dataType: Int, isRequired: Bool) { + self.dataType = dataType + self.isRequired = isRequired + super.init() + } +} + @objc(MPProjectionFieldParser) public final class MPProjectionFieldParser: NSObject { + /// `MPDataTypeString` (MPIConstants.h:41). Mirrored because the Swift module + /// cannot import the ObjC module. + private static let dataTypeString = 1 + @objc(actionFromConfiguration:) public static func action(from configuration: [AnyHashable: Any]?) -> [AnyHashable: Any]? { configuration?["action"] as? [AnyHashable: Any] @@ -145,6 +163,43 @@ public final class MPProjectionFieldParser: NSObject { } } + /// `data_type` and `is_required` for one attribute map. + /// + /// The Objective-C read `attributeMaps[attributeIndex]` with no guard on the + /// container, the index, or the element, so a null or non-dictionary entry + /// crashed. Missing or unreadable values now fall back to the same defaults + /// the absent-key path always used. + @objc(attributeProjectionFieldsFromConfiguration:attributeIndex:) + public static func attributeProjectionFields( + from configuration: [AnyHashable: Any]?, + attributeIndex: UInt + ) -> MPAttributeProjectionFields { + guard let attributeMap = attributeMap(from: configuration, attributeIndex: attributeIndex) else { + return MPAttributeProjectionFields(dataType: dataTypeString, isRequired: false) + } + + return MPAttributeProjectionFields( + dataType: MPJSONCoercion.integerValue(attributeMap["data_type"]) ?? dataTypeString, + isRequired: MPJSONCoercion.boolValue(attributeMap["is_required"]) ?? false + ) + } + + /// The same navigation and guards `attributeFields(from:attributeIndex:)` + /// applies, so the two readers cannot drift. + private static func attributeMap( + from configuration: [AnyHashable: Any]?, + attributeIndex: UInt + ) -> [AnyHashable: Any]? { + guard let action = action(from: configuration), + let attributeMaps = action["attribute_maps"] as? [Any], + attributeIndex < attributeMaps.count + else { + return nil + } + + return attributeMaps[Int(attributeIndex)] as? [AnyHashable: Any] + } + private static func isNull(_ value: Any?) -> Bool { value == nil || value is NSNull } diff --git a/mParticle-Apple-SDK-Swift/Test/Kits/MPProjectionFieldParserTests.swift b/mParticle-Apple-SDK-Swift/Test/Kits/MPProjectionFieldParserTests.swift index 5822a0011..f91928f8c 100644 --- a/mParticle-Apple-SDK-Swift/Test/Kits/MPProjectionFieldParserTests.swift +++ b/mParticle-Apple-SDK-Swift/Test/Kits/MPProjectionFieldParserTests.swift @@ -182,4 +182,88 @@ final class MPProjectionFieldParserTests: XCTestCase { XCTAssertEqual(MPProjectionFieldParser.projectionId(from: ["id": "314"]), 314) XCTAssertEqual(MPProjectionFieldParser.projectionId(from: ["id": "not a number"]), 0) } + + // MARK: - attribute projection fields + + private func attributeConfiguration(_ attributeMaps: Any) -> [AnyHashable: Any] { + ["action": ["attribute_maps": attributeMaps]] + } + + func testAttributeProjectionFieldsReadsTheDataTypeAndRequiredFlag() { + let fields = MPProjectionFieldParser.attributeProjectionFields( + from: attributeConfiguration([["data_type": 3, "is_required": true]]), + attributeIndex: 0 + ) + + XCTAssertEqual(fields.dataType, 3) + XCTAssertTrue(fields.isRequired) + } + + func testAttributeProjectionFieldsDefaultsWhenTheKeysAreAbsent() { + let fields = MPProjectionFieldParser.attributeProjectionFields( + from: attributeConfiguration([["value": "attr"]]), + attributeIndex: 0 + ) + + // MPDataTypeString, and not required. + XCTAssertEqual(fields.dataType, 1) + XCTAssertFalse(fields.isRequired) + } + + func testAttributeProjectionFieldsDefaultsOnExplicitNulls() { + let fields = MPProjectionFieldParser.attributeProjectionFields( + from: attributeConfiguration([["data_type": NSNull(), "is_required": NSNull()]]), + attributeIndex: 0 + ) + + XCTAssertEqual(fields.dataType, 1) + XCTAssertFalse(fields.isRequired) + } + + func testAttributeProjectionFieldsAcceptsStringEncodedValues() { + // -integerValue and -boolValue were defined on NSString too, and remote + // configuration uses either form. + let fields = MPProjectionFieldParser.attributeProjectionFields( + from: attributeConfiguration([["data_type": "3", "is_required": "true"]]), + attributeIndex: 0 + ) + + XCTAssertEqual(fields.dataType, 3) + XCTAssertTrue(fields.isRequired) + } + + func testAttributeProjectionFieldsDefaultsOnANullAttributeMap() { + // The ObjC subscripted this element unguarded, so NSNull crashed with + // -[NSNull objectForKeyedSubscript:]. + let fields = MPProjectionFieldParser.attributeProjectionFields( + from: attributeConfiguration([NSNull()]), + attributeIndex: 0 + ) + + XCTAssertEqual(fields.dataType, 1) + XCTAssertFalse(fields.isRequired) + } + + func testAttributeProjectionFieldsDefaultsWhenAttributeMapsIsNotAnArray() { + for maps: Any in [["not": "an array"] as [AnyHashable: Any], "string", 7, NSNull()] { + let fields = MPProjectionFieldParser.attributeProjectionFields( + from: attributeConfiguration(maps), + attributeIndex: 0 + ) + + XCTAssertEqual(fields.dataType, 1) + XCTAssertFalse(fields.isRequired) + } + } + + func testAttributeProjectionFieldsDefaultsOutOfBoundsAndWithoutAConfiguration() { + let outOfBounds = MPProjectionFieldParser.attributeProjectionFields( + from: attributeConfiguration([["data_type": 3]]), + attributeIndex: 9 + ) + let missing = MPProjectionFieldParser.attributeProjectionFields(from: nil, attributeIndex: 0) + + XCTAssertEqual(outOfBounds.dataType, 1) + XCTAssertEqual(missing.dataType, 1) + } } diff --git a/mParticle-Apple-SDK/Kits/MPAttributeProjection.m b/mParticle-Apple-SDK/Kits/MPAttributeProjection.m index a29096eca..70ecac628 100644 --- a/mParticle-Apple-SDK/Kits/MPAttributeProjection.m +++ b/mParticle-Apple-SDK/Kits/MPAttributeProjection.m @@ -1,5 +1,7 @@ #import "MPAttributeProjection.h" +@import mParticle_Apple_SDK_Swift; + @implementation MPAttributeProjection - (id)init { @@ -13,13 +15,14 @@ - (instancetype)initWithConfiguration:(NSDictionary *)configuration projectionTy return nil; } - NSDictionary *actionDictionary = configuration[@"action"]; - NSArray *attributeMaps = actionDictionary[@"attribute_maps"]; - NSDictionary *attributeMap = attributeMaps[attributeIndex]; - - self.dataType = !MPIsNull(attributeMap[@"data_type"]) ? (MPDataType)[attributeMap[@"data_type"] integerValue] : MPDataTypeString; - _required = !MPIsNull(attributeMap[@"is_required"]) ? [attributeMap[@"is_required"] boolValue] : NO; - + MPAttributeProjectionFields *fields = + [MPProjectionFieldParser attributeProjectionFieldsFromConfiguration:configuration + attributeIndex:attributeIndex]; + + // The setter keeps the range clamp, so it stays the single source of it. + self.dataType = (MPDataType)fields.dataType; + _required = fields.isRequired; + return self; } diff --git a/mParticle-Apple-SDK/Kits/MPKitConfiguration.m b/mParticle-Apple-SDK/Kits/MPKitConfiguration.m index a999722ba..b17968ea4 100644 --- a/mParticle-Apple-SDK/Kits/MPKitConfiguration.m +++ b/mParticle-Apple-SDK/Kits/MPKitConfiguration.m @@ -168,10 +168,23 @@ - (void)configureProjections:(NSArray *)projections { MPEventProjection *eventProjection = [[MPEventProjection alloc] initWithConfiguration:projectionDictionary]; if (eventProjection) { - configuredMessageTypeProjectionsArray[eventProjection.messageType] = @YES; - + // message_type comes straight from remote configuration and indexes + // two arrays holding numberOfMessageTypes entries. + // -setObject:atIndexedSubscript: allows index == count and appends, + // which is how MPMessageTypeMedia (20) has always been registered + // even though +[MPEnum messageTypeSize] is only 20 — so that case is + // preserved rather than skipped. A larger value, or a negative one + // that wrapped, is a genuine NSRangeException. + NSUInteger messageType = eventProjection.messageType; + if (messageType > numberOfMessageTypes) { + MPILogError(@"Ignoring projection with out-of-range message type: %@", @(messageType)); + continue; + } + + configuredMessageTypeProjectionsArray[messageType] = @YES; + if (eventProjection.isDefault) { - defaultProjectionsArray[eventProjection.messageType] = eventProjection; + defaultProjectionsArray[messageType] = eventProjection; } else { [projectionsArray addObject:eventProjection]; }