Repository navigation
Fetch only brings latest value of same-name headers on Android, works on iOS. #23005
Description
- [ x ] Review the documentation: https://facebook.github.io/react-native
- [ x ] Search for existing issues: https://github.com/facebook/react-native/issues
- [ x ] Use the latest React Native release: https://github.com/facebook/react-native/releases
This is a long standing bug reported here #18837. Unfortunately #18837 became stale but i can confirm the problem persists.
Environment
React Native Environment Info:
System:
OS: macOS Sierra 10.12.6
CPU: (4) x64 Intel(R) Core(TM) i5-5350U CPU @ 1.80GHz
Memory: 31.03 MB / 8.00 GB
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 10.15.0 - ~/.nvm/versions/node/v10.15.0/bin/node
Yarn: 1.12.3 - /usr/local/bin/yarn
npm: 6.4.1 - ~/.nvm/versions/node/v10.15.0/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
iOS SDK:
Platforms: iOS 11.2, macOS 10.13, tvOS 11.2, watchOS 4.2
Android SDK:
API Levels: 23, 24, 25, 26, 27, 28
Build Tools: 23.0.1, 25.0.2, 26.0.3, 27.0.2, 27.0.3, 28.0.0
System Images: android-28 | Google Play Intel x86 Atom
IDEs:
Android Studio: 3.1 AI-173.4819257
Xcode: 9.2/9C40b - /usr/bin/xcodebuild
npmPackages:
react: 16.6.3 => 16.6.3
react-native: 0.57.8 => 0.57.8
Description
This is a long standing bug reported here #18837 and in different forms in many other places that i havent kept track of, will append later on with an edit.
The issue is that in the presence of multiple headers with the same name as is common with e.g Set-Cookie the android version of fetch will always only ever keep the latest cookie.
So in practice if one wants to get more than one cookies at a time from the response they received they cant do it. There is a workaround which also described on #18837 and essentially introduces the following changes to MainApplication.java:
ReadableNativeArray.setUseNativeAccessor(true);
ReadableNativeMap.setUseNativeAccessor(true);
However I am very uncertain as to what side-effects it might be causing elsewhere. Right now this is what im using but not feeling secure about it.
Reproducible Demo
Reproduction is identical to #18837 found here Reproduction can also be found in 18837 which i run and can confirm.
Activity
Return-1 commentedon Jan 19, 2019
Why this is important
You cant use fetch with any server setting more than 1 cookie 🍪, in many S.O the resolution ended up being reverting to token based auth or custom headers
The fact that it works on iOS is dangerous because one might develop their entire logic around cookie based auth only to find later that this isnt possible on Android having then to perform backend changes as well, maybe even the app is released originally on iOS even.
dulmandakh commentedon Jan 24, 2019
on Android, RN concatenates values separated by comma(,), see https://github.com/facebook/react-native/blob/4936d284df36071047ce776d9e2486c0371f7b97/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java#L606
Return-1 commentedon Jan 24, 2019
@dulmandakh The behaviour you're describing would have been fine but it is not the one I experienced. Only the latest value was present in my case.
Please attempt to reproduce locally and you will see the same. I think the fact that it works with
means that it's a problem with the data structure used maybe?
dulmandakh commentedon Jan 24, 2019
currently it uses ReadableNativeMap, where you must have unique keys. So they concatenate to workaround this limitation. Didn't see iOS code.
Return-1 commentedon Jan 24, 2019
works ok on iOS
i will check again to make sure but i am fairly certain that concatenation isn't working and that only the latest value of the latest header is exposed
Return-1 commentedon Jan 24, 2019
@dulmandakh i've tested this again on a fresh install and i can definitely verify that it doesnt concatenate and only keeps the latest value without adding
on MainApplication.java
without the addition:
"otf_remember_me=some_token; Max-Age=5184000; Expires=Mon, 25-Mar-2019 08:21:17 GMT; Path=/some_path; HttpOnly"with the addition:
"JSESSIONID=some_id; Path=/some_path; HttpOnly, otf_remember_me=some_token; Max-Age=5184000; Expires=Mon, 25-Mar-2019 08:25:56 GMT; Path=/some_path; HttpOnly"I can be of assistance if you need to reproduce and I believe we should re-open this issue
Return-1 commentedon Jan 27, 2019
I can verify this is still an outstanding issue. The fact that i made it work for me shouldnt be reason to abandon this as others can have a really bad time with this in the future when already very deep in development with a server architecture in place that might be hard to change.
Please consider re-opening. This is far from minor in my opinion.
alubeck commentedon Mar 25, 2019
The truth @Return-1 ! I'm experiencing exactly the same, for me only the last
Set-Cookieheader gets properly used and read out on android - no problems on IOS.This solution here #18837 (comment), is working for me, thanks Patrick. But it's highly annoying that this doesn't work :( ... + maybe uncertain side effects this workaround comes with.
christoph-jerolimov commentedon Apr 4, 2019
Hello, I had the same problem here and analysed the internal issue:
The problem is a combination of
NetworkingModule.javaand the usage ofWritableNativeMap. The functiontranslateHeadersmaps the OKHTTP header objekt to a React Native header map. Headers with the same key was overridden because thehasKeymethod always return false.https://github.com/facebook/react-native/blob/9895d011374e655bcaeb390167abafb9c01fef18/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java#L647-L661
That
hasKeyalways return false is an implementation error inReadableNativeMapandWritableNativeMap(which extends the readable map). This native maps supports two different modes, which can be enabled and disabled for all maps with a static booleanuseNativeAccessor.This flag defines if the ReadableNativeMap reads data from a native map or a internal (java.util.) HashMap.
But the problem was that WritableNativeMap always writes into the native map and never uses the (Readable) internal HashMap.
That was the reason why
hasKeyalways return null ifuseNativeAccessorwas false and only one header was returned bytranslateHeaders.Like said by @hey99xx in #21795 (comment) and by @Return-1 #23005 (comment) its possible to activate the this native accessor, if you add this code to your own/project
MainActivity.java:Add this imports:
And this add the beginning of the
createReactActivityDelegatemethod, before your application was loaded.For me this works fine. But notice that this global flag may cause some other troubles.
I also started to fix this behaviour in the latest React Native 0.59.3 release, but notice than that the master version of ReadableNativeMap/WritableNativeMap was already changed.
The commits b257e06 and a062b34 by FB eng @sahrens changed/removed the useNativeAccessor behaviour, so I hope this issue was also fixed with the next/upcoming minor release 0.60.0. 🙌
(I couldn't test the new version yet, because running from the npm package (also with sourcecode changes) works fine for me, but the sourcecode version of RN let fail some of my 3rd party libraries. I'm looking forward for a RC of 0.60.0 and maybe will update this text here then.)