capacitor icon indicating copy to clipboard operation
capacitor copied to clipboard

bug: Android web view encapsulate 302 redirection response

Open mrkelvinli opened this issue 5 years ago • 18 comments

Bug Report

Capacitor Version

💊   Capacitor Doctor  💊

Latest Dependencies:

  @capacitor/cli: 2.4.6
  @capacitor/core: 2.4.6
  @capacitor/android: 2.4.6
  @capacitor/electron: 2.4.6
  @capacitor/ios: 2.4.6

Installed Dependencies:

  @capacitor/cli 2.4.2
  @capacitor/android 2.4.2
  @capacitor/core 2.4.2
  @capacitor/ios 2.4.2
  @capacitor/electron not installed

Platform(s)

Only on android

Current Behavior

When the android web view navigates to a URL that supposed to returns a 302 redirection response to a different domain, a 200 response was received with the HTML content. So that the web view does not perform any URL redirection. Instead the window URL remain unchanged. Assets (eg. images, CSS, static JS files) with relative URLs in the HTML reference the original window URL rather than the redirected URL. A broken HTML page is shown as a result of missing assets required by the HTML.

Expected Behavior

When navigating to URL which returns a 302 response, the web view should perform the redirection instead of it being resolved by Capacitor. The web view window location should be updated to the redirected location and the assets with relative URL in the HTML should be resolved with the correct URL path

Code Reproduction

Navigating to any URL which returns a 302 redirection response to a different domain produce this issue.

I made a repo to reproduce the issue here: https://github.com/mrkelvinli/capacitor-android-issue-4240-reproduction

Other Technical Details

npm --version output: 6.14.10

node --version output: v14.15.4

pod --version output (iOS issues only):

Additional Context

I did some investigation on this in @capacitor/android

When the web view made a request to the serverUrl domain, Capacitor will proxy the network request from the web view into WebViewLocalServer. If the request in shouldInterceptRequest is pass into handleProxyRequest(), that means it has a PathHandler associated to the hostname of the request. If the request also has text/html in the HTTP accept header, then the HttpURLConnection object which performs the network request will follow redirects by default (as HttpURLConnection::setFollowRedirects() is true by default). Thus, all successfully request will result in a 200 (status code is from the PathHandler associated to the request hostname) response and redirection will be resolved within HttpURLConnection instead of the web view.

Solution I propose

diff --git a/node_modules/@capacitor/android/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.java b/node_modules/@capacitor/android/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.java
index d632065..bc10831 100755
--- a/node_modules/@capacitor/android/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.java
+++ b/node_modules/@capacitor/android/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.java
@@ -311,6 +311,28 @@ public class WebViewLocalServer {
           conn.setRequestMethod(method);
           conn.setReadTimeout(30 * 1000);
           conn.setConnectTimeout(30 * 1000);
+
+          /**
+           * This should be the final step in the setup process of the HttpURLConnection object.
+           *
+           * Initiate the connection with follow redirects disable. It allows us
+           * identifying any redirection response and avoid intercepting such request.
+           * This is because returning a 200 response when a redirection response (ie. 301 or 302)
+           * is expected could result in issues with resolving relative URLs for static assets
+           * in the HTML.
+           */
+          conn.setInstanceFollowRedirects(false);
+          conn.connect();
+          int status = conn.getResponseCode();
+          if (
+            status == HttpURLConnection.HTTP_MOVED_TEMP ||
+              status == HttpURLConnection.HTTP_MOVED_PERM ||
+              status == HttpURLConnection.HTTP_SEE_OTHER
+          ) {
+            // Return null so that this request will not be intercepted.
+            return null;
+          }
+
           String cookie = conn.getHeaderField("Set-Cookie");
           if (cookie != null) {
             CookieManager.getInstance().setCookie(url, cookie);

mrkelvinli avatar Feb 23 '21 02:02 mrkelvinli

This issue may need more information before it can be addressed. In particular, it will need a reliable Code Reproduction that demonstrates the issue.

Please see the Contributing Guide for how to create a Code Reproduction.

Thanks! Ionitron 💙

Ionitron avatar Feb 23 '21 10:02 Ionitron

Hey @jcesarmobile, I have added a reproduction for the issue: https://github.com/mrkelvinli/capacitor-android-issue-4240-reproduction

mrkelvinli avatar Feb 24 '21 05:02 mrkelvinli

@mrkelvinli, you are a lifesaver! Been really stuck on this for some days; your patch works fine. Hope soon to be merged.

sl45sms avatar Mar 06 '21 20:03 sl45sms

Hey @sl45sms,

Glad to hear that it works for you. Hope the ionic team can confirm this soon.

mrkelvinli avatar Mar 08 '21 22:03 mrkelvinli

I'm having the same issue, but some URLs the app can redirect, some cannot. So strange :(

stevenguyen186 avatar Mar 09 '21 09:03 stevenguyen186

I'm having the same issue, but some URLs the app can redirect, some cannot. So strange :(

Hey @stevenguyen186 That's weird. Did my patch works for you? Did you have a custom service worker client installed on the Android side?

mrkelvinli avatar Mar 09 '21 22:03 mrkelvinli

No, it doesn't work :( I don't have any custom service worker client.

stevenguyen186 avatar Mar 10 '21 08:03 stevenguyen186

It's a working method. Thank you! But why I can see no pull request?

Vasek18 avatar Mar 25 '21 16:03 Vasek18

Your patch made my day! Thanks a lot!

TimBroddin avatar Mar 30 '21 13:03 TimBroddin

Ran into this same redirect issue and unless anyone has workaround suggestions going to depend on a fork. Hoping the fix gets merged soon.

tony-scio avatar Aug 24 '21 22:08 tony-scio

I solved this problem another way. For all redirects to another hosts I use InAppBrowser All server redirects work in iab. So I just listen to loadstart event and when host of a new url equals to the host of the app I close the iab and just use location.href for redirect to this last url

Vasek18 avatar Aug 27 '21 10:08 Vasek18

This issue is still present. The fix seems to work out great. Any reasons why the fix was not deployed yet? @Ionitron

@mrkelvinli would you mind opening a PR?

DSergiu avatar Apr 06 '22 13:04 DSergiu

@Ionitron any updates on the status of this bug? What would you need to fix this issue?

DSergiu avatar May 25 '22 10:05 DSergiu

Is there any news when this might be integrated? This is still present in Capacitor 4.

Zacherl avatar Aug 17 '22 14:08 Zacherl

@jcesarmobile @Ionitron Did you already have time to have a look at the linked PR? Any response would be appreciated.

Zacherl avatar Sep 16 '22 11:09 Zacherl

Temporarily you can patch the @capacitor/android source code using patch-package. The steps below work for version 4.2.0. Otherwise you need to generate your own patch.

  1. install yarn add -D patch-package
  2. add package.json scripts: "postinstall": "patch-package"
  3. Create a file patches/@capacitor+android+4.2.0.patch with following content:
diff --git a/node_modules/@capacitor/android/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.java b/node_modules/@capacitor/android/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.java
index c9def87..0f285d9 100644
--- a/node_modules/@capacitor/android/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.java
+++ b/node_modules/@capacitor/android/capacitor/src/main/java/com/getcapacitor/WebViewLocalServer.java
@@ -360,6 +360,20 @@ public class WebViewLocalServer {
                         String base64 = Base64.encodeToString(userInfoBytes, Base64.NO_WRAP);
                         conn.setRequestProperty("Authorization", "Basic " + base64);
                     }
+
+                    // See more: https://github.com/ionic-team/capacitor/issues/4240
+                    conn.setInstanceFollowRedirects(false);
+                    conn.connect();
+                    int status = conn.getResponseCode();
+                    if (status == HttpURLConnection.HTTP_MOVED_TEMP ||
+                            status == HttpURLConnection.HTTP_MOVED_PERM ||
+                            status == HttpURLConnection.HTTP_SEE_OTHER) {
+                        return null;
+                    }
+
                     String cookie = conn.getHeaderField("Set-Cookie");
                     if (cookie != null) {
                         CookieManager.getInstance().setCookie(url, cookie);
  1. Run yarn install

This solution works fine for 302 redirects to external hosts, but does not work for 302 redirects towards your local http://localhost which can happen in case of identity authorize redirects.

DSergiu avatar Sep 16 '22 12:09 DSergiu

@DSergiu Thank you for your feedback! I'll probably use that workaround, but with our own patch (as far as I know, the fix you posted leaves the native bridge broken after redirects).

I'd still like to know if we could help fixing the underlying problem within capacitor.

Zacherl avatar Sep 16 '22 12:09 Zacherl

Hi there, by any chance the issue can be resolved without using workarounds? We what to use keycloak in our capacitor and this is the only issue blocking this.

rengert avatar Sep 19 '22 06:09 rengert

We are facing this issue now to when integration with third party provider.

patrik-skilling avatar Mar 15 '23 14:03 patrik-skilling

I believe that you're having the same issue I came across. My solution is slightly different but I suspect might also solve this issue #6449 .

TomCarey avatar Mar 29 '23 21:03 TomCarey

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.

ionitron-bot[bot] avatar Feb 21 '24 14:02 ionitron-bot[bot]