GH-359: Fix beforeload to work with POST requests (#367)

### Platforms affected iOS and Android ### What does this PR do? Fixes the behaviour of `beforeload` to resolve the problem with POST requests outlined in #359. The `beforeload` parameter has been changed from taking only a boolean (`yes` or not defined) to a discrete string with possible values of `get`, `post`, or `yes` which correspond to request types of GET, POST or GET&POST respectively. The `README.md` has been updated to reflect this. Note that use of `beforeload` to intercept POST requests is currently not supported on Android or iOS, so if `beforeload=yes` is specified and a POST request is detected as the HTTP request method, `beforeload` behaviour will not be applied. If `beforeload=post` is specified, a `loaderror` event will be dispatched which states that POST requests are not yet supported. #### Notes for Android The `shouldOverrideUrlLoading()` override method has been updated to support the [new method interface added in API 24 / Android 7][1] which receives the `WebResourceRequest` instead of just the `String url`, enabling the HTTP method of the request to be determined. The [deprecated method interface][2] has also been preserved for API <=23, but in this case the HTTP method cannot be determined so is passed as null. Also note that due to a [Chromium bug](https://bugs.chromium.org/p/chromium/issues/detail?id=155250), `shouldOverrideUrlLoading()` is currently not called for POST requests. It's possible this may be resolved in a future Chromium version in the Android System Webview (given that this is now self-updating and independent of Android version since Android 5.0) - in prospective anticipation of this, code to handle POST requests has been added to `shouldOverrideUrlLoading()`. However, it seems more likely that this won't be resolved any time soon given that [a Chromium dev said](https://bugs.chromium.org/p/chromium/issues/detail?id=155250#c39): > We're looking at implementing a better way to handle request interception in a future OS version. There's no way to just "fix" this, the API doesn't accommodate this usage at all. This will not be something you can use any time soon. Therefore if we want to go ahead and use `beforeload` to intercept request types other than GET, it's likely we'll instead need to use the `shouldInterceptRequest()` method override. As with `shouldOverrideUrlLoading()`, there are a two variants: the [new method interface][3] added in API 21 / Android 5.0 which which receives the `WebResourceRequest` object and the [deprecated one][4] which receives only `String url`. If we want to determine the HTTP request method, we'll need to use the new implementation. This has been empirically tested and *is* called for POST requests so would allow the possibility to intercept, delay, modify and send the POST request and its data via `beforeload`. Both `shouldInterceptRequest()` method interfaces have been exposed in the Android implentation for potential future use but they currently do nothing other than return the unadulterated request object. ### What testing has been done on this change? Manual testing of POST and GET requests on both platforms using a test app container: https://github.com/dpa99c/cordova-plugin-inappbrowser-test [1]: https://developer.android.com/reference/android/webkit/WebViewClient.html#shouldOverrideUrlLoading(android.webkit.WebView,%20android.webkit.WebResourceRequest) [2]: https://developer.android.com/reference/android/webkit/WebViewClient.html#shouldOverrideUrlLoading(android.webkit.WebView,%20java.lang.String) [3]: https://developer.android.com/reference/android/webkit/WebViewClient.html#shouldInterceptRequest(android.webkit.WebView,%20android.webkit.WebResourceRequest) [4]: https://developer.android.com/reference/android/webkit/WebViewClient.html#shouldInterceptRequest(android.webkit.WebView,%20java.lang.String)
apache · Dec 20, 2018 · 632a395 · 632a395
1 parent c54d100
commit 632a395
Show file tree

Hide file tree

Showing 8 changed files with 199 additions and 42 deletions.
diff --git a/README.md b/README.md
@@ -106,7 +106,7 @@ instance, or the system browser.
     Android supports these additional options:
 
     - __hidden__: set to `yes` to create the browser and load the page, but not show it. The loadstop event fires when loading is complete. Omit or set to `no` (default) to have the browser open and load normally.
-    - __beforeload__: set to `yes` to enable the `beforeload` event to modify which pages are actually loaded in the browser.
+    - __beforeload__: set to enable the `beforeload` event to modify which pages are actually loaded in the browser. Accepted values are `get` to intercept only GET requests, `post` to intercept on POST requests or `yes` to intercept both GET & POST requests. Note that POST requests are not currently supported and will be ignored (if you set `beforeload=post` it will raise an error).
     - __clearcache__: set to `yes` to have the browser's cookie cache cleared before the new window is opened
     - __clearsessioncache__: set to `yes` to have the session cookie cache cleared before the new window is opened
     - __closebuttoncaption__: set to a string to use as the close button's caption instead of a X. Note that you need to localize this value yourself.
@@ -130,7 +130,7 @@ instance, or the system browser.
 
     - __usewkwebview__: set to `yes` to use WKWebView engine for the InappBrowser. Omit or set to `no` (default) to use UIWebView. Note: Using `usewkwebview=yes` requires that a WKWebView engine plugin be installed in the Cordova project (e.g. [cordova-plugin-wkwebview-engine](https://github.com/apache/cordova-plugin-wkwebview-engine) or [cordova-plugin-ionic-webview](https://github.com/ionic-team/cordova-plugin-ionic-webview)).
     - __hidden__: set to `yes` to create the browser and load the page, but not show it. The loadstop event fires when loading is complete. Omit or set to `no` (default) to have the browser open and load normally.
-    - __beforeload__: set to `yes` to enable the `beforeload` event to modify which pages are actually loaded in the browser.
+    - __beforeload__: set to enable the `beforeload` event to modify which pages are actually loaded in the browser. Accepted values are `get` to intercept only GET requests, `post` to intercept on POST requests or `yes` to intercept both GET & POST requests. Note that POST requests are not currently supported and will be ignored (if you set `beforeload=post` it will raise an error).
     - __clearcache__: set to `yes` to have the browser's cookie cache cleared before the new window is opened
     - __clearsessioncache__: set to `yes` to have the session cookie cache cleared before the new window is opened. For WKWebView, requires iOS 11+ on target device.
     - __cleardata__: set to `yes` to have the browser's entire local storage cleared (cookies, HTML5 local storage, IndexedDB, etc.) before the new window is opened
@@ -212,7 +212,7 @@ The object returned from a call to `cordova.InAppBrowser.open` when the target i
   - __loadstop__: event fires when the `InAppBrowser` finishes loading a URL.
   - __loaderror__: event fires when the `InAppBrowser` encounters an error when loading a URL.
   - __exit__: event fires when the `InAppBrowser` window is closed.
-  - __beforeload__: event fires when the `InAppBrowser` decides whether to load an URL or not (only with option `beforeload=yes`).
+  - __beforeload__: event fires when the `InAppBrowser` decides whether to load an URL or not (only with option `beforeload` set).
   - __message__: event fires when the `InAppBrowser` receives a message posted from the page loaded inside the `InAppBrowser` Webview.
 
 - __callback__: the function that executes when the event fires. The function is passed an `InAppBrowserEvent` object as a parameter.

diff --git a/src/android/InAppBrowser.java b/src/android/InAppBrowser.java
@@ -19,6 +19,7 @@ Licensed to the Apache Software Foundation (ASF) under one
 package org.apache.cordova.inappbrowser;
 
 import android.annotation.SuppressLint;
+import android.annotation.TargetApi;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
@@ -51,6 +52,8 @@ Licensed to the Apache Software Foundation (ASF) under one
 import android.webkit.JavascriptInterface;
 import android.webkit.ValueCallback;
 import android.webkit.WebChromeClient;
+import android.webkit.WebResourceRequest;
+import android.webkit.WebResourceResponse;
 import android.webkit.WebSettings;
 import android.webkit.WebView;
 import android.webkit.WebViewClient;
@@ -141,7 +144,7 @@ public class InAppBrowser extends CordovaPlugin {
     private boolean hideUrlBar = false;
     private boolean showFooter = false;
     private String footerColor = "";
-    private boolean useBeforeload = false;
+    private String beforeload = "";
     private String[] allowedSchemes;
 
     /**
@@ -251,8 +254,8 @@ else if (action.equals("close")) {
             closeDialog();
         }
         else if (action.equals("loadAfterBeforeload")) {
-            if (!useBeforeload) {
-              LOG.e(LOG_TAG, "unexpected loadAfterBeforeload called without feature beforeload=yes");
+            if (beforeload == null) {
+                LOG.e(LOG_TAG, "unexpected loadAfterBeforeload called without feature beforeload=yes");
             }
             final String url = args.getString(0);
             this.cordova.getActivity().runOnUiThread(new Runnable() {
@@ -692,9 +695,8 @@ public String showWebPage(final String url, HashMap<String, String> features) {
             if (footerColorSet != null) {
                 footerColor = footerColorSet;
             }
-            String beforeload = features.get(BEFORELOAD);
-            if (beforeload != null) {
-                useBeforeload = beforeload.equals("yes") ? true : false;
+            if (features.get(BEFORELOAD) != null) {
+                beforeload = features.get(BEFORELOAD);
             }
         }
 
@@ -946,7 +948,7 @@ public void openFileChooser(ValueCallback<Uri> uploadMsg, String acceptType)
                     }
 
                 });
-                WebViewClient client = new InAppBrowserClient(thatWebView, edittext, useBeforeload);
+                WebViewClient client = new InAppBrowserClient(thatWebView, edittext, beforeload);
                 inAppWebView.setWebViewClient(client);
                 WebSettings settings = inAppWebView.getSettings();
                 settings.setJavaScriptEnabled(true);
@@ -1123,7 +1125,7 @@ public void onActivityResult(int requestCode, int resultCode, Intent intent) {
     public class InAppBrowserClient extends WebViewClient {
         EditText edittext;
         CordovaWebView webView;
-        boolean useBeforeload;
+        String beforeload;
         boolean waitForBeforeload;
 
         /**
@@ -1132,35 +1134,86 @@ public class InAppBrowserClient extends WebViewClient {
          * @param webView
          * @param mEditText
          */
-        public InAppBrowserClient(CordovaWebView webView, EditText mEditText, boolean useBeforeload) {
+        public InAppBrowserClient(CordovaWebView webView, EditText mEditText, String beforeload) {
             this.webView = webView;
             this.edittext = mEditText;
-            this.useBeforeload = useBeforeload;
-            this.waitForBeforeload = useBeforeload;
+            this.beforeload = beforeload;
+            this.waitForBeforeload = beforeload != null;
         }
 
         /**
          * Override the URL that should be loaded
          *
-         * This handles a small subset of all the URIs that would be encountered.
+         * Legacy (deprecated in API 24)
+         * For Android 6 and below.
          *
          * @param webView
          * @param url
          */
+        @SuppressWarnings("deprecation")
         @Override
         public boolean shouldOverrideUrlLoading(WebView webView, String url) {
+            return shouldOverrideUrlLoading(url, null);
+        }
+
+        /**
+         * Override the URL that should be loaded
+         *
+         * New (added in API 24)
+         * For Android 7 and above.
+         *
+         * @param webView
+         * @param request
+         */
+        @TargetApi(Build.VERSION_CODES.N)
+        @Override
+        public boolean shouldOverrideUrlLoading(WebView webView, WebResourceRequest request) {
+            return shouldOverrideUrlLoading(request.getUrl().toString(), request.getMethod());
+        }
+
+        /**
+         * Override the URL that should be loaded
+         *
+         * This handles a small subset of all the URIs that would be encountered.
+         *
+         * @param url
+         * @param method
+         */
+        public boolean shouldOverrideUrlLoading(String url, String method) {
             boolean override = false;
+            boolean useBeforeload = false;
+            String errorMessage = null;
+
+            if(beforeload.equals("yes")
+                    //TODO handle POST requests then this condition can be removed:
+                    && !method.equals("POST"))
+            {
+                useBeforeload = true;
+            }else if(beforeload.equals("get") && (method == null || method.equals("GET"))){
+                useBeforeload = true;
+            }else if(beforeload.equals("post") && (method == null || method.equals("POST"))){
+                //TODO handle POST requests
+                errorMessage = "beforeload doesn't yet support POST requests";
+            }
 
             // On first URL change, initiate JS callback. Only after the beforeload event, continue.
-            if (this.waitForBeforeload) {
+            if (useBeforeload && this.waitForBeforeload) {
+                if(sendBeforeLoad(url, method)){
+                    return true;
+                }
+            }
+
+            if(errorMessage != null){
                 try {
+                    LOG.e(LOG_TAG, errorMessage);
                     JSONObject obj = new JSONObject();
-                    obj.put("type", "beforeload");
+                    obj.put("type", LOAD_ERROR_EVENT);
                     obj.put("url", url);
-                    sendUpdate(obj, true);
-                    return true;
-                } catch (JSONException ex) {
-                    LOG.e(LOG_TAG, "URI passed in has caused a JSON error.");
+                    obj.put("code", -1);
+                    obj.put("message", errorMessage);
+                    sendUpdate(obj, true, PluginResult.Status.ERROR);
+                }catch(Exception e){
+                    LOG.e(LOG_TAG, "Error sending loaderror for " + url + ": " + e.toString());
                 }
             }
 
@@ -1239,12 +1292,58 @@ else if (!url.startsWith("http:") && !url.startsWith("https:") && url.matches("^
                 }
             }
 
-            if (this.useBeforeload) {
+            if (useBeforeload) {
                 this.waitForBeforeload = true;
             }
             return override;
         }
 
+        private boolean sendBeforeLoad(String url, String method){
+            try {
+                JSONObject obj = new JSONObject();
+                obj.put("type", "beforeload");
+                obj.put("url", url);
+                if(method != null){
+                    obj.put("method", method);
+                }
+                sendUpdate(obj, true);
+                return true;
+            } catch (JSONException ex) {
+                LOG.e(LOG_TAG, "URI passed in has caused a JSON error.");
+            }
+            return false;
+        }
+
+
+        /**
+         * Legacy (deprecated in API 21)
+         * For Android 4.4 and below.
+         * @param view
+         * @param url
+         * @return
+         */
+        @SuppressWarnings("deprecation")
+        @Override
+        public WebResourceResponse shouldInterceptRequest (final WebView view, String url) {
+            return shouldInterceptRequest(url, super.shouldInterceptRequest(view, url), null);
+        }
+
+        /**
+         * New (added in API 21)
+         * For Android 5.0 and above.
+         *
+         * @param webView
+         * @param request
+         */
+        @TargetApi(Build.VERSION_CODES.LOLLIPOP)
+        @Override
+        public WebResourceResponse shouldInterceptRequest(WebView view, WebResourceRequest request) {
+            return shouldInterceptRequest(request.getUrl().toString(), super.shouldInterceptRequest(view, request), request.getMethod());
+        }
+
+        public WebResourceResponse shouldInterceptRequest(String url, WebResourceResponse response, String method){
+            return response;
+        }
 
         /*
          * onPageStarted fires the LOAD_START_EVENT

diff --git a/src/ios/CDVInAppBrowserOptions.h b/src/ios/CDVInAppBrowserOptions.h
@@ -45,7 +45,7 @@
 @property (nonatomic, assign) BOOL suppressesincrementalrendering;
 @property (nonatomic, assign) BOOL hidden;
 @property (nonatomic, assign) BOOL disallowoverscroll;
-@property (nonatomic, assign) BOOL beforeload;
+@property (nonatomic, copy) NSString* beforeload;
 
 + (CDVInAppBrowserOptions*)parseOptions:(NSString*)options;
 

diff --git a/src/ios/CDVInAppBrowserOptions.m b/src/ios/CDVInAppBrowserOptions.m
@@ -46,6 +46,7 @@ - (id)init
         self.closebuttoncolor = nil;
         self.toolbarcolor = nil;
         self.toolbartranslucent = YES;
+        self.beforeload = @"";
     }
 
     return self;

diff --git a/src/ios/CDVUIInAppBrowser.h b/src/ios/CDVUIInAppBrowser.h
@@ -35,7 +35,7 @@
   UIWindow * tmpWindow;
 
   @private
-  BOOL _useBeforeload;
+  NSString* _beforeload;
   BOOL _waitForBeforeload;
 }
 

diff --git a/src/ios/CDVUIInAppBrowser.m b/src/ios/CDVUIInAppBrowser.m
@@ -53,7 +53,7 @@ - (void)pluginInitialize
     instance = self;
     _previousStatusBarStyle = -1;
     _callbackIdPattern = nil;
-    _useBeforeload = NO;
+    _beforeload = @"";
     _waitForBeforeload = NO;
 }
 
@@ -219,8 +219,12 @@ - (void)openInInAppBrowser:(NSURL*)url withOptions:(NSString*)options
     }
 
     // use of beforeload event
-    _useBeforeload = browserOptions.beforeload;
-    _waitForBeforeload = browserOptions.beforeload;
+    if([browserOptions.beforeload isKindOfClass:[NSString class]]){
+        _beforeload = browserOptions.beforeload;
+    }else{
+        _beforeload = @"yes";
+    }
+    _waitForBeforeload = ![_beforeload isEqualToString:@""];
 
     [self.inAppBrowserViewController navigateTo:url];
     if (!browserOptions.hidden) {
@@ -320,7 +324,7 @@ - (void)loadAfterBeforeload:(CDVInvokedUrlCommand*)command
 {
     NSString* urlStr = [command argumentAtIndex:0];
 
-    if (!_useBeforeload) {
+    if ([_beforeload isEqualToString:@""]) {
         NSLog(@"unexpected loadAfterBeforeload called without feature beforeload=yes");
     }
     if (self.inAppBrowserViewController == nil) {
@@ -454,6 +458,22 @@ - (BOOL)webView:(UIWebView*)theWebView shouldStartLoadWithRequest:(NSURLRequest*
     NSURL* url = request.URL;
     BOOL isTopLevelNavigation = [request.URL isEqual:[request mainDocumentURL]];
     BOOL shouldStart = YES;
+    BOOL useBeforeLoad = NO;
+    NSString* httpMethod = request.HTTPMethod;
+    NSString* errorMessage = nil;
+
+    if([_beforeload isEqualToString:@"post"]){
+        //TODO handle POST requests by preserving POST data then remove this condition
+        errorMessage = @"beforeload doesn't yet support POST requests";
+    }
+    else if(isTopLevelNavigation && (
+         [_beforeload isEqualToString:@"yes"]
+         || ([_beforeload isEqualToString:@"get"] && [httpMethod isEqualToString:@"GET"])
+      // TODO comment in when POST requests are handled
+      // || ([_beforeload isEqualToString:@"post"] && [httpMethod isEqualToString:@"POST"])
+    )){
+        useBeforeLoad = YES;
+    }
 
     // See if the url uses the 'gap-iab' protocol. If so, the host should be the id of a callback to execute,
     // and the path, if present, should be a JSON-encoded value to pass to the callback.
@@ -498,15 +518,23 @@ - (BOOL)webView:(UIWebView*)theWebView shouldStartLoadWithRequest:(NSURLRequest*
         }
     }
 
-    // When beforeload=yes, on first URL change, initiate JS callback. Only after the beforeload event, continue.
-    if (_waitForBeforeload && isTopLevelNavigation) {
+    // When beforeload, on first URL change, initiate JS callback. Only after the beforeload event, continue.
+    if (_waitForBeforeload && useBeforeLoad) {
         CDVPluginResult* pluginResult = [CDVPluginResult resultWithStatus:CDVCommandStatus_OK
                                                       messageAsDictionary:@{@"type":@"beforeload", @"url":[url absoluteString]}];
         [pluginResult setKeepCallback:[NSNumber numberWithBool:YES]];
-
+        
         [self.commandDelegate sendPluginResult:pluginResult callbackId:self.callbackId];
         return NO;
     }
+
+    if(errorMessage != nil){
+        NSLog(errorMessage);
+        CDVPluginResult* pluginResult = [CDVPluginResult resultWithStatus:CDVCommandStatus_ERROR
+                                                      messageAsDictionary:@{@"type":@"loaderror", @"url":[url absoluteString], @"code": @"-1", @"message": errorMessage}];
+        [pluginResult setKeepCallback:[NSNumber numberWithBool:YES]];
+        [self.commandDelegate sendPluginResult:pluginResult callbackId:self.callbackId];
+    }
 
     //if is an app store link, let the system handle it, otherwise it fails to load it
     if ([[ url scheme] isEqualToString:@"itms-appss"] || [[ url scheme] isEqualToString:@"itms-apps"]) {
@@ -523,7 +551,7 @@ - (BOOL)webView:(UIWebView*)theWebView shouldStartLoadWithRequest:(NSURLRequest*
         [self.commandDelegate sendPluginResult:pluginResult callbackId:self.callbackId];
     }
 
-    if (_useBeforeload && isTopLevelNavigation) {
+    if (useBeforeLoad) {
         _waitForBeforeload = YES;
     }
 

diff --git a/src/ios/CDVWKInAppBrowser.h b/src/ios/CDVWKInAppBrowser.h
@@ -28,7 +28,7 @@
 
 @interface CDVWKInAppBrowser : CDVPlugin {
     @private
-    BOOL _useBeforeload;
+    NSString* _beforeload;
     BOOL _waitForBeforeload;
 }