BranchMetrics / ios-branch-deep-linking-attribution

The Branch iOS SDK for deep linking and attribution. Branch helps mobile apps grow with deep links / deeplinks that power paid acquisition and re-engagement campaigns, referral programs, content sharing, deep linked emails, smart banners, custom user onboarding, and more.
https://help.branch.io/developers-hub/docs/ios-sdk-overview
MIT License
727 stars 229 forks source link

URI Scheme Returns "null" Because of "pin" Prefix #1341

Open kvenn opened 6 months ago

kvenn commented 6 months ago

Describe the bug

The validator is saying my scheme is invalid because URI schemes prefixed with "pin" are ignored (return null). My app name is "ping".

Is this causing fingerprinting issues?

Steps to reproduce

  1. Set URI Scheme to "ping" in Info.plist
  2. Run validator
  3. It shows uri scheme as invalid. The logger prints it as "null"

Expected behavior

The URI scheme is returned as "ping", not null. I believe this prefix check should be changed to an equals to not break all apps that have the prefix in their scheme.

SDK Version

3.0.1

XCode Version

15.2

Device

iPhone 15 (and all)

OS

17.0.1 (and all)

Additional Information/Context

The bug is here in BNCSystemObserver

if ([uriScheme hasPrefix:@"pin"]) continue; // Pinterest

CodeBlock + (NSString *)defaultURIScheme { NSArray *urlTypes = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"CFBundleURLTypes"]; for (NSDictionary *urlType in urlTypes) { NSArray *urlSchemes = [urlType objectForKey:@"CFBundleURLSchemes"]; for (NSString *uriScheme in urlSchemes) { if ([uriScheme hasPrefix:@"fb"]) continue; // Facebook if ([uriScheme hasPrefix:@"db"]) continue; // DB? if ([uriScheme hasPrefix:@"twitterkit-"]) continue; // Twitter if ([uriScheme hasPrefix:@"pdk"]) continue; // Pinterest if ([uriScheme hasPrefix:@"pin"]) continue; // Pinterest if ([uriScheme hasPrefix:@"com.googleusercontent.apps"]) continue; // Google // Otherwise this must be it! return uriScheme; } } return nil; }
kvenn commented 6 months ago

I've had issues with deferred deep-linking working only 10% of the time on iOS (where it works almost every time on Android). Is it possible this is related?

This code to me suggests that the uri scheme is used in tracking installs (addAppleReceiptSourceToJSON includes uri scheme). I don't get why some still succeed (as opposed to all failing), though.

    // Install and Open
    [self addDeveloperUserIDToJSON:json];
    [self addSystemObserverDataToJSON:json];
    [self addPreferenceHelperDataToJSON:json];
    [self addPartnerParametersToJSON:json];
    [self addAppleReceiptSourceToJSON:json];
    [self addTimestampsToJSON:json];

    [self addAppleAttributionTokenToJSON:json];

    // Install Only
    [self addAppleReceiptDataToJSON:json];

Thank you!

kvenn commented 5 months ago

I can imagine this issue will be deprioritized since it will only affect a small set of companies.

But aside from doing the work, would it be possible to get an answer on if it's likely that this is impacting the unreliability of deferred deep linking? The fact that it succeeds around 40% is the only reason I'm thinking it could be unrelated.

But I don't know how the fingerprinting works.

Thank you!!

echo-branch commented 5 months ago

@kvenn Sorry for the late reply. We've been a bit short of dev resources lately. For support questions it's usually faster to contact our support team via the branch website.

Apple's privacy policy does not allow device fingerprinting, so we do not build or use device fingerprints on Apple platforms. This means we cannot confidently tell similar devices apart when on the same IP and time window and will err on the side of caution and not return Deferred deeplink data. We recommend using NativeLink, https://help.branch.io/faq/docs/nativelink-deferred-deep-linking. It is a pasteboard based deterministic deferred deeplinking solution.

echo-branch commented 5 months ago

@kvenn About the original issue with the prefix check. FB uses a large number of URI schemes prefixed with fb. These URI schemes are not for FB, but rather apps that FB opens via URI scheme. For example, say your app is integrated with FB then they will assign you a URI scheme such as fb123456789 to serve as the URI scheme that FB uses to open your app.

I'll need to double check, but I believe Pinterest and others had a similar designs. Which conflicts with your preferred URI scheme. One option would be to add an API to override this check. I'll log a ticket for it, but it may take a bit before we ship something with this option.

kvenn commented 5 months ago

Thank you for your reply! I really appreciate it.

For deferred deeplinking: Thank you for the insight. That makes perfect sense. Support informed me it's likely that it will be extremely unreliable without nativelink. So it's good to have that corroborated.

For the uri scheme: Is it possible this is causing any issues for us? Or is it just the validator that's failing?

If it's possible it's causing problems, I'd be willing to open a PR to implement an optional override.

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale due to inactivity for 60 days. If this issue is still relevant, please respond with any updates or this issue will be closed in 7 days. If you believe this is a mistake, please comment to let us know. Thank you for your contributions.

kvenn commented 3 months ago

I'm still hoping to get a few answers (I think they should be quick ones!)

For the uri scheme: Is it possible this is causing any issues for us (the fact that 50% of iOS installs aren't attributed)? Or is it just the validator that's failing?

If it's possible it's causing problems, I'd be willing to open a PR to implement an optional override.

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale due to inactivity for 60 days. If this issue is still relevant, please respond with any updates or this issue will be closed in 7 days. If you believe this is a mistake, please comment to let us know. Thank you for your contributions.

kvenn commented 1 month ago

@echo-branch any chance you know if it's possible this is the source of our issues? Or is the check just a "warning".