[Bug]: Android's WebViewCompat.addWebMessageListener fails when navigating to a local website with http scheme

martin-braun commented 2 months ago

Capacitor Version

Capacitor Doctor 💊

Latest Dependencies:

@capacitor/cli: 6.1.1 @capacitor/core: 6.1.1 @capacitor/android: 6.1.1 @capacitor/ios: 6.1.1

Installed Dependencies:

@capacitor/ios: not installed @capacitor/core: 6.1.1 @capacitor/android: 6.1.1 @capacitor/cli: 6.1.1

[success] Android looking great! 👌

Other API Details

`npm --version`: `10.8.1`
`node --version`: `v22.4.1`

Test device: Android 11

Platforms Affected

[ ] iOS
[X] Android
[ ] Web

Current Behavior

When your app loads a website on the local network, such as http://192.168.77.166:8080 this will open a new browser until allowNavigation is supplied with 192.168.77.166 in the Capacitor configuration. When using the legacy bridge useLegacyBridge: true the MessageHandler would use webView.addJavascriptInterface to inject the window.androidBridge object into the remote website, so that the platform is recognized as android and the remote website would be able to communicate with the native plugins of Capactor, see:

https://github.com/ionic-team/capacitor/blob/e0f299daaa112fa3bcea81ae539983756f1f7304/android/capacitor/src/main/java/com/getcapacitor/MessageHandler.java#L41

If the useLegacyBridge: false (default behavior), the MessageHandler will attempt to use WebViewCompat.addWebMessageListener to inject the androidBridge object:

https://github.com/ionic-team/capacitor/blob/e0f299daaa112fa3bcea81ae539983756f1f7304/android/capacitor/src/main/java/com/getcapacitor/MessageHandler.java#L36

This however isn't working if the target website has http:// instead of https:// scheme, thus androidBridge is null after navigating to unsafe remote websites.

WebViewCompat.addWebMessageListener explicitly states:

Each allowedOriginRules entry must follow the format SCHEME "://" [ HOSTNAME_PATTERN [ ":" PORT ] ]

So, Capacitor requires allowNavigation to be a list of domains without scheme, such as example.org and you then make bridge.getAllowedOriginRules() return an array of such list with the proper scheme, which is https:// always. This is the wrong behavior on local websites that don't use SSL for obvious reasons.

Even when doing useLegacyBridge: false or changing the code to prefix http:// instead, native plugins will still not work, until the domain is specified with a port another time within the allowNavigation list. Otherwise the pattern matching will fail and, despite the androidBridge not being null, won't have an implementation for the platform and report so.

I was able to bypass both issues by having this in my configuration:

  "server": {
    "allowNavigation": [
      "192.168.77.166",
      "192.168.77.166:8080",
      "http://192.168.77.166:8080"
    ]
  }

Now window.androidBridge is not null on the network website and my native plugins work.

Expected Behavior

We need a better way here. This was confusing, wasted a lot of time, is not documented. Providing http://192.168.0.80:8080 should be enough to satisfy all rules. I was going crazy testing this. There is no documentation explaining this.

Project Reproduction

Additional Information

I cannot share my private project, you should be able to reproduce with:

{
  "appId": "org.example.wrapper",
  "appName": "Example Navigation Wrapper",
  "bundledWebRuntime": false,
  "npmClient": "npm",
  "webDir": "mobile",
  "android": {
    "allowMixedContent": true,
    "hideLogs": false,
    "webContentsDebuggingEnabled": true
  },
  "plugins": {
    "SplashScreen": {
      "launchShowDuration": 0
    }
  },
  "cordova": {},
  "server": {
    "allowNavigation": [
      "192.168.77.166",
    ]
  }
}

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Test</title>
  </head>
  <body>
    <script type="text/javascript" src="mobile.js"></script>
  </body>
</html>

// mobile.js
window.location.href = "http://192.168.77.166"

Use your own network IP and host a little web server on 0.0.0.0:8080 to see how it's not working, until you add all the allowNavigation entries.

martin-braun commented 2 months ago

I have tested my above work around on an Android 7 POS device and this causes a crash of the app:

java.lang.RuntimeException: Unable to start activity ComponentInfo{..../.....MainActivity}: java.lang.IllegalArgumentException: allowedOriginRules https://192.168.77.166 is invalid

It seems older Android WebView's were more sensitive about allowOriginRules and didn't just ignore invalid rules.

Another reason why I think Capacitor isn't explicit enough about those allowOriginRules. The configuration should allow me to tell that my entry is http, instead of https and prevent those prefixes from being part of the URL set:

Look at this breakpoint when the stuff is passed to the WebView within the bridge (192.168.77.166 in this example). If Capacitor would allow me to tell what's https and whats not.

I hate this black box magic. All I want is testing my Capacitor app with a locally hosted website, I wish this was actually tested.

jcesarmobile commented 2 months ago

closing as duplicate of https://github.com/ionic-team/capacitor/issues/7454

martin-braun commented 2 months ago

@jcesarmobile Please re-open. It is NOT a duplicate of #7454 and I have fixed #7454 in #7571, but the issue I am talking about here is still present despite fixing #7454.

My devices return false on WebViewFeature.isFeatureSupported(WebViewFeature.DOCUMENT_START_SCRIPT), thus they don't leverage the WebViewCompat.addDocumentStartJavaScript method what #7454 is all about, instead it keeps the jsInjector and passes it to the constructor of WebViewLocalServer to fall back to the old way of injecting code. The issue you claim to be a duplicate isn't even an issue on my older devices.

This issue here is about the fact that you rice all allowNavigation with a https:// prefix before passing it to WebViewCompat.addWebMessageListener which consequently breaks all apps that try to work with http-addresses such as local network addresses. We need more control over this in the configuration.

martin-braun commented 2 months ago

@jcesarmobile @mlynch Sorry for pinging (again), but I have to ask, is this a bad joke or common practice? Yes, I'm afraid I'm starting to get a bit salty right now, because it feels like my research and contribution regarding this (and the other issue) is not appreciated.

Let's try to keep it professional. Why is my above comment marked "abuse" and why is it being ignored? I have detailed why this issue is different from the issue that is marked as duplicate.

When I had to deal with these issues mentioned here and there, I took time out of my day to make a PR and report the ultimate issue that caused things to malfunction for me (this one). In my opinion it was triaged the wrong way and is not addressed now anymore.

Why do I try to give something back by being transparent about the issues I'm facing again? Should I just monkey patch and let the framework speak for itself?

Sorry, I'm just disappointed.

ionitron-bot[bot] commented 1 month ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.

ionic-team / capacitor