Allow Bundle IDs that have a valid prefix - strict validation #2515
Conversation
This allows iOS Extensions that have a Bundle ID of the format `com.google.appName.extension`
Firebase/Core/FIRBundleUtil.m
Outdated
| return YES; | ||
| } | ||
| } | ||
| return NO; | ||
| } | ||
|
|
||
| + (NSString *)bundleIdentifierByRemovingLastPartFrom:(NSString *)bundleIdentifier { | ||
| NSString *bundleIdComponentsSeparator = @"."; |
There was a problem hiding this comment.
Nit: bundleID instead of Id here and below.
Firebase/Core/FIRBundleUtil.m
Outdated
| return @""; | ||
| } | ||
|
|
||
| NSMutableArray<NSString *> *mutableNundleIdComponents = [bundleIdComponents mutableCopy]; |
There was a problem hiding this comment.
Nit: s/Nundle/Bundle/ here and below.
| @@ -45,8 +45,8 @@ | |||
| + (NSArray *)relevantURLSchemes; | |||
|
|
|||
| /** | |||
| * Checks if the bundle identifier exists in the given bundles. | |||
| * Checks the given bundles to see of them is a prefix of the given identifier. | |||
There was a problem hiding this comment.
Maybe "Checks if any of the given bundles have a matching bundle identifier prefix (removing extension suffixes)."?
| @@ -29,6 +29,7 @@ Firebase Core includes FIRApp and FIROptions which provide central configuration | |||
| s.private_header_files = 'Firebase/Core/Private/*.h' | |||
| s.framework = 'Foundation' | |||
| s.dependency 'GoogleUtilities/Logger', '~> 5.2' | |||
| s.dependency 'GoogleUtilities/Environment', '~> 5.2' | |||
There was a problem hiding this comment.
@ryanwilson Not sure we catch missing dependencies in our tests...
There was a problem hiding this comment.
Nice catch - I'm not sure why this wouldn't be caught by pod lib lint - @paulb777 any ideas?
There was a problem hiding this comment.
Because it's always a transitive dep - https://github.com/firebase/firebase-ios-sdk/blob/master/GoogleUtilities.podspec#L38
There was a problem hiding this comment.
Makes sense! Can you please move it up one to be above Logger (alphabetical order)?
| @@ -29,6 +29,7 @@ Firebase Core includes FIRApp and FIROptions which provide central configuration | |||
| s.private_header_files = 'Firebase/Core/Private/*.h' | |||
| s.framework = 'Foundation' | |||
| s.dependency 'GoogleUtilities/Logger', '~> 5.2' | |||
| s.dependency 'GoogleUtilities/Environment', '~> 5.2' | |||
There was a problem hiding this comment.
Makes sense! Can you please move it up one to be above Logger (alphabetical order)?
This is an updated version of #2430 to address comments: