http: Cookie extend

[zekth]

Add delCookie and setCookie.

I have a concern about the multiple Set-Cookie header. It's common to use BUT Headers does not give us the possibility to do this. Do you think any workaround is possible? I think we can keep it like this for the moment and work on multiple Set-Cookie headers handling in a future PR.

Also Days / Months format are hardcoded in the module because ATM there is some discussions about embedding or not the ICU in Deno which may break this module. Ref: denoland/deno#1968

[ry]

Use jsdoc style comments.

[ry]

Regarding the naming of the functions getCookie, setCookie, and delCookie... I wonder if you could research a similar functionality in Go (and Node?) and see what they call these functions? I have the feeling there are better names we could use here.

[zekth]

About the name consistency for the delCookie there is no standard way. Some languages uses SetCookie with expiration date way earlier (like it's done here) or clearCookie / rmCookie / delCookie . Depends on the framework. Personnaly after this research i like clearCookie

For setCookie, example of golang : https://golang.org/pkg/net/http/#SetCookie . But setCookie fits our styleguide.

Finally for getCookie example in golang : readCookies ref: https://golang.org/src/net/http/cookie.go . And concerning node there is no helper for this. For example express add this to its ServerResponse class.

[ry]

Is this a specific date format for cookies? Does it have a name? If it's not just for cookies, consider moving this to //datetime

[zekth]

I was thinking it was specific but no. It's called IMF-fixdate. I'll move it to datetime. Also is date.toIMF() good naming for you?

ref: https://tools.ietf.org/html/rfc7231#section-7.1.1.1

[ry]

Is there a name for this in Go? Perhaps you can borrow that.

[zekth]

No, the only mention of it i can find is IMF-fixdate

[ry]

I think you can call it "RFC1123_GMT"

We don't have the same way of serializing datetime formats as Go. I'd like to slowly move in that direction tho. So I think you can just keep the toIMF() where it is now. But please familiarize yourself with how I think this would be ideally structured:
https://github.com/golang/go/blob/f67f5511ee0f225bcc8943994ba6139eed375e85/src/net/http/server.go#L903-L909
https://github.com/golang/go/blob/f67f5511ee0f225bcc8943994ba6139eed375e85/src/net/http/cookie.go#L200-L204
Consider adding some of the commentary from Go to the jsdoc of "toIMF()"

[ry]

Go doesn't have a corresponding function to delCookie, it just has this:
https://github.com/golang/go/blob/f67f5511ee0f225bcc8943994ba6139eed375e85/src/net/http/cookie.go#L29
Do you think we can also get away with not having delCookie?

[zekth]

@ry about the go Implementation, it's not really common about the manipulation of the MaxAge. We can do both i think, because a lot of frameworks use to have deleteCookie functions too. I'm ok with both implementation but i think it's more easy to use to have a delCookie function.

Just about the toIMF() review. You want me to port the code from golang to the toIMF() function in a future PR?

[ry]

LGTM - nice work

[motss]

@zekth @ry

Correct me if I'm wrong for the following codes:

https://github.com/denoland/deno_std/blob/f1114691038888fc3d8995b64a8028f072569672/http/cookie.ts#L78-L83

In the case of splitting the cookie value with = and joining them back with "", it will form an incorrect value as compared to the original one. For example, 'PREF=al=en-GB&f1=123; wide=1; SID=123'.

The expected output should be

{
  PREF: 'al=en-GB&f1=123',
  wide: '1',
  SID: '123'
}

instead of

{
  PREF: alen-GB&f1123,
  wide: '1',
  SID: '123'
}

[zekth]

@zekth @ry

Correct me if I'm wrong for the following codes:

deno_std/http/cookie.ts

Lines 78 to 83 in f111469

const c = req.headers.get("Cookie").split(";");
for (const kv of c) {
const cookieVal = kv.split("=");
const key = cookieVal.shift().trim();
out[key] = cookieVal.join("");
}
In the case of splitting the cookie value with = and joining them back with "", it will form an incorrect value as compared to the original one. For example, 'PREF=al=en-GB&f1=123; wide=1; SID=123'.

The expected output should be

{
  PREF: 'al=en-GB&f1=123',
  wide: '1',
  SID: '123'
}

instead of

{
  PREF: alen-GB&f1123,
  wide: '1',
  SID: '123'
}

Right, fair analysis. Should be out[key] = cookieVal.join("=");