You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by mlaponder <gi...@git.apache.org> on 2015/06/16 13:36:53 UTC
[GitHub] cordova-plugin-file-transfer pull request: Fix/wp8 browser cookies
GitHub user mlaponder opened a pull request:
https://github.com/apache/cordova-plugin-file-transfer/pull/90
Fix/wp8 browser cookies
[wp8] Make it possible to use the webview's cookiestore to do HTTP requests
On the wp8 platform, cookies used to be always copied from the webview when performing a request. However, only the cookies for the currently loaded URL could be copied. This made it impossible to pass cookies cross-origin.
This commit introduces a flag `useBrowserHttp` which when `false` keeps the same behavior. When `true`, it creates the WebRequest through a different factory method, reusing the cookies of the webview.
Using this option comes at a price though: it makes it impossible to add cookies manually by setting headers and it can't perform GET requests with headers set. Therefore this functionality is put behind a flag.
Because we had to change the API between the JS and native code to add the flag, this commit also includes platform changes. Code was tested on ios, android and wp8 platforms.
Co-Authored-By: Leroy van Engelen <le...@mendix.com>
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mlaponder/cordova-plugin-file-transfer fix/wp8-browser-cookies
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugin-file-transfer/pull/90.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #90
----
commit fc1a60c0ee8c049c1d659ee275bbf3ced8c0a682
Author: Marco Laponder <ma...@mendix.com>
Date: 2015-06-15T14:15:29Z
Add useBrowserHttp argument to native interfaces
commit c70c3aa8b94d8e87f525d66ecdd1888b69b66fd1
Author: Marco Laponder <ma...@mendix.com>
Date: 2015-06-15T14:27:40Z
Use cookies directly from the webview
Originally, cookies from the webview were always copied to the
WebRequest. When getting the cookies from the webiew only those
for the current URL can be retrieved. This means that cookies of other
domains were never copied to the WebRequest for upload and download.
This commit adds the option to use a HTTP request constructed by the
webview, which automatically adds any available cookies for that
request.
Fixed by using the BrowserHttp implementation of IWebRequestCreate
which shares the cookie store with the webview. See also
http://stackoverflow.com/questions/4212713/grabbing-cookies-in-web-browser-control-wp7
This functionality is enabled by a new useBrowserHttp flag. We need
this to be optional because:
- It removes the possibility of adding cookie headers manually
- It makes it impossible to perform GET requests with manually set
headers
commit 78d216a94e02a15faee81b008aa4662151344b3b
Author: Marco Laponder <ma...@mendix.com>
Date: 2015-06-15T14:55:30Z
Move Uri exception handling to outer catch block
This will make it easier to unify the WebRequest creation code later.
commit 58fcc98e7ffa79650d2900c8d16cbea1a19c3d77
Author: Marco Laponder <ma...@mendix.com>
Date: 2015-06-15T14:56:50Z
Use an Uri to create WebRequest in download as well
commit e63d707445d0c03a4b0065e5e5d11a7d4e978896
Author: Marco Laponder <ma...@mendix.com>
Date: 2015-06-15T15:06:22Z
Extract WebRequest creation
Share the WebRequest creation code between download and upload.
commit fdaa1cf5a8df18fd0f2a8feefe90d54babbbc457
Author: Marco Laponder <ma...@mendix.com>
Date: 2015-06-16T11:34:02Z
Add documentation for `useBrowserHttp`
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file-transfer pull request: Fix/wp8 browser cookies
Posted by lvanengelen <gi...@git.apache.org>.
Github user lvanengelen commented on the pull request:
https://github.com/apache/cordova-plugin-file-transfer/pull/90#issuecomment-161928602
@daserge @purplecabbage: what is this I hear about deprecation of the wp8 platform? I haven't been able to find any references to that on the Cordova site. Maybe I am missing something.
@daserge: we didn't move the parameter to the end because we thought it was the more logical place wrt. the other parameters. Moving it to the end to not disturb the other platform code is fine by us.
Btw, sorry for not responding. It will take some time before we can take a better look at the comments and rebase. Between us submitting it and you looking into it, higher priority stuff has come up :(
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file-transfer pull request: Fix/wp8 browser cookies
Posted by lvanengelen <gi...@git.apache.org>.
Github user lvanengelen commented on the pull request:
https://github.com/apache/cordova-plugin-file-transfer/pull/90#issuecomment-117064312
Filed a ticket for this issue: https://issues.apache.org/jira/browse/CB-9267
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file-transfer pull request: Fix/wp8 browser cookies
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on the pull request:
https://github.com/apache/cordova-plugin-file-transfer/pull/90#issuecomment-161909635
@purplecabbage: Jesse, could you please take a look at this and #85 - do we need to accept `wp8` PRs taking into account the `wp8` platform deprecation?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file-transfer pull request: Fix/wp8 browser cookies
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on the pull request:
https://github.com/apache/cordova-plugin-file-transfer/pull/90#issuecomment-161929619
@lvanengelen, here is the discussion link about wp8 deprecation: http://markmail.org/message/y2j62jbwrnlzzwrz
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file-transfer pull request: Fix/wp8 browser cookies
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on the pull request:
https://github.com/apache/cordova-plugin-file-transfer/pull/90#issuecomment-161930336
Also, regarding the `useBrowserHttp` parameter - would it even be better to place it into optional `options` parameter? That way the signature will not be changed at all.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file-transfer pull request: Fix/wp8 browser cookies
Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on the pull request:
https://github.com/apache/cordova-plugin-file-transfer/pull/90#issuecomment-161917051
Plugins yes!
Assiuming they don't require platform changes.
Deprecation window is 6 months or more, and we haven't fully decided on it yet.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file-transfer pull request: Fix/wp8 browser cookies
Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on the pull request:
https://github.com/apache/cordova-plugin-file-transfer/pull/90#issuecomment-149303421
+1 to @daserge 's proposition.
Also, please rebase to current master as it has conflicts currently.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-file-transfer pull request: Fix/wp8 browser cookies
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on the pull request:
https://github.com/apache/cordova-plugin-file-transfer/pull/90#issuecomment-149195485
@mlaponder, @lvanengelen, thanks for your contribution!
Is there a specific reason why you did not put the new flag to the end of the list?
This way it would not be needed to change other platforms interface besides `wp8`.
Is there an opportunity to add a test case for this?
Do I get it right that the feature is an extension of [CB-8761 WP8: FileTransfer does not inherit cookies from WebBrowser](https://issues.apache.org/jira/browse/CB-8761) for cross-origin case, so that the test will require a host separate from [cordova-labs/file-transfer](https://github.com/apache/cordova-labs/tree/cordova-filetransfer) -> [cordova-vm.apache.org](https://github.com/apache/cordova-plugin-file-transfer/blob/c79ee367a108384cbe10b8bc8038911612a67bef/tests/tests.js#L42) test server?
I propose you to refactor this in terms of not changing unrelated platforms (move `useBrowserHttp` to the end) and send a proposal to dev@cordova.apache.org (and optionally to https://github.com/cordova/cordova-discuss/issues/) so that the community can discuss whether to merge it in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org