Skip to content

Conversation

@eldhosembabu
Copy link
Contributor

@eldhosembabu eldhosembabu commented Sep 20, 2021

[FDL] Refactoring and fixing url validation

  • Refactored the existing code to use Regex to validate FDL URL formats.

  • Added checks to make sure the 'link' query param is having an http/https prefix input.
    == This is going to be breaking the existing SDK's unintended behavior to support links with non http/https content. The existing SDK was constructing the Dynamic link eventhough the backend response to resolve the link is not successfull.

  • Added support for URL(s) with Query string starting with '?' alone. Previously it was supporting '/?' formats only. This fixes 177469304: Firebase dynamic links with custom domain will only work if the custom domai has a trailing '/' #7087

  • Added more test cases to be in sync with FDL backend validation.

  • Removed invalid test inputs and corrected few test cases to make the input in proper format.

@google-oss-bot
Copy link

google-oss-bot commented Sep 20, 2021

Coverage Report

Affected SDKs

  • FirebaseDynamicLinks-iOS-FirebaseDynamicLinks.framework

    SDK overall coverage changed from 60.16% (959313e) to 60.41% (4ae84bd) by +0.25%.

    Filename Base (959313e) Head (4ae84bd) Diff
    FDLUtilities.m 71.23% 72.96% +1.73%

Test Logs

@google-oss-bot
Copy link

google-oss-bot commented Sep 20, 2021

Binary Size Report

Affected SDKs

  • FirebaseDynamicLinks

    Type Base (959313e) Head (4ae84bd) Diff
    firebase-ios-sdk ? 403 kB ? (?)

Test Logs

@eldhosembabu eldhosembabu requested review from paulb777 and ryanwilson and removed request for paulb777 September 20, 2021 22:41
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, since we don't yet have Xcode 13 in CI pending #8665, please confirm that the PR builds and tests with Xcode 13.

Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Corrected TYPO
added more test inputs.
@eldhosembabu
Copy link
Contributor Author

#8665

Also, since we don't yet have Xcode 13 in CI pending #8665, please confirm that the PR builds and tests with Xcode 13.

I was doing my testing in XCode 13 Beta 5 only

@eldhosembabu
Copy link
Contributor Author

Checked on xCode 13 and it is working.

@eldhosembabu eldhosembabu merged commit 85864c6 into master Sep 22, 2021
@eldhosembabu eldhosembabu deleted the custom-domain-issue-7087 branch September 22, 2021 21:21
@firebase firebase locked and limited conversation to collaborators Oct 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

177469304: Firebase dynamic links with custom domain will only work if the custom domai has a trailing '/'

5 participants