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

Cookie handling in net module #8891

Closed
arantes555 opened this issue Mar 10, 2017 · 13 comments · Fixed by #22704
Closed

Cookie handling in net module #8891

arantes555 opened this issue Mar 10, 2017 · 13 comments · Fixed by #22704

Comments

@arantes555
Copy link

  • Electron version: 1.6.2
  • Operating system: macOS 10.12

Expected behavior

Requests sent with the net module should behave one of two ways regarding cookies.

  • Either not care about cookies at all, and neither store them nor send them back automatically.
  • Or, fully handle cookies, store them when getting a set-cookie header, and send them back when necessary, automatically (with the possibility of an option to disable this behaviour)

Actual behavior

Requests sent with the net module store cookies in the Session used by the request when getting a set-cookie header, but do not send them back afterwards.

How to reproduce

git clone https://github.com/arantes555/electron-net-cookies-testcase.git && cd electron-net-cookies-testcase
npm i
npm start

output:

Server started
REQUEST-HEADERS { 'Accept-Encoding': 'gzip, deflate',
  'Accept-Language': 'fr',
  'User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Electron/1.6.2 Safari/537.36' }
Response SET-COOKIE: undefined
STORED COOKIES: []
REQUEST-HEADERS { 'Accept-Encoding': 'gzip, deflate',
  'Accept-Language': 'fr',
  'User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Electron/1.6.2 Safari/537.36' }
Response SET-COOKIE: a=1,b=1
STORED COOKIES: [{"name":"a","value":"1","domain":"localhost"},{"name":"b","value":"1","domain":"localhost"}]
REQUEST-HEADERS { 'Accept-Encoding': 'gzip, deflate',
  'Accept-Language': 'fr',
  'User-Agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Electron/1.6.2 Safari/537.36' }
Response SET-COOKIE: a=1,b=1

Btw, if I'm reporting multiple issues with the net module these days, it's because I'm working on https://github.com/arantes555/electron-fetch , if you wanna take a look :)

@deepak1556
Copy link
Member

deepak1556 commented Mar 15, 2017

The behavior was initially to save and send cookies from the cookie store. This was changed due to #8223 . Thinking back on it, we should expose LoadFlags api and follow the second option described above.

fully handle cookies, store them when getting a set-cookie header, and send them back when necessary, automatically (with the possibility of an option to disable this behaviour)

/cc @zcbenz @kevinsawicki thoughts ?

@rajbala
Copy link

rajbala commented Apr 22, 2017

I can't figure out how to send a cookie with net.request. Since it doesn't send it automatically, I am trying to set a header like this:

request.setHeader("Cookie", cookies);

cookies is an object of type Cookies.

When I try to send that HTTP request I get a runtime error saying "Error processing argument at index 1, conversion failure from [object Array]"

So what type should the cookies header be set to?

@arantes555
Copy link
Author

@rajbala Yes, you have to use req.setHeader, but the second argument should be a string, not an Object.
To handle cookies, you can use a library like tough-cookie. Using it, you can build a cookie string like this :

cookieJar
        .getCookiesSync(url, { allPaths: true })
        .map(cookie => cookie.cookieString())
        .join('; ')

cookieJar being a ToughCookie cookie jar

@jayesbe
Copy link

jayesbe commented Jan 25, 2018

Tried to get around this using ToughCookie and isomorphic-fetch.

const Tough = require('tough-cookie');
const CookieStore = new Tough.MemoryCookieStore();
const Cookies = new Tough.CookieJar(CookieStore);
const fetch = require('fetch-cookie')(require('isomorphic-fetch'), Cookies);
  return fetch( endpoint , {
    ...options,
    method: 'POST',
    headers: headerData,
    body: bodyData,
    credentials: 'include',
  })
  .then( response => {
  })

And cookies are still not being passed in subsequent requests. I use the above to handle cookies for Jest in a react-native project.

Current version of Electron (1.7)

@arantes555
Copy link
Author

@jayesbe I'm preaching for my choir here, but take a look at https://www.npmjs.com/package/electron-fetch , and arantes555/electron-fetch#3 (comment) . It's not as elegant as having it all automated, but it works

@codebytere
Copy link
Member

We are no longer implementing bugfixes for versions of Electron <= 1.7.x, so i'm going to close this issue but if it is still persisting in more recent versions of Electron we can certainly reopen it!

@Anrock
Copy link
Contributor

Anrock commented Sep 24, 2018

@codebytere reproducible with provided project on 1.8.8, 2.0.10, 3.0.0.

@arantes555
Copy link
Author

I can confirm, the bug is still present on newer electron versions

@codebytere
Copy link
Member

Great! Thanks for the fast response.

@sofianguy
Copy link
Contributor

Thank you for taking the time to report this issue and helping to make Electron better.

The version of Electron you reported this on has been superseded by newer releases.

If you're still experiencing this issue in Electron v4.2.x or later, please add a comment specifying the version you're testing with and any other new information that a maintainer trying to reproduce the issue should know.

I'm setting the blocked/need-info label for the above reasons. This issue will be closed 7 days from now if there is no response.

Thanks in advance! Your help is appreciated.

@sofianguy sofianguy added the blocked/need-info ❌ Cannot proceed without more information label Aug 9, 2019
@arantes555
Copy link
Author

arantes555 commented Aug 9, 2019

@sofianguy I can confirm the issue still happens with all newer major electron versions.

I have tested:

  • 2.0.18
  • 3.1.13
  • 4.2.9
  • 5.0.9

And the issue still happens with all these versions.

I have of course updated the repo with a commit for each electron version.

@sofianguy sofianguy added 4-2-x and removed blocked/need-info ❌ Cannot proceed without more information labels Aug 9, 2019
@arantes555
Copy link
Author

I confirm the problem still happens on electron@6.1.3 and electron@7.0.1. The repo has been updated accordingly.

@zcbenz
Copy link
Member

zcbenz commented Dec 17, 2019

I'm labeling this as a feature instead of bug since this would be a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
5.0.x
Unsorted Issues
Development

Successfully merging a pull request may close this issue.

9 participants