Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NR-359870 fix for the attribute validator crash #340

Merged
merged 4 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions Agent.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1500,6 +1500,9 @@
F8678AAE2CDBC62B008FD2A2 /* NRAutoCollectLogStressTest.m in Sources */ = {isa = PBXBuildFile; fileRef = F8678AAC2CDBC62B008FD2A2 /* NRAutoCollectLogStressTest.m */; };
F8728E412ACC9D5A0056F641 /* NRMANetworkMonitor.m in Sources */ = {isa = PBXBuildFile; fileRef = F8728E402ACC9D5A0056F641 /* NRMANetworkMonitor.m */; };
F8728E422ACC9D5A0056F641 /* NRMANetworkMonitor.m in Sources */ = {isa = PBXBuildFile; fileRef = F8728E402ACC9D5A0056F641 /* NRMANetworkMonitor.m */; };
F87925652D441AC300576F5D /* NRMAAttributeValidator.m in Sources */ = {isa = PBXBuildFile; fileRef = F87925642D441AC300576F5D /* NRMAAttributeValidator.m */; };
F87925662D441AC300576F5D /* NRMAAttributeValidator.m in Sources */ = {isa = PBXBuildFile; fileRef = F87925642D441AC300576F5D /* NRMAAttributeValidator.m */; };
F87925672D441AC300576F5D /* NRMAAttributeValidator.m in Sources */ = {isa = PBXBuildFile; fileRef = F87925642D441AC300576F5D /* NRMAAttributeValidator.m */; };
F87954D529E89D5F00319FCD /* NRMAWKWebViewTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 0209ABB324E704A600E45C90 /* NRMAWKWebViewTests.m */; settings = {COMPILER_FLAGS = "-fno-objc-arc"; }; };
F88C2CF72A2FA7AC00373EFE /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = F88C2CE32A2FA7AC00373EFE /* PrivacyInfo.xcprivacy */; };
F89167372BC9D1270085BCFC /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = F88C2CE32A2FA7AC00373EFE /* PrivacyInfo.xcprivacy */; };
Expand Down Expand Up @@ -2472,6 +2475,8 @@
F8678AAC2CDBC62B008FD2A2 /* NRAutoCollectLogStressTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = NRAutoCollectLogStressTest.m; sourceTree = "<group>"; };
F8728E402ACC9D5A0056F641 /* NRMANetworkMonitor.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = NRMANetworkMonitor.m; sourceTree = "<group>"; };
F8728E562ACC9F840056F641 /* NRMANetworkMonitor.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = NRMANetworkMonitor.h; sourceTree = "<group>"; };
F87925642D441AC300576F5D /* NRMAAttributeValidator.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = NRMAAttributeValidator.m; sourceTree = "<group>"; };
F879257B2D441ACC00576F5D /* NRMAAttributeValidator.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = NRMAAttributeValidator.h; sourceTree = "<group>"; };
F88C2CE32A2FA7AC00373EFE /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = "<group>"; };
F89167382BC9D1540085BCFC /* libc++.tbd */ = {isa = PBXFileReference; lastKnownFileType = "sourcecode.text-based-dylib-definition"; name = "libc++.tbd"; path = "Platforms/WatchOS.platform/Developer/SDKs/WatchOS10.2.sdk/usr/lib/libc++.tbd"; sourceTree = DEVELOPER_DIR; };
F891A6BD2CB6F5C1007675F4 /* NRAutoLogCollector.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = NRAutoLogCollector.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3202,9 +3207,7 @@
34544D122A3142F900B5A8D1 /* NRMAEventManager.h */,
34544D132A3142F900B5A8D1 /* NRMAEventManager.m */,
34544D2D2A32489C00B5A8D1 /* NRMAAnalyticEventProtocol.h */,
342C4B402A3BCE9300116992 /* AttributeValidatorProtocol.h */,
342C4B562A3BD15A00116992 /* BlockAttributeValidator.h */,
342C4B572A3BD15A00116992 /* BlockAttributeValidator.m */,
F879257C2D441DBF00576F5D /* AttributeValidator */,
F8FBFB1E2A83CEBD00CDC8C5 /* Constants.m */,
F8FBFB212A83CEDC00CDC8C5 /* Constants.h */,
340D64122AA92FCE00E63CC1 /* PersistentEventStore.h */,
Expand Down Expand Up @@ -3885,6 +3888,18 @@
path = "Logging Stress Tests";
sourceTree = "<group>";
};
F879257C2D441DBF00576F5D /* AttributeValidator */ = {
isa = PBXGroup;
children = (
342C4B402A3BCE9300116992 /* AttributeValidatorProtocol.h */,
342C4B562A3BD15A00116992 /* BlockAttributeValidator.h */,
342C4B572A3BD15A00116992 /* BlockAttributeValidator.m */,
F879257B2D441ACC00576F5D /* NRMAAttributeValidator.h */,
F87925642D441AC300576F5D /* NRMAAttributeValidator.m */,
);
path = AttributeValidator;
sourceTree = "<group>";
};
F8FBFA3C2A71A32400CDC8C5 /* Events */ = {
isa = PBXGroup;
children = (
Expand Down Expand Up @@ -5359,6 +5374,7 @@
02FF49EF24DC645B00115469 /* NRMAAppInstallMetricGenerator.m in Sources */,
02FF4A7724DC64FC00115469 /* NRTimer.m in Sources */,
02FF484024DC614200115469 /* NRMALastActivityTraceController.m in Sources */,
F87925652D441AC300576F5D /* NRMAAttributeValidator.m in Sources */,
02FF489924DC61D200115469 /* NRMAURLSessionTaskDelegateBase.m in Sources */,
02FF4A9F24DC652E00115469 /* NRMAMemoryVitals.m in Sources */,
02FF4AB124DC652E00115469 /* NRMAJSON.m in Sources */,
Expand Down Expand Up @@ -5591,6 +5607,7 @@
02FF4C0224E3201400115469 /* NRMAHTTPTransactions.m in Sources */,
02FF4C0424E3201400115469 /* NRMAMachineMeasurementConsumer.m in Sources */,
2B33E5D32AA9160E00AEB7B4 /* NRMASessionEvent.m in Sources */,
F87925662D441AC300576F5D /* NRMAAttributeValidator.m in Sources */,
F8FBFA632A78416300CDC8C5 /* NRMAMobileEvent.m in Sources */,
02FF4C0524E3201400115469 /* NRMAMetric.m in Sources */,
F8E202DF2B07BA61008E0B7B /* NRMAOfflineStorage.m in Sources */,
Expand Down Expand Up @@ -5870,6 +5887,7 @@
348232F22BC5F20B0070FAC3 /* NRMAHarvesterConfiguration.m in Sources */,
348233122BC5F21B0070FAC3 /* NRMAExceptionHandlerJSONKeys.m in Sources */,
3482333A2BC5F2490070FAC3 /* NRMAClassNode.m in Sources */,
F87925672D441AC300576F5D /* NRMAAttributeValidator.m in Sources */,
3482334C2BC5F2710070FAC3 /* NRMAURLSessionTaskSearch.m in Sources */,
348232DA2BC5F2050070FAC3 /* NRMAHarvestableTrace.m in Sources */,
3482329B2BC5F1D70070FAC3 /* NRMAMeasurementException.m in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
//

#import "BlockAttributeValidator.h"
#import "Constants.h"
#import "NRMAAnalytics.h"
#import "NRLogger.h"

@implementation BlockAttributeValidator

Expand Down
14 changes: 14 additions & 0 deletions Agent/Analytics/AttributeValidator/NRMAAttributeValidator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//
// NRMAAttributeValidator.h
// Agent
//
// Created by Mike Bruin on 1/24/25.
// Copyright © 2025 New Relic. All rights reserved.
//

#import <Foundation/Foundation.h>
#import "AttributeValidatorProtocol.h"

@interface NRMAAttributeValidator : NSObject <AttributeValidatorProtocol>

@end
70 changes: 70 additions & 0 deletions Agent/Analytics/AttributeValidator/NRMAAttributeValidator.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
//
// NRMAAttributeValidator.m
// Agent
//
// Created by Mike Bruin on 1/24/25.
// Copyright © 2025 New Relic. All rights reserved.
//

#import "NRMAAttributeValidator.h"
#import "Constants.h"
#import "NRMAAnalytics.h"
#import "NRLogger.h"

@implementation NRMAAttributeValidator

- (BOOL)eventTypeValidator:(NSString *)eventType {
return YES;
}

- (BOOL)nameValidator:(NSString *)name {
if ([name length] == 0) {
NRLOG_AGENT_ERROR(@"invalid attribute: name length = 0");
return false;
}
if ([name hasPrefix:@" "]) {
NRLOG_AGENT_ERROR(@"invalid attribute: name prefix = \" \"");
return false;
}
// check if attribute name is reserved or attribute name matches reserved prefix.
for (NSString* key in [NRMAAnalytics reservedKeywords]) {
if ([key isEqualToString:name]) {
NRLOG_AGENT_ERROR(@"invalid attribute: name prefix disallowed");
return false;
}
}
for (NSString* key in [NRMAAnalytics reservedPrefixes]) {
if ([name hasPrefix:key]) {
NRLOG_AGENT_ERROR(@"invalid attribute: name prefix disallowed");
return false;
}
}

// check if attribute name exceeds max length.
if ([name length] > kNRMA_Attrib_Max_Name_Length) {
NRLOG_AGENT_ERROR(@"invalid attribute: name length exceeds limit");
return false;
}
return true;
}

- (BOOL)valueValidator:(id)value {
if ([value isKindOfClass:[NSString class]]) {
if ([(NSString*)value length] == 0) {
NRLOG_AGENT_ERROR(@"invalid attribute: value length = 0");
return false;
}
else if ([(NSString*)value length] >= kNRMA_Attrib_Max_Value_Size_Bytes) {
NRLOG_AGENT_ERROR(@"invalid attribute: value exceeded maximum byte size exceeded");
return false;
}
}
if (value == nil || [value isKindOfClass:[NSNull class]]) {
NRLOG_AGENT_ERROR(@"invalid attribute: value cannot be nil");
return false;
}

return true;
}

@end
54 changes: 2 additions & 52 deletions Agent/Analytics/NRMAAnalytics.mm
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
#import "NRMAPayload.h"
#import "NRMANetworkErrorEvent.h"
#import "NRMASAM.h"
#import "BlockAttributeValidator.h"
#import "NRMAAttributeValidator.h"
#import "NRMASessionEvent.h"

//******************* THIS FILE HAS ARC DISABLED *******************
Expand Down Expand Up @@ -116,59 +116,9 @@ - (id) initWithSessionStartTimeMS:(long long) sessionStartTime {
PersistentEventStore *eventStore = [[PersistentEventStore alloc] initWithFilename:filename andMinimumDelay:.025];

_eventManager = [[NRMAEventManager alloc] initWithPersistentStore:eventStore];
_attributeValidator = [[BlockAttributeValidator alloc] initWithNameValidator:^BOOL(NSString *name) {
if ([name length] == 0) {
NRLOG_AGENT_ERROR(@"invalid attribute: name length = 0");
return false;
}
if ([name hasPrefix:@" "]) {
NRLOG_AGENT_ERROR(@"invalid attribute: name prefix = \" \"");
return false;
}
// check if attribute name is reserved or attribute name matches reserved prefix.
for (NSString* key in [NRMAAnalytics reservedKeywords]) {
if ([key isEqualToString:name]) {
NRLOG_AGENT_ERROR(@"invalid attribute: name prefix disallowed");
return false;
}
}
for (NSString* key in [NRMAAnalytics reservedPrefixes]) {
if ([name hasPrefix:key]) {
NRLOG_AGENT_ERROR(@"invalid attribute: name prefix disallowed");
return false;
}
}

// check if attribute name exceeds max length.
if ([name length] > kNRMA_Attrib_Max_Name_Length) {
NRLOG_AGENT_ERROR(@"invalid attribute: name length exceeds limit");
return false;
}
return true;

} valueValidator:^BOOL(id value) {
if ([value isKindOfClass:[NSString class]]) {
if ([(NSString*)value length] == 0) {
NRLOG_AGENT_ERROR(@"invalid attribute: value length = 0");
return false;
}
else if ([(NSString*)value length] >= kNRMA_Attrib_Max_Value_Size_Bytes) {
NRLOG_AGENT_ERROR(@"invalid attribute: value exceeded maximum byte size exceeded");
return false;
}
}
if (value == nil || [value isKindOfClass:[NSNull class]]) {
NRLOG_AGENT_ERROR(@"invalid attribute: value cannot be nil");
return false;
}

return true;
} andEventTypeValidator:^BOOL(NSString *eventType) {
return YES;
}];
_attributeValidator = [[NRMAAttributeValidator alloc] init];
_sessionAttributeManager = [[NRMASAM alloc] initWithAttributeValidator:_attributeValidator];


NSString* attributes = [self sessionAttributeJSONString];
if (attributes != nil && [attributes length] > 0) {
NSDictionary* dictionary = [NSJSONSerialization JSONObjectWithData:[attributes dataUsingEncoding:NSUTF8StringEncoding]
Expand Down
4 changes: 3 additions & 1 deletion Agent/General/NewRelicAgentInternal.m
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
#import "NRMAUDIDManager.h"
#import "NRMASupportMetricHelper.h"
#import "NRAutoLogCollector.h"
#import "NRMAAttributeValidator.h"


// Support for teardown and re-setup of the agent within a process lifetime for our test harness
Expand Down Expand Up @@ -568,7 +569,8 @@ - (void) onSessionStart {
sessionStartTime:self.appSessionStartDate
agentConfiguration:self.agentConfiguration
platform:[NewRelicInternalUtils osName]
sessionId:[self currentSessionId]];
sessionId:[self currentSessionId]
attributeValidator:[[NRMAAttributeValidator alloc] init]];

if (status != NotReachable) { // Because we support offline mode check if we're online before sending the handled exceptions
[self.handledExceptionsController processAndPublishPersistedReports];
Expand Down
3 changes: 2 additions & 1 deletion Agent/HandledException/NRMAHandledExceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ extern const NSString* kHexBackupStoreFolder;
sessionStartTime:(NSDate*)sessionStartDate
agentConfiguration:(NRMAAgentConfiguration*)agentConfiguration
platform:(NSString*)platform
sessionId:(NSString*)sessionId;
sessionId:(NSString*)sessionId
attributeValidator:(id<AttributeValidatorProtocol>) attributeValidator;

- (void) recordHandledException:(NSException*) exception
attributes:(NSDictionary* _Nullable)attributes;
Expand Down
21 changes: 14 additions & 7 deletions Agent/HandledException/NRMAHandledExceptions.mm
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#import "NRMABool.h"
#import "NRMASupportMetricHelper.h"
#import "Constants.h"
#import "NRMAAttributeValidator.h"

@interface NRMAAnalytics(Protected)
// Because the NRMAAnalytics class interfaces with non Objective-C++ files, we cannot expose the API on the header. Therefore, we must use this reference.
Expand All @@ -39,6 +40,7 @@ @implementation NRMAHandledExceptions {
NewRelic::Hex::HexUploadPublisher* _publisher;

NRMAAnalytics *analyticsParent;
id<AttributeValidatorProtocol> _attributeValidator;
}

- (void) dealloc {
Expand All @@ -49,6 +51,7 @@ - (void) dealloc {

self.sessionId = nil;
self.sessionStartDate = nil;
[_attributeValidator release];

[super dealloc];
}
Expand All @@ -57,7 +60,8 @@ - (instancetype) initWithAnalyticsController:(NRMAAnalytics*)analytics
sessionStartTime:(NSDate*)sessionStartDate
agentConfiguration:(NRMAAgentConfiguration*)agentConfiguration
platform:(NSString*)platform
sessionId:(NSString*)sessionId {
sessionId:(NSString*)sessionId
attributeValidator:(id<AttributeValidatorProtocol>) attributeValidator {
if (analytics == nil || sessionStartDate == nil || [agentConfiguration applicationToken] == nil || platform == nil || sessionId == nil) {
NSMutableArray* missingParams = [[NSMutableArray new] autorelease];
if ([agentConfiguration applicationToken] == nil) [missingParams addObject:@"appToken"];
Expand All @@ -71,6 +75,8 @@ - (instancetype) initWithAnalyticsController:(NRMAAnalytics*)analytics
self = [super init];
if (self) {
analyticsParent = analytics;

_attributeValidator = [attributeValidator retain];

_analytics = std::shared_ptr<NewRelic::AnalyticsController>([analytics analyticsController]);
self.sessionStartDate = sessionStartDate;
Expand Down Expand Up @@ -198,7 +204,8 @@ - (void) recordError:(NSError * _Nonnull)error
resultMap,
[self createThreadVector:callstack length:frames]
);
NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:[analyticsParent getAttributeValidator]] autorelease];

NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:_attributeValidator] autorelease];

if (attributes != nil) {
[contextAdapter addAttributesNewValidation:attributes];
Expand All @@ -218,7 +225,7 @@ - (void) recordError:(NSError * _Nonnull)error
[self createThreadVector:callstack length:frames]
);

NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:[analyticsParent getAttributeValidator]] autorelease];
NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:_attributeValidator] autorelease];

if (attributes != nil) {
[contextAdapter addAttributes:attributes];
Expand Down Expand Up @@ -268,7 +275,7 @@ - (void) recordHandledException:(NSException*)exception

[self checkOffline:report];

NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:[analyticsParent getAttributeValidator]] autorelease];
NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:_attributeValidator] autorelease];

if (attributes != nil) {
[contextAdapter addAttributesNewValidation:attributes];
Expand All @@ -287,7 +294,7 @@ - (void) recordHandledException:(NSException*)exception

[self checkOffline:report];

NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:[analyticsParent getAttributeValidator]] autorelease];
NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:_attributeValidator] autorelease];

if (attributes != nil) {
[contextAdapter addAttributes:attributes];
Expand Down Expand Up @@ -371,7 +378,7 @@ - (void) recordHandledExceptionWithStackTrace:(NSDictionary*)exceptionDictionary
report->setAttributeNoValidation("timeSinceLoad", [[[NSDate new] autorelease] timeIntervalSinceDate:self.sessionStartDate]);
[self checkOffline:report];

NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:[analyticsParent getAttributeValidator]] autorelease];
NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:_attributeValidator] autorelease];

if (exceptionDictionary != nil) {
[contextAdapter addAttributesNewValidation:exceptionDictionary];
Expand All @@ -388,7 +395,7 @@ - (void) recordHandledExceptionWithStackTrace:(NSDictionary*)exceptionDictionary
report->setAttribute("timeSinceLoad", [[[NSDate new] autorelease] timeIntervalSinceDate:self.sessionStartDate]);
[self checkOffline:report];

NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:[analyticsParent getAttributeValidator]] autorelease];
NRMAExceptionReportAdaptor* contextAdapter = [[[NRMAExceptionReportAdaptor alloc] initWithReport:report attributeValidator:_attributeValidator] autorelease];

if (exceptionDictionary != nil) {
[contextAdapter addAttributes:exceptionDictionary];
Expand Down
Loading
Loading