Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`Fetch` no longer returns all rows of the same-name response headers after RN Upgrade #18837

Closed
Rah1x opened this Issue Apr 13, 2018 · 21 comments

Comments

Projects
None yet
@Rah1x
Copy link

Rah1x commented Apr 13, 2018

Fetch is no longer returning all rows of the same-name response headers! This only just happened after recent upgrade.

Environment

Environment:
OS: Windows 10
Node: 9.2.0
Yarn: 1.3.2
npm: 5.8.0
Watchman: Not Found
Xcode: N/A
Android Studio: Version 3.0.0.0 AI-171.4443003

Packages: (wanted => installed)
react: 16.3.1 => 16.3.1
react-native: ^0.55.3 => 0.55.3

Steps to Reproduce

Server returns the following response

return response()->json(['msg'=>'1'], 200)
->withHeaders(['Content-Type'=>'application/json', 't1'=>[1, 2]]);

** react native code **

let response = await fetch('http://im-sick-and-tired-of-hunting-react-native-bugs',
{
            credentials: 'include',
            method: 'GET',
            cache: 'no-cache',
            mode: 'cors',
            headers: {'Cache-Control': 'no-cache', 'Accept':'*/*', 'Content-Type':'application/json'},
});
console.log(response.headers.get('t1')); 

Expected Behavior

The test hdr (t1) must have both values but only the 2nd one is received. See the response from postman

image

Actual Behavior

image

@Rah1x

This comment has been minimized.

Copy link
Author

Rah1x commented Apr 13, 2018

This btw JUST STARTED!! so dont tell me the code / server etc is wrong.. cause it was working for long while

@ajaykumar97

This comment has been minimized.

Copy link

ajaykumar97 commented Apr 16, 2018

Also have same issue after upgrading RN version 0.53 to 0.55. fetch() is not returning body returned by the server. When I downgraded to RN version 0.53, it works well. Please give the solution.

@Rah1x

This comment has been minimized.

Copy link
Author

Rah1x commented Apr 19, 2018

So ive upgraded to latest commit and still same issue:

Packages: (wanted => installed)
react: 16.3.1 => 16.3.1
react-native: ^0.55.3 => 0.55.3

@ajaykumar97

This comment has been minimized.

Copy link

ajaykumar97 commented Apr 19, 2018

I have just resolved this problem by converting fetch() response into json using response.json() method, which in-turns, returns a promise which returns the actual response from server using .then().

fetch('https://facebook.github.io/react-native/movies.json')
.then((response) => response.json()) <----converting response into json
.then((responseJson) => { <----handeling promise
return responseJson.movies;
})
.catch((error) => {
console.error(error);
});

reference: https://facebook.github.io/react-native/docs/network.html

@Rah1x

This comment has been minimized.

Copy link
Author

Rah1x commented Apr 19, 2018

@ajaykumar97 your problem was with the body and not with the header. So actually you didnt have a problem on this thread, as the body is always there and retrieved using response.json()

@Rah1x Rah1x changed the title Fetch no longer returning all rows of returned header after RN Upgrade `Fetch` no longer returns all rows of the same-name response headers after RN Upgrade Apr 26, 2018

@ciaranw

This comment has been minimized.

Copy link

ciaranw commented Apr 27, 2018

I've also just ran into this problem. It seems to be an issue with the WritableMap that collects the response headers in the translateHeaders() method in com.facebook.react.modules.network.NetworkingModule. The code checks to see if the key exists in the map, but for some reason it returns false. Debugging the code, it calls hasKey() on ReadableNativeMap. This drops down to the local map since mUseNativeAccessor is false, and the local map is empty.

@Rah1x

This comment has been minimized.

Copy link
Author

Rah1x commented Apr 29, 2018

Awesome man @ciaranw thanks for the investigation. I hope someone picks it up and solves it.. As a temporary solution I had to push the CSRF tokens from the api framework into the body instead..

@ciaranw

This comment has been minimized.

Copy link

ciaranw commented May 3, 2018

I suspect the issue was introduced in commit 7891805 but i'm not an expert on android or C++ so I can't be sure!

@jmheik

This comment has been minimized.

Copy link
Contributor

jmheik commented May 18, 2018

Our app was also affected by this when trying to update to 0.55.4. It breaks use case of reading cookies from responses as there's often more than one set-cookie headers in one response.

@Rah1x

This comment has been minimized.

Copy link
Author

Rah1x commented May 20, 2018

@jmheik
Its true, thats exactly what happened to me too. its common to have many set-cookie in a header, but not anymore. You have to now find a way to manually pas and receive the cookie in the body like I did.

@kelvinlawson

This comment has been minimized.

Copy link

kelvinlawson commented Jun 7, 2018

I'm seeing the same thing happening since upgrading from RN 0.52 to 0.54.

Headers being returned by some servers we use have the same fields spread across multiple lines. Since 0.54 it is not possible to talk to these devices any more using RN on Android! RN on iOS is fine.

@ChrisBlood

This comment has been minimized.

Copy link

ChrisBlood commented Jun 7, 2018

Same issue affect our app when moving from RN 0.52 The server we are connecting to returns Basic & Digest authentication headers but in the Android app we only get the Basic authentication header returned. The same servers work OK on iOS

@patricksmith

This comment has been minimized.

Copy link

patricksmith commented Jun 20, 2018

We were affected by this issue after upgrading our Android app from RN 0.53.3 to RN 0.54.4, and I see that it's still an issue in RN 0.55.4. We're using an API that returns multiple Link headers and we're now only receiving one of those headers in our app.

@patricksmith

This comment has been minimized.

Copy link

patricksmith commented Jun 20, 2018

I found a workaround for this issue, but I'm not sure of any additional implications of it. Inspired by comments on a ReadableNativeArray issue, if you add the following lines to MainApplication.java, the duplicate headers are properly parsed:

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

I created a repo to reproduce the issue using 0.55.4: https://github.com/patricksmith/rn-headers. You can see the workaround in this commit: patricksmith/rn-headers@e666aa8. I can't find any documentation on what setUseNativeAccessor is doing, so it may have additional unintended repercussions.

@Rarapony

This comment has been minimized.

Copy link

Rarapony commented Jun 30, 2018

I've been trying to deal with this issue for a while. The fix above seems to do nothing to fix the issue for me. I still only see an array of a single value in the "set-cookie" header.

@mtt87

This comment has been minimized.

Copy link

mtt87 commented Jul 11, 2018

Facing the same issue, spent the morning trying to debug why on iOS we had 3 cookies and on Android only 1 cookie and the problem happens when there are more headers with the same name, on Android it takes only the last one ignoring the others.

An easy way to reproduce, using an express server

    app.get('/test', noCache, (req, res) => {
      res.cookie('mattia1', 'hello');
      res.cookie('mattia2', 'world');
      res.sendStatus(200);
    });

Then on the app

fetch({
  url: 'http://localhost:3000/test'
}).then(res => {
  console.log(res.headers);
})

On Android you get mattia2=world; Path=/
On iOS you get mattia1=hello; Path=/, mattia2=world; Path=/

Only if you control the server the workaround is to not send 2 headers of the same type

In this case with cookies you have to manually set the header with

// doesnt work
// res.cookie('mattia1', 'hello');
// res.cookie('mattia2', 'world');
// works
res.set('Set-Cookie', 'mattia1=hello; Path=/, mattia2=world; Path=/');
@ghost

This comment has been minimized.

Copy link

ghost commented Jul 19, 2018

For me it surprisingly work on the emulator, but not on a real device. The api's I make use of have cookies. There seems to be a problem in sending these cookies(which is part of the header) from a real device, but somehow works on an emulator.

This happens for me on RN 0.56.

@MetalGuardian

This comment has been minimized.

Copy link

MetalGuardian commented Sep 2, 2018

This is still an issue on RN v0.57.0-rc.3

@kanerogers

This comment has been minimized.

Copy link

kanerogers commented Sep 19, 2018

I am having this issue on v.0.57.

@stale

This comment has been minimized.

Copy link

stale bot commented Dec 18, 2018

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale label Dec 18, 2018

@stale

This comment has been minimized.

Copy link

stale bot commented Dec 25, 2018

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

@stale stale bot closed this Dec 25, 2018

@facebook facebook locked as resolved and limited conversation to collaborators Dec 26, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.