cmorten / opine

Minimalist web framework for Deno ported from ExpressJS.
https://github.com/cmorten/opine/blob/main/.github/API/api.md
MIT License
854 stars 43 forks source link

res.cookie maxAge can't be 0 #125

Closed 4ov closed 3 years ago

4ov commented 3 years ago

Issue

Setup:

Details

res.cookie's maxAge option can be 0 in express while it's giving an error in opine

error: Uncaught (in promise) DenoStdInternalError: Max-Age must be an integer superior to 0
  if (!expr) {
           ^
    at assert (https://deno.land/std@0.97.0/_util/assert.ts:12:12)
    at toString (https://deno.land/std@0.97.0/http/cookie.ts:60:5)
    at setCookie (https://deno.land/std@0.97.0/http/cookie.ts:180:13)
    at Response1.cookie (https://deno.land/x/opine@1.4.0/src/response.ts:142:5)
cmorten commented 3 years ago

Hey @4ov 👋

Looks like this error is coming from the std lib.

REF: https://deno.land/std@0.98.0/http/cookie.ts#L60

The git blame takes you to a PR where the author states that 0 is valid ( well, says negative values are invalid ) according to the RFC ( https://github.com/denoland/deno_std/pull/359#discussion_r279169244 ).

Indeed, the RFC states:

Max-Age=delta-seconds Optional. The Max-Age attribute defines the lifetime of the cookie, in seconds. The delta-seconds value is a decimal non- negative integer. After delta-seconds seconds elapse, the client should discard the cookie. A value of zero means the cookie should be discarded immediately.

REF: https://www.ietf.org/rfc/rfc2109.txt

So think this is a mistake in the http std lib - we should also raise an issue there. 😄

asos-craigmorten commented 3 years ago

This has been fixed upstream, awaiting next deno_std release.

REF: https://github.com/denoland/deno_std/pull/992

4ov commented 3 years ago

So we are good here, Thanks.