beforeload causing POST failure #359

keenan35i · 2018-11-29T22:19:51Z

Hi i really was happy about this feature as i needed it for my app, however when i enable beforeload, it fails to send post data to the server on a forum submit. The page load goes through but does not send the post data to the server

<form accept-charset="UTF-8" action="/search_results?mobile_app=true" id="login" method="post">

activeBrowser.addEventListener('beforeload', function(e, callBack){
        console.log('before load event triggered', e, e.url);
        //prints www.someurl.com/search_results?mobile_app=true
        // so when it redirects you dont get the post data
        callBack(e.url);
      });

The text was updated successfully, but these errors were encountered:

janpio · 2018-11-30T10:54:12Z

What platform? What OS version?

I am not sure I am following.

Can you explain your problem and what you are trying to do a bit better?
How did you open the InAppBrowser window showing that page with the form?
Are you trying to "hook" into the POST?
Without the listener everything is working, but with it the POST data gets lost?

keenan35i · 2018-11-30T15:43:42Z

iphone xs ios 12, i am trying to implement cacheing with the beforeload event, however if i include the beforeload even on a simple passthrough like the example i posted, post data within a form gets lost, the server gets the hit but without any post data. Everything works ok when i dont have the event listener or beforeload=no.


var options = 'beforeload=yes,usewkwebview=yes,location=no,toolbar=no,allowInlineMediaPlayback=yes,hidden=yes,disableAnimation=yes,hidespinner=yes';

cordova.InAppBrowser.open(url, '_blank', options);

janpio · 2018-11-30T17:01:00Z

Could you create a super simple reproduction app that helps reproduce this issue? Start a new Cordova app with cordova create, add the minimal code to reproduce and understand this, put it on GitHub and post a link to the repository here. Thanks.

keenan35i · 2018-11-30T17:40:55Z

I dont have a server i can give you access to to make the server hit, i made a slightly more complete html/js structure maybe this helps. On button click submit, the form hits the server at "/search_results?mobile_app=true", but the input value is lost, possibly the url data you are useing does not include post data, thus the server only gets the url without the attached data, i looked through the code:

navigationAction.request.URL;
possibly navigationAction.request.URL does not get the full url including post data

<html>
<head>
  <meta name="viewport" content="width=device-width,initial-scale=1,maximum-scale=1,user-scalable=0,viewport-fit=cover">
  <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
</head>
<body>
  <form accept-charset="UTF-8" action="/search_results?mobile_app=true" id="login" method="post">
    <input type="text" name="someuser">
    <button type="submit">Continue</button>
  </form>
  testApp.init();
</body>
</html>

var testApp = {
  init: function(){
    var options = options || 'beforeload=yes,usewkwebview=yes,location=no,toolbar=no,allowInlineMediaPlayback=yes,hidden=yes,disableAnimation=yes,hidespinner=yes';
    testApp.activeBrowser = cordova.InAppBrowser.open(url, '_blank', options);
    
    testApp.activeBrowser.addEventListener('beforeload', function(e, callBack){
      console.log('before load event triggered', e, e.url);
      callBack(e.url);
    });
  }
};

janpio · 2018-11-30T17:49:22Z

the form hits the server at "/search_results?mobile_app=true"

POST or GET?

i made a slightly more complete html/js structure maybe this helps.

Probably will, but having a ready made app one just has to check out and build/run would be even better - saves a few minutes of work for the maintainer.

keenan35i · 2018-11-30T18:00:03Z

post, its in the form attribute, method="post" in the example

janpio · 2018-11-30T18:03:21Z

That I see. But is the request that is missing the POST data really a POST?

keenan35i · 2018-11-30T18:05:53Z

yes it is a POST

janpio · 2018-11-30T18:24:29Z

Ok thanks, now a future maintainer looking into this knows what to look at and for.

keenan35i · 2018-11-30T18:39:08Z

@dpa99c you added this feature correct? Any insights or ideas on what it could be, maybe it needs an alternate to using navigationAction.request.URL; as that seems to be only communicating the url without getting the data sent with it?

dpa99c · 2018-11-30T18:46:53Z

This feature was originally added by @wvengen in #276, but I patched it to the iOS WKWebView implementation in #350.

Does it work with the UIWebView implementation (usewkwebview=no)?

keenan35i · 2018-11-30T20:14:11Z

Not sure, however i dont think my app will work properly in UIwebview, it was built in wkwebview in a fork of the inappbrowser, but i switched back to master after the release of wkwebview. ill give it a shot though and give some feed back soon, thanks.

keenan35i · 2018-12-03T16:35:45Z

@dpa99c , I tested with "usewkwebview=no" same issue.

dpa99c · 2018-12-03T16:39:57Z

@keenan35i OK, that implies the issue must've been present in the original UIWebView implementation in #276 and that I've translated the same problem across to the WKWebView implementation in #350.

@wvengen any ideas how this could be resolved in your original UIWebView implementation (i.e. preservation of POST data in the intercepted request)?

wvengen · 2018-12-04T13:21:28Z

That's a good point - the callback basically loads the URL again, indeed POST data would not be present - I'm afraid even the method would get lost. This might also be an issue for the referer (depending on how that is derived).

To fix this, I think the following would be needed:

include method and POST data in the event
pass these on to the callback (e.g. as extra parameters)

When these arguments are missing, the GET method would be assumed.
I hope this data is available in the relevant callbacks, it depends on platform support. If the method is available but POST data not, we might want to skip the callback for POST requests.

dpa99c · 2018-12-04T15:59:17Z

On iOS with UIWebView, it should be possible to use the NSURLRequest object passed to shouldStartLoadWithRequest to capture the HTTP method type and the POST data.
We should then be able to use that to make a new request.

With WKWebView, while we can extract the POST data and request type from NSURLRequest *request property of the WKNavigationAction object passed in to decidePolicyForNavigationAction, according to this SO post, it's not possible to programmatically make a POST request using WKWebView.

On Android it seems there might be an issue with intercepting POST requests in the first place.
According to this SO post, shouldOverrideUrlLoading() isn't called on POST requests and it's that method we're currently using to intercept the request on Android when beforeload is specified.
And according to this SO post, the same is true for shouldInterceptRequest().
So there doesn't seem to be an easy way of arbitrarily intercepting POST requests on Android.

keenan35i · 2018-12-04T16:44:29Z

In regards to IOS (wkwebview), why not just pass the POST data back to a js function within the plugin js that can handle creating a secondary POST function after the logic determines which url to hit if any.

dpa99c · 2018-12-04T17:48:59Z

@keenan35i that would be a possible workaround for WKWebView POST requests, but it's a bit hacky.
If POST request prove too problematic (i.e. Android also), we may need to restrict beforeload to only GET requests.

In the case of both GET and POST, we'd ideally want to preserve any request headers also, particularly given that custom request headers is something that's being considered as a feature in #361.

keenan35i · 2018-12-04T18:47:13Z

Ok in that case, can we disable listening to POST requests and rename it to beforeGETLoad, adding beforePOSTLoad in the future as a different feature, at least i would be able to use this feature instead of it causing the app to break.

wvengen · 2018-12-05T07:51:34Z

What about: beforeload=get, beforeload=post (to be developed later), beforeload=get+post (combination of the previous; or =both). For backward compatibility, we could let beforeload=yes mean beforeload=get.
Do we need to consider other methods like DELETE, PUT - I suppose one would generally want to intercept these methods together with POST.

janpio · 2018-12-05T09:43:31Z

Better solution, if really necessary, would probably be to only handle GET requests inside the function that does this and document that it only works for GET requests, agree? But of course it just working for all methods would be a lot better ;)

keenan35i · 2018-12-05T16:29:03Z

beforeload=get, beforeload=post, leaving POST to get developed later seems like a good idea, at least it keeps POST in the mix for the future

dpa99c · 2018-12-05T16:54:03Z

This should work for both iOS implementations.

On Android, we have no way of intercepting the initial POST request anyway so it will proceed (along with its POST data) regardless of whether beforeload is set and the beforeload callback will not be invoked.

dpa99c · 2018-12-07T21:05:30Z

@wvengen I'm working on a PR to address this issue.

iOS is done, but on Android I'm finding that on testing the existing implementation on the master branch, shouldOverrideUrlLoading() is never being called, regardless of whether it's a POST or GET request being made by the InappBrowser.

I'm debugging it in Android Studio and while the breakpoint on inAppWebView.loadUrl() is being reached, the one on shouldOverrideUrlLoading() is never hit.

Any idea why this might be?

I'm testing on a device running Android 8.1 and using this test project using cordova-android@7.1.4.

Note that if I insert an override for shouldInterceptRequest(), then put a breakpoint on this, it does get hit (for every resource being loaded).

…beforeload. For now only GET is supported with beforeload, so document this.

dpa99c · 2018-12-10T15:47:47Z

OK, I've figured it out: on Android shouldOverrideUrlLoading() is not called on initial navigation (to the initial URL passed in with inAppWebView.loadUrl()) but only on subsequent navigation (e.g. click a hyperlink or form submit button in the IAB content page) whereas shouldInterceptRequest() is also called on initial navigation.

On iOS (both implementations), the relevant callbacks are always invoked for both initial and subsequent navigation.

dpa99c · 2018-12-10T16:46:46Z

I've created a PR to address the cause of this issue and to leave the future possibility open of supporting interception of POST requests using beforeload.

Please give my branch a test:

cordova plugin add https://github.com/dpa99c/cordova-plugin-themeablebrowser#GH-359-fix-beforeload-POST

I've also created a test app container which I used to validate this:

https://github.com/dpa99c/cordova-plugin-inappbrowser-test

keenan35i · 2018-12-10T20:04:51Z

The link you provided is for themeable browser plugin not in app browser plugin?

dpa99c · 2018-12-10T21:06:23Z

Don't be misled by the name: the repo is called cordova-plugin-themeablebrowser because it's originally a fork of initialxy/cordova-plugin-themeablebrowser which is a fork of apache/cordova-plugin-inappbrowser.

However the branch GH-359-fix-beforeload-POST is derived from apache/cordova-plugin-inappbrowser#master.
There are other branches derived from initialxy/cordova-plugin-themeablebrowser.

Fun with git: you can have multiple remotes!

keenan35i · 2018-12-10T21:10:45Z

ah ok thanks, will test it out and let you know if it works for my app.

keenan35i · 2018-12-10T21:38:28Z

Just tested it seems to be working great, POST is ignored when beforeload=get.

keenan35i · 2018-12-11T22:34:58Z

@dpa99c do you happen to have a branch with both this fix and the postMessage API feature together? Seems to be taking a while for the PR's to be merged and im trying to finish building my cacheing tool which uses both of these features.

dpa99c · 2018-12-12T09:47:50Z

@keenan35i I've kept the changes associated with both PRs separate so as not to confuse the PR review process. You can always checkout both of my PR branches and merge them into your own combined fork for now.

But yeah, it would be good to get this merged to master, as well as #362

### 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)

janpio added the support label Nov 30, 2018

janpio added the platform: ios label Nov 30, 2018

keenan35i changed the title ~~beforeload causing post failure~~ beforeload causing POST failure Nov 30, 2018

dpa99c added a commit to dpa99c/cordova-plugin-themeablebrowser that referenced this issue Dec 10, 2018

apacheGH-359: Distinguish between GET and POST requests when applied …

a6f2427

…beforeload. For now only GET is supported with beforeload, so document this.

dpa99c mentioned this issue Dec 10, 2018

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

Merged

dpa99c mentioned this issue Dec 12, 2018

(iOS & Android) Add postMessage API support #362

Merged

janpio added a commit to dpa99c/cordova-plugin-themeablebrowser that referenced this issue Dec 17, 2018

Merge branch 'master' into apacheGH-359-fix-beforeload-POST

1d58272

keenan35i closed this as completed Jan 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

beforeload causing POST failure #359

beforeload causing POST failure #359

keenan35i commented Nov 29, 2018

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018 •

edited

Loading

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018 •

edited

Loading

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018 •

edited

Loading

dpa99c commented Nov 30, 2018

keenan35i commented Nov 30, 2018

keenan35i commented Dec 3, 2018

dpa99c commented Dec 3, 2018

wvengen commented Dec 4, 2018

dpa99c commented Dec 4, 2018

keenan35i commented Dec 4, 2018

dpa99c commented Dec 4, 2018

keenan35i commented Dec 4, 2018

wvengen commented Dec 5, 2018

janpio commented Dec 5, 2018 •

edited

Loading

keenan35i commented Dec 5, 2018

dpa99c commented Dec 5, 2018

dpa99c commented Dec 7, 2018

dpa99c commented Dec 10, 2018

dpa99c commented Dec 10, 2018

keenan35i commented Dec 10, 2018

dpa99c commented Dec 10, 2018

keenan35i commented Dec 10, 2018

keenan35i commented Dec 10, 2018

keenan35i commented Dec 11, 2018

dpa99c commented Dec 12, 2018

beforeload causing POST failure #359

beforeload causing POST failure #359

Comments

keenan35i commented Nov 29, 2018

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018 • edited Loading

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018 • edited Loading

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018

janpio commented Nov 30, 2018

keenan35i commented Nov 30, 2018 • edited Loading

dpa99c commented Nov 30, 2018

keenan35i commented Nov 30, 2018

keenan35i commented Dec 3, 2018

dpa99c commented Dec 3, 2018

wvengen commented Dec 4, 2018

dpa99c commented Dec 4, 2018

keenan35i commented Dec 4, 2018

dpa99c commented Dec 4, 2018

keenan35i commented Dec 4, 2018

wvengen commented Dec 5, 2018

janpio commented Dec 5, 2018 • edited Loading

keenan35i commented Dec 5, 2018

dpa99c commented Dec 5, 2018

dpa99c commented Dec 7, 2018

dpa99c commented Dec 10, 2018

dpa99c commented Dec 10, 2018

keenan35i commented Dec 10, 2018

dpa99c commented Dec 10, 2018

keenan35i commented Dec 10, 2018

keenan35i commented Dec 10, 2018

keenan35i commented Dec 11, 2018

dpa99c commented Dec 12, 2018

keenan35i commented Nov 30, 2018 •

edited

Loading

keenan35i commented Nov 30, 2018 •

edited

Loading

keenan35i commented Nov 30, 2018 •

edited

Loading

janpio commented Dec 5, 2018 •

edited

Loading