Skip to content
This repository was archived by the owner on Jan 13, 2020. It is now read-only.

Android set support? #1

Open
harrydema opened this issue Dec 13, 2019 · 21 comments
Open

Android set support? #1

harrydema opened this issue Dec 13, 2019 · 21 comments
Labels
core dependency enhancement New feature or request

Comments

@harrydema
Copy link

When it is going to support the set() method for Android?

@safaiyeh safaiyeh added the enhancement New feature or request label Dec 13, 2019
@safaiyeh
Copy link
Owner

safaiyeh commented Dec 13, 2019

Looks like Android API supports it: https://developer.android.com/reference/android/webkit/CookieManager.html#setCookie(java.lang.String,%20java.lang.String)

I'll check it out and put up a PR. Would you mind testing it when it is up? @harrydema

@safaiyeh
Copy link
Owner

React Native has support for adding cookies; however, the method is private: https://github.com/facebook/react-native/blob/aee88b6843cea63d6aa0b5879ad6ef9da4701846/ReactAndroid/src/main/java/com/facebook/react/modules/network/ForwardingCookieHandler.java#L127

Will make a PR to core to make the method public and see how that route goes.

@harrydema
Copy link
Author

harrydema commented Dec 16, 2019

Let me know when it is available, and i will test it.

This is the PR on core:
facebook/react-native#27512

Take a look a test failed

@harrydema
Copy link
Author

harrydema commented Dec 16, 2019

Can't we use this publlic method instead? @safaiyeh

image

@safaiyeh
Copy link
Owner

Yeah that test is a known issue. It fails on every PR.

We currently use that with setFromResponse: https://github.com/safaiyeh/react-native-cookie-store/blob/master/android/src/main/java/com/psykar/cookiemanager/CookieManagerModule.java#L40

My issue is that I have to transform the data in a weird way for it to work. I can use it as a temporary implementation.

@harrydema
Copy link
Author

harrydema commented Dec 16, 2019

I am trying to implement it, the thing is that when i set 3 cookies on a same url, they get replaced, even if they have different names.

This is what i have so far:

image

I am setting the domain as the url, IDK if this is ok. What do you think?

@safaiyeh
Copy link
Owner

safaiyeh commented Dec 16, 2019

So cookie will be in this format:

{
  name: 'myCookie',
  value: 'myValue',
  domain: 'some domain',
  origin: 'some origin',
  path: '/',
  version: '1',
  expiration: '2015-05-30T12:30:00.00-05:00'
}

My initial thought was to create a string builder and make these fields required.

Thanks for taking the initiative to seek out an implementation!

@harrydema
Copy link
Author

I am not being able to set several cookies for the same url, when ever a set a new cookie on a url, the past values are removed. Would you please help me to figure out this?

@safaiyeh
Copy link
Owner

I’ll take a look today 😁

@harrydema
Copy link
Author

I will send a PR

@harrydema
Copy link
Author

harrydema commented Dec 16, 2019

I am setting 3 cookies:

2019-12-16 16:10:04.528 7313-7367/com.test.local I/headers: {Set-Cookie=[path=/, origin=.test.com, domain=.test.com, name=CloudFront-Key-Pair-Id, expiration=2099-05-30T12:30:00.00-05:00, version=1, value=XXXX]}
2019-12-16 16:10:04.534 7313-7367/com.test.local I/headers: {Set-Cookie=[path=/, origin=.test.com, domain=.test.com, name=CloudFront-Policy, expiration=2099-05-30T12:30:00.00-05:00, version=1, value=XXXX]}
2019-12-16 16:10:04.541 7313-7367/com.test.local I/headers: {Set-Cookie=[path=/, origin=.test.com, domain=.test.com, name=CloudFront-Signature, expiration=2099-05-30T12:30:00.00-05:00, version=1, value=XXXX]}

But when I do a get, and print all the cookies:

image

I get only the last cookie:

2019-12-16 16:10:01.531 7313-7367/com.test.local I/cookieMap: {Cookie=[path=/; origin=.test.com; domain=.test.com; name=CloudFront-Signature; expiration=2099-05-30T12:30:00.00-05:00; version=1; value=XXXX]}

@harrydema
Copy link
Author

harrydema commented Dec 16, 2019

I noticed this is for webkit only. It won't work for requests. Could it be?

image

@harrydema
Copy link
Author

How can I test if this is woking? If I do a fetch('https://google.com') I can't see the cookies being sent

@harrydema
Copy link
Author

After researching a lot, I found out there is a bigger problem behind this:

facebook/react-native#23185

@safaiyeh
Copy link
Owner

If it is an issue with the react native repo, we can migrate out of using their functions and just use CookieManager. I'll have more time today to look into it. Sorry, still at work haha.

@safaiyeh
Copy link
Owner

@harrydema I might have figured out a solution, I pushed it to your PR.

@KingAmo
Copy link

KingAmo commented Dec 26, 2019

could you please explain to me that what is the difference between set and setFromResponse ? i am a lit confused, the README showcase seems they are the same...

facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Dec 27, 2019
Summary:
react-native-cookie-store wants the ability to set custom cookies on Android. We use ForwardingCookieHandler to mange the cookies. Exposing the `addCookies` method will allow the module to provide the same functionality on Android.

safaiyeh/react-native-cookie-store#1

## Changelog

[Android] [Changed] - Expose addCookies method
Pull Request resolved: #27512

Test Plan: N/A

Differential Revision: D19236309

Pulled By: cpojer

fbshipit-source-id: bf1a0730165456c34c5bf432ac370176a881cbcf
@safaiyeh
Copy link
Owner

The PR I made to React Native, just got merged in today. I'll try out using their API

@safaiyeh
Copy link
Owner

@KingAmo they are the same in the sense that they both store cookies.

The data format the method accepts is different. setFromResponse takes Set-Cookie response headers. While set should take a JSON object of attributes.

@safaiyeh
Copy link
Owner

on pause until change to react native gets released, will work to get cherry picked.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core dependency enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants