Firing a deep link with an unknown pathPrefix launches airbnb app about deeplinkdispatch HOT 13 CLOSED

I think the problem is that the generated notifyListener() method broadcasts an intent with an action equal to com.airbnb.deeplinkdispatch.DEEPLINK_ACTION. Because the intent namespace is global, any app can respond to that broadcast. On @MichaelGoj's phone, probably Airbnb is the only app that does so.

A possible fix would be to add the app's package to the action of the generated intent instead of always having it be com.airbnb.deeplinkdispatch.DEEPLINK_ACTION. This would make it more difficult to accidentally launch a different app. The action could even be randomly generated or contain some random characters, although I'm not sure that level of obscurity is necessary.

What do you think @felipecsl? Is this a change that would be accepted?

from deeplinkdispatch.

@scompt is correct, the implicit intent can be handled by any app, in this case the Airbnb app is handling it, which is launching the home screen. This is an interesting unintended behavior, of course.
I think adding the app package name to the action is a good idea, it would definitely be accepted.

from deeplinkdispatch.

That seems sorta tricky, because the annotation processor can't use tricks like BuildConfig.APPLICATION_ID to determine the package name for the intent action, since it's not an Android project. There might be a way to do it like this:

private PackageElement getPackageInfo(Element element) {
    PackageElement packageElement = null;
    Element parentElement = element.getEnclosingElement();
    while (parentElement != null) {
      if (parentElement instanceof PackageElement) {
        packageElement = (PackageElement) parentElement;
        break;
      }
      parentElement = parentElement.getEnclosingElement();
    }
    return packageElement;
  }

to crawl up the tree to get the PackageElement of one of the annotations (to pick the package identifier for the intent action) but it's possible that you'll have elements in different packages - so I'm not sure how you'd decide which one to use as the ACTION.

Suggestions @felipecsl ? Maybe I'm going about this in a more complex way, but I'd be glad to put up a change if we can come up with a way forward.

from deeplinkdispatch.

We could allow the application subclass to be annotated with @DeepLinkDispatch to provide library-wide configuration options, such as the name of the action. This would make it possible to have different action names for different application variants, which might be useful if you, for example, have a debug and release build of the same app installed on your device.

from deeplinkdispatch.

@scompt that's an interesting approach.
@MichaelEvans thanks for digging into this, but it indeed sounds like an overcomplicated solution.

I like @scompt's approach because it could be extended in the future with more application-wide configurations.

from deeplinkdispatch.

👍 I have the same issue.

What about using LocalBroadcastManager instead of global sendBroadcast?
http://developer.android.com/reference/android/support/v4/content/LocalBroadcastManager.html

from deeplinkdispatch.

@jeanbaptistemorin I believe @MichaelEvans is working on it and should have a PR soon, amirite @MichaelEvans ?

from deeplinkdispatch.

@jeanbaptistemorin I tried exactly your suggested approach with using a LocalBroadcastManager couple of hours ago MichaelGoj@0054a71 I am still not sure about registering (without unregistering) a BroadcastReceiver within a custom Application class but I see no problems with doing it this way. Maybe I am missing sth. What do you think @felipecsl ? If this approach is also fine, I can create a PR with an updated README as well.

from deeplinkdispatch.

But what I am still trying to understand is: What are the best practices if you, let's say you register one Activity with @DeepLink("http://www.example.com/someLink") and in AndroidManifest you use <data android:scheme="http" android:host="www.example.com" /> and then you click on some link (which is not registered) outside of your app which has the following url http://www.example.com/someOtherLink Android is asking me then for example if I want to open it with the App or with a Browser, I choose the App and then what? Start an Activity with Intent.ACTION_VIEW and Android will ask me the same with which app i want to open this link (maybe exclude the app from the chooser). Actually what I want is that the app should not appear in the chooser right from the beginning. Therefore I also have to add /someLink as a path to the data element, too. And I would do that for all other registered DeepLinks within the App, too and the meaning of using that custom BroadcastReceiver just for the rare cases e.g. if one of the Exceptions is thrown. If this is also the way you use this library, then the README should also mention this in a way.

from deeplinkdispatch.

@MichaelGoj, as the source code of the LocalBroadcastReceiver reveals that it's a singleton build upon the Application context, I think it's ok to register a Receiver in onCreate of the Application and not call unregister, as the LocalBroadcastReceiver will be destroyed as soon as the Application gets killed. In that way your PR looks ok to me.

About the usage, I think that some apps needs to have a fallback behaviour to avoid that nothing opens when being redirecting on a deep-link. Especially when using a custom scheme.
If the manifest triggers the deep-links and the usage of the DeepLinkDispatch does not, it's not good, but you need a kind of security:
For example if you publish an App v1 that handles a format of your deep-links and that in the v2 the format changed a bit but still matches the pathPattern of the v1, I think you want that something happens even if the new format is not recognized on a older version of the app.

The best practise to me is to reduce the HTTP(s) deep-links scope in the manifest to match only what is really handled (pathPatternis nice for that). And the fallback is using to handle all the edge cases.

But for internal product reasons custom schemes for deep-links should always match all the deeplinks (myexample://someLink) and handle the errors/malformed url inside the app. The fallback behaviour of the local broadcast can make more sense in that case.

from deeplinkdispatch.

Thanks @jeanbaptistemorin for this really helpful input.

from deeplinkdispatch.

Yep, just adding on to what @jeanbaptistemorin said, we use it mostly for custom scheme deep links instead of http ones. In that case you want a fallback for cases where none of your activities matched your URI. That's the specific use case we're trying to address

from deeplinkdispatch.

Fixed in #56

from deeplinkdispatch.