Fetch only brings latest value of same-name headers on Android, works on iOS.

[Return-1]

• [ 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.

[Return-1]

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]

on Android, RN concatenates values separated by comma(,), see

react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java

Line 606 in 4936d28

// multiple values for the same header

[Return-1]

@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

ReadableNativeArray.setUseNativeAccessor(true);
ReadableNativeMap.setUseNativeAccessor(true);

means that it's a problem with the data structure used maybe?

[dulmandakh]

currently it uses ReadableNativeMap, where you must have unique keys. So they concatenate to workaround this limitation. Didn't see iOS code.

[Return-1]

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]

@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

ReadableNativeArray.setUseNativeAccessor(true);
ReadableNativeMap.setUseNativeAccessor(true);

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]

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]

The truth @Return-1 ! I'm experiencing exactly the same, for me only the last Set-Cookie header 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]

Hello, I had the same problem here and analysed the internal issue:

The problem is a combination of NetworkingModule.java and the usage of WritableNativeMap. The function translateHeaders maps the OKHTTP header objekt to a React Native header map. Headers with the same key was overridden because the hasKey method always return false.

react-native/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java

Lines 647 to 661 in 9895d01

	private static WritableMap translateHeaders(Headers headers) {
	WritableMap responseHeaders = Arguments.createMap();
	for (int i = 0; i < headers.size(); i++) {
	String headerName = headers.name(i);
	// multiple values for the same header
	if (responseHeaders.hasKey(headerName)) {
	responseHeaders.putString(
	headerName,
	responseHeaders.getString(headerName) + ", " + headers.value(i));
	} else {
	responseHeaders.putString(headerName, headers.value(i));
	}
	}
	return responseHeaders;
	}

That hasKey always return false is an implementation error in ReadableNativeMap and WritableNativeMap (which extends the readable map). This native maps supports two different modes, which can be enabled and disabled for all maps with a static boolean useNativeAccessor.

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 hasKey always return null if useNativeAccessor was false and only one header was returned by translateHeaders.

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:

import com.facebook.react.bridge.ReadableNativeArray;
import com.facebook.react.bridge.ReadableNativeMap;

And this add the beginning of the createReactActivityDelegate method, before your application was loaded.

        ReadableNativeArray.setUseNativeAccessor(true);
        ReadableNativeMap.setUseNativeAccessor(true);

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.)