apache / cordova-android

Apache Cordova Android
https://cordova.apache.org/
Apache License 2.0
3.65k stars 1.54k forks source link

[Feature Request]: Improve error feedback to webview from native #1435

Open tudordumitriu opened 2 years ago

tudordumitriu commented 2 years ago

Bug Report

After upgrading to Cordova Android 10.1.1 the login stopped working in release mode. The login being a call to a remote API on https that returns a Set-Cookie header.

Problem

First of all, the very same call to the very same API in debug deployment works perfectly After days of trial and error, first of all trying to attach to a release deployment with an inspector to see the console warning, that finally didn't help at all we found out that the problem was caused by a SSL error (still don't know what it is) that was handled differently in debug mode

What is expected to happen?

To work in both release and debug mode the same

Information

SystemWebViewClient onReceivedSslError method treats the SSL errors differently in debug mode and makes calls being handled differently

Command or Code

Our temporary solution was to always call handler.proceed() if (true || (appInfo.flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0) { // debug = true handler.proceed(); return; } else { // debug = false super.onReceivedSslError(view, handler, error); }

Environment, Platform, Device

Cordova Android 10.1.1

Version information

Cordova CLI 11 Windows Android Studio Chipmunk

Checklist

breautek commented 2 years ago

SSL errors are generally caused by:

  1. Missing intermediate certificate on the local device (e..g the webserver does not supply the full certificate chain)
  2. There is a mismatch of supported encryption algorithms and/or ciphers between the device and the server. This is often the case with older phones or phones that isn't up-to-date with their updates. Sometimes in order to support older devices, you need to use/enable weaker, potentially insecure SSL settings on the server. Of course, this becomes a risk vs value assessment.

SSL Labs offers a great testing tool for the server to test your server's configuration, including potential security vulnerabilities, as well as generally supported browsers / devices.

SystemWebViewClient onReceivedSslError method treats the SSL errors differently in debug mode and makes calls being handled differently

I think I agree with this sentiment. Relaxed SSL errors are often saught upon for local development, so that you can run self-signed certificates for example, but maybe this should be either an opt-in or opt-out kind of feature.

tudordumitriu commented 2 years ago

Thanks @breautek We have just figured it out, the intermediate certificate had to be installed in the API server services properly. All fine once we have correctly done that So the underlying issue fixed, but the secondary problem persists, how can you get properly informed on the nature of the real issue, could these maybe be forwarded to console, at least to get an idea what the problem might be. In my case I ended up commenting out that flag just because there were only few places and since in debug was working, well, we were trying to eliminate one by one the potential issues, but in the end this consumed us few days, and I guess other people might be facing this By all means feel free to close it if there is no other way to elevate these errors.

breautek commented 2 years ago

could these maybe be forwarded to console

It's possible they are printed in the native console, but yah, maybe native logs using the cordova logger could be printed to the webview console. That could simplify a lot of debugging for many people, especially if they aren't familiar with using native tools, as with the case with many cordova users.

By all means feel free to close it if there is no other way to elevate these errors.

I think we can probably keep this open for context, but I'll reword the title. My idea for changing how SSL is accepted is a bit more of a dangerous territory. Forwarding console logs is definitely a doable feature I think that can be added at any time, and definitely safer I think.

If you don't like the new title, feel free to adjust it. Naming things can be hard sometimes :laughing:

tudordumitriu commented 2 years ago

I think the title is comprehensive and thanks for all the support!