denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.69k stars 5.34k forks source link

Deno lacks a cookie jar for `fetch` #5862

Open legowerewolf opened 4 years ago

legowerewolf commented 4 years ago

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 commented 4 years ago

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

marcosc90 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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!

jd1378 commented 3 years ago

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)

shivam-tripathi commented 2 years ago

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 commented 2 years ago

@shivam-tripathi use cookie module from deno_std https://deno.land/std@0.116.0/http/cookie.ts

shivam-tripathi commented 2 years ago

@bartlomieju I am guessing you want to point to getCookies function in https://deno.land/std@0.116.0/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 commented 2 years ago

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

lucacasonato commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

One can now use the following as a workaround:

import makeFetchCookie from "npm:fetch-cookie";

const fetchCookie = makeFetchCookie(fetch);
iuioiua commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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