Fix for issue where matchesshortlinkformat was returning true for all…

… links. (#2563) * Fix issue with matchesShortLinkFormat always returning true. * Fix style.
firebase · Mar 19, 2019 · 3c3fe79 · 3c3fe79
1 parent 6cd4f44
commit 3c3fe79
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 22 deletions.
diff --git a/Example/DynamicLinks/Tests/FIRDynamicLinksTest.m b/Example/DynamicLinks/Tests/FIRDynamicLinksTest.m
@@ -77,6 +77,7 @@ - (BOOL)setUpWithLaunchOptions:(nullable NSDictionary *)launchOptions
                       clientID:(NSString *)clientID
                      urlScheme:(nullable NSString *)urlScheme
                   userDefaults:(nullable NSUserDefaults *)userDefaults;
+- (BOOL)canParseUniversalLinkURL:(nullable NSURL *)url;
 @end
 
 @interface FakeShortLinkResolver : FIRDynamicLinkNetworking
@@ -774,13 +775,90 @@ - (void)testUniversalLinkWithSubdomain_DeepLinkWithParameters {
   XCTAssertEqualObjects(dynamicLink.url.absoluteString, parsedDeepLinkString);
 }
 
-- (void)testMatchesUnversalLinkWithLongDurableLink {
-  NSString *urlString =
-      @"https://sample.page.link?link=https://google.com/test&ibi=com.google.sample&ius=79306483";
-  NSURL *url = [NSURL URLWithString:urlString];
-  BOOL matchesShort = [self.service matchesShortLinkFormat:url];
+- (void)testMatchesShortLinkFormat {
+  NSArray<NSString *> *urlStrings =
+      @[ @"https://test.app.goo.gl/xyz", @"https://test.app.goo.gl/xyz?link=" ];
+
+  for (NSString *urlString in urlStrings) {
+    NSURL *url = [NSURL URLWithString:urlString];
+    BOOL matchesShortLinkFormat = [self.service matchesShortLinkFormat:url];
+
+    XCTAssertTrue(matchesShortLinkFormat,
+                  @"Non-DDL domain URL matched short link format with URL: %@", url);
+  }
+}
+
+// Custom domain entries in plist file:
+//  https://google.com
+//  https://google.com/one
+//  https://a.firebase.com/mypath
+- (void)testFailMatchesShortLinkFormatForCustomDomains {
+  NSArray<NSString *> *urlStrings = @[
+    @"https://google.com",
+    @"https://google.com?link=",
+    @"https://a.firebase.com",
+    @"https://a.firebase.com/mypath?link=",
+  ];
+
+  for (NSString *urlString in urlStrings) {
+    NSURL *url = [NSURL URLWithString:urlString];
+    BOOL matchesShortLinkFormat = [self.service matchesShortLinkFormat:url];
+
+    XCTAssertFalse(matchesShortLinkFormat,
+                   @"Non-DDL domain URL matched short link format with URL: %@", url);
+  }
+}
+
+// Custom domain entries in plist file:
+//  https://google.com
+//  https://google.com/one
+//  https://a.firebase.com/mypath
+- (void)testPassMatchesShortLinkFormatForCustomDomains {
+  NSArray<NSString *> *urlStrings = @[
+    @"https://google.com/xyz", @"https://google.com/xyz/?link=", @"https://google.com/xyz?link=",
+    @"https://google.com/one/xyz", @"https://google.com/one/xyz?link=",
+    @"https://google.com/one?utm_campaignlink=", @"https://google.com/mylink",
+    @"https://google.com/one/mylink", @"https://a.firebase.com/mypath/mylink"
+  ];
+
+  for (NSString *urlString in urlStrings) {
+    NSURL *url = [NSURL URLWithString:urlString];
+    BOOL matchesShortLinkFormat = [self.service matchesShortLinkFormat:url];
+
+    XCTAssertTrue(matchesShortLinkFormat,
+                  @"Non-DDL domain URL matched short link format with URL: %@", url);
+  }
+}
 
-  XCTAssertFalse(matchesShort, @"Long Durable Link should not match short link format");
+- (void)testPassMatchesShortLinkFormat {
+  NSArray<NSString *> *urlStrings = @[
+    @"https://test.app.goo.gl/xyz",
+    @"https://test.app.goo.gl/xyz?link=",
+  ];
+
+  for (NSString *urlString in urlStrings) {
+    NSURL *url = [NSURL URLWithString:urlString];
+    BOOL matchesShortLinkFormat = [self.service matchesShortLinkFormat:url];
+
+    XCTAssertTrue(matchesShortLinkFormat,
+                  @"Non-DDL domain URL matched short link format with URL: %@", url);
+  }
+}
+
+- (void)testFailMatchesShortLinkFormat {
+  NSArray<NSString *> *urlStrings = @[
+    @"https://test.app.goo.gl", @"https://test.app.goo.gl?link=", @"https://test.app.goo.gl/",
+    @"https://sample.page.link?link=https://google.com/test&ibi=com.google.sample&ius=79306483"
+    @"https://sample.page.link/?link=https://google.com/test&ibi=com.google.sample&ius=79306483"
+  ];
+
+  for (NSString *urlString in urlStrings) {
+    NSURL *url = [NSURL URLWithString:urlString];
+    BOOL matchesShortLinkFormat = [self.service matchesShortLinkFormat:url];
+
+    XCTAssertFalse(matchesShortLinkFormat,
+                   @"Non-DDL domain URL matched short link format with URL: %@", url);
+  }
 }
 
 - (void)testMatchesUnversalLinkWithShortDurableLink {
@@ -1073,19 +1151,26 @@ - (void)testValidCustomDomainNames {
   NSArray<NSString *> *urlStrings = @[
     @"https://google.com/mylink",             // Short FDL starting with 'https://google.com'
     @"https://google.com/one",                // Short FDL starting with 'https://google.com'
-    @"https://google.com/?link=abcd",         // Long FDL starting with  'https://google.com'
-    @"https://google.com/one/mylink",         // Long FDL starting with  'https://google.com/one'
+    @"https://google.com/one/mylink",         // Short FDL starting with  'https://google.com/one'
     @"https://a.firebase.com/mypath/mylink",  // Short FDL starting https://a.firebase.com/mypath
+  ];
+
+  NSArray<NSString *> *longFDLURLStrings = @[
     @"https://a.firebase.com/mypath/?link=abcd&test=1",  // Long FDL starting with
                                                          // https://a.firebase.com/mypath
+    @"https://google.com/?link=abcd",  // Long FDL starting with  'https://google.com'
   ];
-
   for (NSString *urlString in urlStrings) {
     NSURL *url = [NSURL URLWithString:urlString];
     BOOL matchesShortLinkFormat = [self.service matchesShortLinkFormat:url];
 
-    XCTAssertTrue(matchesShortLinkFormat,
-                  @"Non-DDL domain URL matched short link format with URL: %@", url);
+    XCTAssertTrue(matchesShortLinkFormat, @"URL did not validate as short link: %@", url);
+  }
+  for (NSString *urlString in longFDLURLStrings) {
+    NSURL *url = [NSURL URLWithString:urlString];
+    BOOL matchesLongLinkFormat = [self.service canParseUniversalLinkURL:url];
+
+    XCTAssertTrue(matchesLongLinkFormat, @"URL did not validate as long link: %@", url);
   }
 }
 

diff --git a/Firebase/DynamicLinks/Utilities/FDLUtilities.m b/Firebase/DynamicLinks/Utilities/FDLUtilities.m
@@ -211,12 +211,11 @@ BOOL FIRDLIsURLForWhiteListedCustomDomain(NSURL *_Nullable URL) {
                                      options:NSCaseInsensitiveSearch | NSAnchoredSearch]
                .location) == 0) {
         // The (short) URL needs to be longer than the domainURIPrefix, it's first character after
-        // the domainURIPrefix needs to be '/' or '?' and should be followed by at-least one more
+        // the domainURIPrefix needs to be '/' and should be followed by at-least one more
         // character.
         if (urlStr.length > domainURIPrefixStr.length + 1 &&
-            ([urlStr characterAtIndex:domainURIPrefixStr.length] == '/' ||
-             [urlStr characterAtIndex:domainURIPrefixStr.length] == '?')) {
-          // Check if there are any more '/' after the first '/' or '?' trailing the
+            ([urlStr characterAtIndex:domainURIPrefixStr.length] == '/')) {
+          // Check if there are any more '/' after the first '/'trailing the
           // domainURIPrefix. This does not apply to unique match links copied from the clipboard.
           // The clipboard links will have '?link=' after the domainURIPrefix.
           NSString *urlWithoutDomainURIPrefix =
@@ -249,17 +248,17 @@ BOOL FIRDLCanParseUniversalLinkURL(NSURL *_Nullable URL) {
 }
 
 BOOL FIRDLMatchesShortLinkFormat(NSURL *URL) {
-  // Short Durable Link URLs always have a path, except for certain custom domain URLs e.g.
-  // 'https://google.com?link=abcd' will not have a path component.
-  // FIRDLIsURLForWhiteListedCustomDomain implicitely checks for path component in custom domain
-  // URLs.
-  BOOL hasPath = URL.path.length > 0 || FIRDLIsURLForWhiteListedCustomDomain(URL);
+  // Short Durable Link URLs always have a path.
+  BOOL hasPath = URL.path.length > 0;
+  BOOL matchesRegularExpression =
+      ([URL.path rangeOfString:@"/[^/]+" options:NSRegularExpressionSearch].location != NSNotFound);
   // Must be able to parse (also checks if the URL conforms to *.app.goo.gl/* or goo.gl/app/*)
-  BOOL canParse = FIRDLCanParseUniversalLinkURL(URL);
+  BOOL canParse = FIRDLCanParseUniversalLinkURL(URL) | FIRDLIsURLForWhiteListedCustomDomain(URL);
+  ;
   // Path cannot be prefixed with /link/dismiss
   BOOL isDismiss = [[URL.path lowercaseString] hasPrefix:@"/link/dismiss"];
 
-  return hasPath && !isDismiss && canParse;
+  return hasPath && matchesRegularExpression && !isDismiss && canParse;
 }
 
 NSString *FIRDLMatchTypeStringFromServerString(NSString *_Nullable serverMatchTypeString) {