Open uen opened 4 years ago
Are there any plans to change this / would a PR changing the cookie jar be accepted?
Anyone having this issue, I forked react-native with a fix here: https://github.com/uen/react-native-34/commit/4a054da2169c4f085b6165648a1290b3ea8b3751 - It should be pretty much the same in every other version of RN too.
The fork simply copies the okhttp JavaNetCookieJar class and fixes the issue, and then makes React Native use it. Ideally JavaNetCookieJar needs to be changed to use CookieJar from okhttp, but this may have unintended side effects & probably needs tests updating etc.
I will eventually create a module that replaces the Networking module here: https://github.com/uen/react-native-cookie-fetch so it's more 'drop-in' and won't require a custom RN version.
If you use Expo managed workflow there is no way around this issue and I doubt it will be fixed since there's over 1k issues open in this repo currently
@whyer
I tried creating a combination of your code @uen , and a technique to try to avoid actually changing the RN code. Haven't gotten it to work just yet but here is an initial version.
@viktorlarsson Getting a 404 on that link
@viktorlarsson Nice that's a lot better than using a custom RN version, doesn't look too difficult to turn that into a module either. Did you fix the issue you were having since it's merged?
Still having issues, hard to debug unfortunately! Unable to get any breakpoint hits within loadForRequest etc.
This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.
It is still an issue
This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.
It's still an issue
On Thu, 11 Jan 2024, 05:13 github-actions[bot], @.***> wrote:
This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.
— Reply to this email directly, view it on GitHub https://github.com/facebook/react-native/issues/29916#issuecomment-1886256324, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6E224BEWBOUZBN3S5TB63YN5YIFAVCNFSM4RDNAMF2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBYGYZDKNRTGI2A . You are receiving this because you were mentioned.Message ID: @.***>
Description
tl;dr Commas are treated as cookie deliminators on Android only
Commas (,) in cookie values are not allowed as part of the cookie specification but they're still allowed on all major browsers and through iOS's networking APIs. However in Android okhttp (specifically okhttp3.JavaNetCookieJar) commas are treated as cookie delimiters in the same way that semicolons are. This behaviour isn't mentioned anywhere and is inconsistent with iOS/Web.
I have found an existing issue on okhttp which suggests the regular CookieJar doesn't have this limitation and should be used instead of JavaNetCookieJar. I can also confirm that removing the comma here fixes the issue. React Native sets the okhttp cookie jar to JavaNetCookieJar here.
I'm working with an external service which I can't control so changing the cookie isn't an option. Because {redirect: "manual"} doesn't work in React Native iOS/Android, it's impossible to override this behaviour if the cookie is set in a 302 response and required for subsequent responses.
React Native version:
Steps To Reproduce and expected results
If a fetch request is made to a server which responds with the following header:
On every platform apart from Android, the following Cookie header is sent in future requests:
but on Android the cookie is parsed incorrectly and the following header is sent (2 cookies):
Snack, code example, screenshot, or link to a repository:
To easily reproduce this, run this express project https://gist.github.com/uen/f8bbded0fc10fd2d3910d388cd3fea6e and load snack https://snack.expo.io/8y09x_Bc7 on a real Android and iOS device (and web). The Android cookie will not be correct as described above