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

[Android] Image loading request is performed without cookie which was set earlier by "SetCookie" header of response #13630

Closed
andrey-skl opened this issue Apr 22, 2017 · 15 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@andrey-skl
Copy link

andrey-skl commented Apr 22, 2017

Description

  1. I have a server which returns image only if request has authorization cookie.
  2. I authorise on this server and get a HTTP response with Set-Cookie header with cookies in content.
  3. Then I show an Image with URL pointing to this server. Image is failed to load. Same thing works OK on iOS.

I checked the network traffic, and found this:

  1. Response which sets the cookie looks like this:
HTTP/1.1 200 OK
Server: nginx
Date: Sat, 22 Apr 2017 16:43:31 GMT
Content-Type: application/json;charset=utf-8
X-XSS-Protection: 1; mode=block
X-Frame-Options: SAMEORIGIN
X-Content-Type-Options: nosniff
Set-Cookie: JSESSIONID={Censored};Path=/somepath;Secure;HttpOnly
Expires: Thu, 01 Jan 1970 00:00:00 GMT
Access-Control-Expose-Headers: Location
Cache-Control: no-cache, no-store, no-transform, must-revalidate
Vary: Accept-Encoding, User-Agent
Content-Encoding: gzip
Strict-Transport-Security: max-age=31536000; includeSubdomains;
X-Content-Type-Options: nosniff
Transfer-Encoding: chunked
Connection: Keep-alive
  1. Regular fetch-drived network requests after this are performed with this cookie:
GET {Censored/some url} HTTP/1.1
content-type: application/json
accept: application/json, text/plain, */*
authorization: {Censored}
Host: {Censored}
Connection: Keep-Alive
Accept-Encoding: gzip
Cookie: JSESSIONID={Censored/HERE IT IS!!}
User-Agent: okhttp/3.4.1
  1. Image requests are performed without this cookie:
GET {Censored}/_persistent/project-diff.png?file=96-37&c=false HTTP/1.1
Host: {Censored}
Connection: Keep-Alive
Accept-Encoding: gzip
User-Agent: okhttp/3.4.1

Solution

The code which loads image should respect that cookie set before.

Additional Information

@andrey-skl
Copy link
Author

I'm pretty sure there is code which implements this, but is it working?

CookieJarContainer container = (CookieJarContainer) client.cookieJar();

@cbjs
Copy link

cbjs commented Jun 19, 2017

same +1

@keith-kurak
Copy link

I'm having this issue, as well.

  • 1

@keith-kurak
Copy link

I think I fixed this on my end. I had a CRNA project that I had just upgraded to Expo v.18. I was serving it from my computer via the default "react-native-scripts" that CRNA automatically puts in package.json, but then I was publishing it via the "exp" CLI. When I would run it via CRNA, it would seemingly throw all of Expo on my Android phone into this state where it was using another stack for image HTTP requests. If I killed Expo entirely on my phone and published again via exp everything would work. When sniffing the network traffic, I noticed that the bad requests had a user-agent of "Davlik 2.1.0/ ..." while the good ones (image requests and otherwise) would use "okhttp". Now that I'm doing all of my testing and publishing with the exp commands, things seem to be OK.

@rakeshgajula
Copy link

I am on "react-native": "0.43.4" and having the same issue.

Image requests on Android are missing cookies

@lechinoix
Copy link

Same here with react-native 0.46.4

@pull-bot
Copy link

pull-bot commented Oct 9, 2017

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

  • Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
  • If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
  • Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

@hramos hramos added the Icebox label Oct 9, 2017
@hramos hramos closed this as completed Oct 9, 2017
@andrey-skl
Copy link
Author

Sadly that's the way how react-native evolves: it doesn't care about great bunch of small but breaking stuff issues there and there. Will it get some kind of polishing and bugfixing eventually? Nobody knows 😞

@hramos
Copy link
Contributor

hramos commented Oct 10, 2017

We do care! With the amount of open issues and PRs, we do focus on more active issues first. After two months, no PR has been opened to fix this, so it probably is not a huge blocker. Feel free to send a PR with polish and bug fixes :)

@joeldalley
Copy link

joeldalley commented Dec 19, 2017

@hramos This is a pretty significant problem for us. We're using React 16 and React Native 0.50.0. I notice that in my AJAX requests on the Android platform, it doesn't matter if I employ the withCredentials request option or not--Android simply doesn't pass cookies along w/ the request. I'm currently struggling to think of a way to ship our app next month with Android not respecting our directive to use cookies. We observe this breakdown by reading the requests that arrive at our server, where the iOS requests have the expected cookies, and Android does not.

EDIT: this note has nothing to do with image loading -- it's just AJAX requests in general, using React Native on the Android platform. Cookies are never sent with any HTTP requests.

@keith-kurak
Copy link

We're discussing this issue further in the Expo client Github (expo/expo#370). We can reproduce the issue with Image HTTP requests using the Davlik user agent (instead of Okhttp) after an Expo app's activity is closed and reopened. If anybody has any suggestion on how to reproduce a similar situation (loading a second activity in the same process running the same JS) in base RN, I'd be happy to give it a try.

@dracostheblack
Copy link

We're seeing the same issue. We have an auth cookie and it's not getting passed so the image will not display.

@andrey-skl
Copy link
Author

Have worked around it by passing authorization every time via "headers" value of Image's source property, but this is really ugly.

https://github.com/JetBrains/youtrack-mobile/blob/master/src/views/show-image/show-image.js#L41

@dracostheblack
Copy link

@huston007 I'm passing it like image = {uri: imageName, headers: imageHeaders, cache: 'force-cache'}, where imageHeaders has 'Cookie': authCookie as part of it. Is what you're doing different, it looks the same.

@ghost
Copy link

ghost commented Aug 27, 2018

Does not work in 0.56.0 as well.

facebook-github-bot pushed a commit that referenced this issue Oct 6, 2018
Summary:
This sync includes the following changes:
- **[d83601080](facebook/react@d83601080)**: Wrap retrySuspendedRoot using SchedulerTracing (#13776) //<Sophie Alpert>//
- **[40a521aa7](facebook/react@40a521aa7)**: Terminology: Functional -> Function Component (#13775) //<Dan Abramov>//
- **[605ab10a4](facebook/react@605ab10a4)**: Add envify transform to scheduler package (#13766) //<Michael Ridgway>//
- **[acc7f404c](facebook/react@acc7f404c)**: Restart from root if promise pings before end of render phase (#13774) //<Andrew Clark>//
- **[cbc224028](facebook/react@cbc224028)**: fix - small misspelling (#13768) //<Spencer Davies>//
- **[4eabeef11](facebook/react@4eabeef11)**: Rename ReactSuspenseWithTestRenderer-test -> ReactSuspense-test //<Andrew Clark>//
- **[95a3e1c2e](facebook/react@95a3e1c2e)**: Rename ReactSuspense-test -> ReactSuspenseWithNoopRenderer-test //<Andrew Clark>//
- **[96bcae9d5](facebook/react@96bcae9d5)**: Jest + test renderer helpers for concurrent mode (#13751) //<Andrew Clark>//
- **[5c783ee75](facebook/react@5c783ee75)**: Remove unreachable code (#13762) //<Heaven>//
- **[36c5d69ca](facebook/react@36c5d69ca)**: Always warn about legacy context within StrictMode tree (#13760) //<Brian Vaughn>//
- **[3e9a5de88](facebook/react@3e9a5de88)**: UMD react-cache build (#13761) //<Maksim Markelov>//
- **[8315a30b9](facebook/react@8315a30b9)**: --save is no longer needed (#13756) //<Joe Cortopassi>//
- **[ce96e2df4](facebook/react@ce96e2df4)**: Rename simple-cache-provider to react-cache (#13755) //<Andrew Clark>//
- **[c5212646f](facebook/react@c5212646f)**: Removed extra typeof checks for contextType.unstable_read (#13736) //<Brian Vaughn>//
- **[806eebdae](facebook/react@806eebdae)**: Enable getDerivedStateFromError (#13746) //<Brian Vaughn>//
- **[a0733fe13](facebook/react@a0733fe13)**: pure (#13748) //<Andrew Clark>//
- **[4d17c3f05](facebook/react@4d17c3f05)**: [scheduler] Improve naive fallback version used in non-DOM environments //<Andrew Clark>//
- **[469005d87](facebook/react@469005d87)**: Revise `AttributeType` React Native Flow Type (#13737) //<Timothy Yung>//
- **[0dc0ddc1e](facebook/react@0dc0ddc1e)**: Rename AsyncMode -> ConcurrentMode (#13732) //<Dominic Gannaway>//
- **[7601c3765](facebook/react@7601c3765)**: Ensure "addEventListener" exists on "window" for "scheduler" package (#13731) //<Dominic Gannaway>//
- **[d0c0ec98e](facebook/react@d0c0ec98e)**: Added a PureComponent contextType test (#13729) //<Brian Vaughn>//
- **[4b68a6498](facebook/react@4b68a6498)**: Support class component static contextType attribute (#13728) //<Brian Vaughn>//
- **[f305d2a48](facebook/react@f305d2a48)**: [scheduler] Priority levels, continuations, and wrapped callbacks (#13720) //<Andrew Clark>//
- **[970a34bae](facebook/react@970a34bae)**: Bump babel-eslint and remove flow supressions (#13727) //<Brian Ng>//
- **[13965b4d3](facebook/react@13965b4d3)**: Interaction tracking ref-counting bug fixes (WIP) (#13590) //<Brian Vaughn>//
- **[17e703cb9](facebook/react@17e703cb9)**: Restore global window.event after event dispatching (#13688) (#13697) //<Sergei Startsev>//
- **[a775a767a](facebook/react@a775a767a)**: Remove redundant logic (#13502) //<Heaven>//
- **[e1a067dea](facebook/react@e1a067dea)**: Fix circular dependency in TracingSubscriptions (#13689) //<Maksim Markelov>//
- **[518812eeb](facebook/react@518812eeb)**: Clarify comment (#13684) //<Heaven>//
- **[eeb817785](facebook/react@eeb817785)**: Remove some old files from stats //<Dan>//
- **[7ea3ca1d1](facebook/react@7ea3ca1d1)**: Rename schedule to scheduler (#13683) //<Dan Abramov>//
- **[bec2ddaf1](facebook/react@bec2ddaf1)**: Update bundle sizes for 16.5.2 release //<Brian Vaughn>//
- **[4269fafb0](facebook/react@4269fafb0)**: Updating package versions for release 16.5.2 //<Brian Vaughn>//
- **[4380f9ba1](facebook/react@4380f9ba1)**: Revert "Updating package versions for release 16.6.0-alpha.0" //<Brian Vaughn>//
- **[72fad84e7](facebook/react@72fad84e7)**: Revert "Updating dependencies for react-noop-renderer" //<Brian Vaughn>//
- **[c3fad5acf](facebook/react@c3fad5acf)**: Revert "Update bundle sizes for 16.6.0-alpha.0 release" //<Brian Vaughn>//
- **[dd9120561](facebook/react@dd9120561)**: Kepp calling peformWork consistent (#13596) //<Heaven>//
- **[42d12317a](facebook/react@42d12317a)**: Update bundle sizes for 16.6.0-alpha.0 release //<Brian Vaughn>//
- **[489614c4f](facebook/react@489614c4f)**: Updating dependencies for react-noop-renderer //<Brian Vaughn>//
- **[351c9015c](facebook/react@351c9015c)**: Updating package versions for release 16.6.0-alpha.0 //<Brian Vaughn>//
- **[a210b5b44](facebook/react@a210b5b44)**: Revert "Do not bind topLevelType to dispatch" (#13674) //<Dan Abramov>//
- **[1d8a75fef](facebook/react@1d8a75fef)**: remove flow typings from Schedule.js (#13662) //<Alexey Raspopov>//
- **[d92114b98](facebook/react@d92114b98)**: Resubmit: Fix updateWrapper causing re-render textarea, even though their data (#13643) //<Nathan Hunzaker>//
- **[0c9c591bf](facebook/react@0c9c591bf)**: Do not bind topLevelType to dispatch (#13618) //<Nathan Hunzaker>//
- **[9f819a5ea](facebook/react@9f819a5ea)**: [schedule] Refactor Schedule, remove React-isms (#13582) //<Andrew Clark>//
- **[9c961c0a2](facebook/react@9c961c0a2)**: Fix some iframe edge cases (#13650) //<Jérôme Steunou>//
- **[8bc0bcabe](facebook/react@8bc0bcabe)**: Add UMD production+profiling entry points (#13642) //<Brian Vaughn>//
- **[b488a5d9c](facebook/react@b488a5d9c)**: Fix test comment typo (#13568) //<Heaven>//
- **[4bcee5621](facebook/react@4bcee5621)**: Rename "tracking" API to "tracing" (#13641) //<Brian Vaughn>//
- **[72217d081](facebook/react@72217d081)**: Update bundle sizes for 16.5.1 release //<Dan Abramov>//
- **[8b93a60c5](facebook/react@8b93a60c5)**: Updating package versions for release 16.5.1 //<Dan Abramov>//
- **[ecbf7af40](facebook/react@ecbf7af40)**: Enhance dev warnings for forwardRef render function (#13627) (#13636) //<Andres Rojas>//
- **[228240085](facebook/react@228240085)**: Delete TapEventPlugin (#13630) //<Dan Abramov>//
- **[a079011f9](facebook/react@a079011f9)**: ð Stop syncing the value attribute on inputs (behind a feature flag) (#13526) //<Nathan Hunzaker>//
- **[a7bd7c3c0](facebook/react@a7bd7c3c0)**: Allow reading default feature flags from bundle tests (#13629) //<Dan Abramov>//
- **[d3bbfe09c](facebook/react@d3bbfe09c)**: Fix IE version in comment //<Dan Abramov>//
- **[1b2646a40](facebook/react@1b2646a40)**: Fix warning without stack for ie9 (#13620) //<Aliaksandr Manzhula>//
- **[e49f3ca08](facebook/react@e49f3ca08)**:  honor displayName set on ForwardRef if available (#13615) //<Evan Jacobs>//

Release Notes:
[GENERAL] [FEATURE] [React] - React sync for revisions ade5e69...d836010

Reviewed By: bvaughn

Differential Revision: D10118547

fbshipit-source-id: ecde7ada80331abdc8bd7d279e0f3dbe9acde071
@facebook facebook locked as resolved and limited conversation to collaborators Oct 9, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

11 participants