apache / cordova-plugin-file-transfer

Apache Cordova File Transfer Plugin
https://cordova.apache.org/
Apache License 2.0
596 stars 887 forks source link

CB-12399: (Android) Fix bug with WebView & CookieManager cookies #174

Closed Lemon-King closed 6 years ago

Lemon-King commented 7 years ago

Platforms affected

Android

What does this PR do?

Resolves the issue where only WebView cookies would be used, affecting hybrid applications which use CookieManager cookies.

What testing has been done on this change?

Tested on Motorola, Nexus 6, and Pixel devices. Also tested on various emulators api ranges (19 - 24).

Checklist

cordova-qa commented 7 years ago

Cordova CI Build has completed successfully.

Commit - Link Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 10 Store Link Link Link
iOS 9.3 Link Link Link
iOS 10.0 Link Link Link
Android 4.4 Link Link Link
Android 5.1 Link Link Link

260 tests run, 13 skipped, 0 failed.

filmaj commented 7 years ago

Hi @Lemon-King, thanks for the PR. Can you explain your use case a bit more?

Lemon-King commented 7 years ago

Can do.

This PR resolves an issue we came across on our framework with File Transfer and were unsure if others were dealing with the same problem.

The issue was with CloudFlare and services like it will send a few cookies downstream such as cfduid and uvt. Instead of reading the other cookies send by the session File Transfer would pull __cfduid only and set that as the cookie header during the File Transfer request. Just the single cookie, no others. We resolved this by appending any cookies that we stored locally within CookieManager to the cookies gathered by the method gcMethod.

We felt if the developer chooses to utilize CookieManager over WebView for cookie handling, locally, within the app. That this PR will allow cookies within CookieManager to be used correctly.

filmaj commented 7 years ago

I see, so any custom handling of cookies via CookieManager are being appended over and on top of whatever the WebView is handling. I guess if there are conflicts between the user's implementation via CookieManager, and what the WebView returns, that's the user's problem?

Lemon-King commented 7 years ago

The patch itself allows WebView and CookieManager's cookies to be used together really. It won't overwrite WebView but join WebView and CookieManager together as a single cookies string which should prevent any compatibility issues between how the user decides to utilize their cookies.

Lemon-King commented 7 years ago

I believe I signed the ICLA on the main apache org jira site.

e: No I have not, lemme look into this with our team.

purplecabbage commented 7 years ago

Thoughts here @infil00p ?

maverickmishra commented 6 years ago

With the new features introduced in XMLHttpRequest, this plugin is not needed any more. Migrating from this plugin to using the new features of XMLHttpRequest, is explained in this Cordova blog post.