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

Deno lacks a cookie jar for fetch #5862

Open
legowerewolf opened this issue May 26, 2020 · 20 comments
Open

Deno lacks a cookie jar for fetch #5862

legowerewolf opened this issue May 26, 2020 · 20 comments
Labels
ext/fetch related to the ext/fetch suggestion suggestions for new features (yet to be agreed) web related to Web APIs

Comments

@legowerewolf
Copy link

Deno should store cookies on a per-run basis (and optionally have methods in the Deno namespace for export/import of cookies) so that it's possible to make a series of requests that depend on cookies.

Right now, a flow like the following would be impossible:

  1. No cookies sent; response sets cookie 1.
  2. Cookie 1 sent; response uses cookie 1 value somewhere in the body

Yes, I'm aware of std/http/cookie. No, it's not what I need - this is not a server. It's a client.

@ry
Copy link
Member

ry commented May 26, 2020

I agree. Deno ought to behave exactly as a browser does when it comes to the fetch() API.

@bartlomieju bartlomieju added cli related to cli/ dir feat new feature (which has been agreed to/accepted) web related to Web APIs labels May 26, 2020
@marcosc90
Copy link
Contributor

marcosc90 commented May 27, 2020

Right now, a flow like the following would be impossible:

it's currently possible to implement a cookie jar on userland.

const res = await fetch('https://example.com/set-cookie');
const cookies = res.headers.get('set-cookie')
// Implement your jar logic
const headers = new Headers();
headers.set('Cookie', cookies); // get from jar
const res = await fetch('https://example.com/set-cookie', { headers });

It should be easy to implement something like fetch-cookie

@legowerewolf
Copy link
Author

If Deno's fetch and headers APIs match specs, the Cookie header is one of the forbidden header names, and that won't work. I'm surprised that that library works after inspecting it.

@marcosc90
Copy link
Contributor

marcosc90 commented May 27, 2020

It's forbidden on the browser, it wouldn't make sense to make that limitation on a server side HTTP client to be honest.

Deno should match the spec where it makes sense to match it. The same way we had to go around the spec for Set-Cookie header.

@legowerewolf
Copy link
Author

legowerewolf commented May 27, 2020

I fiddled around a bit with Deno and Postman-Echo. Deno sends cookie headers. I must be missing something. (I'm trying to rewrite https://github.com/legowerewolf/ao3-fetch-js for Deno and running into snags in the login process.)

Still, though, an actual built-in cookie jar, the way the browser does it, would be appreciated.

@hackermondev
Copy link

I have the same issue. I am using Deno to send cookie headers and when I test it on postman I can see the cookie header but for some reason, the cookie doesn't work.

@iuioiua
Copy link
Contributor

iuioiua commented Jun 9, 2020

I have the same issue as well. Lack of cookie support for the fetch API is the main reason I haven't been able to move over to deno. This functionality would be great to have!

libetl added a commit to libetl/deno that referenced this issue Aug 7, 2020
@jd1378
Copy link

jd1378 commented Jan 31, 2021

Hi
I made one, you can try it out:
https://deno.land/x/another_cookiejar
(influenced by tough-cookie and fetch-cookie, but took me some time still)

@lucacasonato lucacasonato added ext/fetch related to the ext/fetch suggestion suggestions for new features (yet to be agreed) and removed cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels Feb 2, 2021
@shivam-tripathi
Copy link

shivam-tripathi commented Nov 26, 2021

Parsing cookie in result of fetch is a big pain - res.headers.get('cookie') returns all cookies comma separated - which is impractical to use as there can be comma within the cookie description as well - like Expires=Sat, 26 Nov 2022 17:31:13 GMT. There is no easy way to identify when one set-cookie ends and where the other starts.

<key1>=<value1>; Path=/; Secure; SameSite=None, <key2>=<value2>; Expires=Sat, 26 Nov 2022 17:31:13 GMT; 

Not sure how to parse cookie to extract out all key value pairs safely.

@bartlomieju
Copy link
Member

@shivam-tripathi use cookie module from deno_std https://deno.land/[email protected]/http/cookie.ts

@shivam-tripathi
Copy link

shivam-tripathi commented Nov 26, 2021

@bartlomieju I am guessing you want to point to getCookies function in https://deno.land/[email protected]/http/cookie.ts.
Unfortunately, this method will not work - because it assumes there are semi-colon seperated key value pairs in the cookie string. In fetch's response set cookie string contain not just key value but also other fields like Secure, Expires etc all of them separated by semi colon - which breaks this method. Moreover, all the cookies are comma separated, not semicolon separated.
Right now it's just a hack, but this is what I am doing:

res.headers.get("set-cookie")?.split(",")
      .filter((c) => c.indexOf("GMT") == -1) // till now I've only run into Expires which has additional comma
      .map((c) => c.split(";")[0].split("="))
      .forEach(([k, v]) => this.cookieMap[k.trim()] = v.trim());

It would be best if res.headers.get("cookie") could return an array, but I am unsure it that's possible with the spec.

Edit: Changed headers.get('cookie') to headers.get('set-cookie')

@bartlomieju
Copy link
Member

@shivam-tripathi yes I meant getCookies. @lucacasonato what do you think?

@lucacasonato
Copy link
Member

lucacasonato commented Nov 26, 2021

@shivam-tripathi Are you talking about set-cookie, or cookie? Fields like Expires, Secure are present in set-cookie, but not cookie. To get an unconcatenated list of all set-cookie headers, do the following:

const setCookieHeaders = [...headers.entries()].filter(([k]) => k === 'set-cookie').map(([k, v]) => v);

@shivam-tripathi
Copy link

shivam-tripathi commented Nov 27, 2021

@lucacasonato Hey, yes you're right I meant set-cookie not cookie - res.headers.get('cookie') gives null. Using headers.entries() to filter out all set-cookie headers solved it for me.
Thanks @bartlomieju @lucacasonato!
I had a follow up question: Does concatenation of headers of same header type requires comma , to be used separator according to spec / some internal decision? I can see this can cause problem for some cases (where the value itself has commas).

@garretmh
Copy link

@shivam-tripathi The behavior of Headers#get() is defined by WHATWG.

headers . get(name)
Returns as a string the values of all headers whose name is name, separated by a comma and a space.

https://fetch.spec.whatwg.org/#ref-for-dom-headers-get①

@iuioiua
Copy link
Contributor

iuioiua commented Sep 20, 2022

One can now use the following as a workaround:

import makeFetchCookie from "npm:fetch-cookie";

const fetchCookie = makeFetchCookie(fetch);

@iuioiua
Copy link
Contributor

iuioiua commented Oct 30, 2022

What are the current standings, from both the Deno team and the community, on supporting automatic cookie handling in the Fetch API? Do we want it?

Personally, I'd like to have it implemented, same as the browser and within Deno to ensure correctness and quality. However, it'd come with added complexity and security concerns.

AFAIK, no other server-side JS runtime has this functionality native. There could be good reasons for this, as previously mentioned, but this could be an opportunity for Deno 👀

@phil294
Copy link

phil294 commented Jun 29, 2023

I agree. Deno ought to behave exactly as a browser does when it comes to the fetch() API.

so... shouldn't Deno introduce a new document namespace then so we can do document.cookie = 'a=b;'; fetch(...)? As that is how the browser behaves

@xjfnet
Copy link

xjfnet commented Jul 7, 2023

https://deno.land/x/another_cookiejar/mod.ts

This library offers a fetch wrapper that retain cookies. This library also provides a simple cookiejar;

@garretmh
Copy link

garretmh commented Aug 6, 2023

Could an option be added to Deno.createHttpClient() to enable cookie handling?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext/fetch related to the ext/fetch suggestion suggestions for new features (yet to be agreed) web related to Web APIs
Projects
None yet
Development

Successfully merging a pull request may close this issue.