vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.36k stars 26.78k forks source link

Encode isn't an option in Cookie #64346

Open c0d3rm0n opened 5 months ago

c0d3rm0n commented 5 months ago

Link to the code that reproduces this issue

https://github.com/c0d3rm0n/test-set-cookies

To Reproduce

I'm creating an app that uses third party libraries that use Go and set / send their cookies in raw format, and they expect to receive them in the same format.

I use those services in my server actions, and then I have to set the cookies they send inside of response headers. As result of this action, the cookie(s) sent to my server will be set in my browser.

However, the function _cookies().set({...mycookie}) from next/headers will encode the cookie and the value set will not match the original one, causing my calls to those 3rd party libs to fail or give error.

From what I could find, encode should be an option of Cookie, and in next.js modules it is possible to find an interface that includes it (CookieSerializeOptions) but then, the other interfaces that use it don't include encode... So, even if I try to set a cookie like _cookies().set({...mycookie, encode: String}) the browser record will be encoded.

The same happens in Middleware, using _nextResponse.cookies.set({...mycookie}).

Current vs. Expected behavior

I would like to be able to set a cookie and define the encoding function in it's options.

For example, the function _cookies().set({...mycookie, encode: String}) should accept encode as option and save a value like qwerty123= and not qwerty123%3D as it is saving at the moment...

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
Binaries:
  Node: 20.11.0
  npm: 10.2.4
Relevant Packages:
  next: 14.1.4
  eslint-config-next: 14.1.4
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.4.4
Next.js Config:
  output: standalone

Which area(s) are affected? (Select all that apply)

Middleware / Edge (API routes, runtime)

Which stage(s) are affected? (Select all that apply)

next dev (local)

Additional context

No response

xluk9 commented 5 months ago

Yeah, I'm having the same error in my codebase. Lost hours. For now I'm returing them from server action and setting them in client component with document.cookie=.....

c0d3rm0n commented 5 months ago

I ended up creating route handlers (API) so I can serialize the cookies and set them in my response header. This should be possible to do in server actions as one of its intent is to create a shortcut for server handlers instead of creating API routes. For me it is a bit disappointing to create these route handlers to deal with some form submits instead of making all of my 3rd party calls by server actions... I really think that it should be possible to set the encode function as a cookie option. If you want to use something different from the default encode method, why not do it if it is an option that is referred in cookie standard?

ejasarevic commented 3 months ago

Same issue here, would be useful to have a way to specify custom "encode" function whenever we don't want to use encodeURIComponent which is default one.

wyattjoh commented 2 months ago

We're looking into this particular use case now. It may be possible to modify the encode property in the future for the cookies().set(). We're investigating this now.


Bear in mind that RFC2109 requires that the cookie value either be a:

  1. quoted-string (see RFC2109#4.1):

          quoted-string  = ( <"> *(qdtext) <"> )
    
          qdtext         = <any TEXT except <">>
  2. token (see RFC2068#2.2):
          token          = 1*<any CHAR except CTLs or tspecials>

          tspecials      = "(" | ")" | "<" | ">" | "@"
                         | "," | ";" | ":" | "\" | <">
                         | "/" | "[" | "]" | "?" | "="
                         | "{" | "}" | SP | HT

This would mean that if the user sets a cookie via cookies().set(), and specifies a encode: String option, they must quote the string if it contains special characters like =. If these aren't encoded according to the spec, then the risk of other parsers not correctly handling the cookie parsing logic is high.

DrewLandgraveCbsi commented 2 months ago

@wyattjoh any updates on this? We're having to create an api route to set cookies for a client because their backend doesn't decode the cookie string before ingesting.

Frustrating and I'm hoping to get them to update their code but if the cookies().set() method would allow us to override the encoding that would be super helpful

Thanks!

aaronzhongg commented 1 month ago

I was able to work around this issue by manually appending the Set-Cookie header instead of using response.cookies.set(), but I agree that exposing the encode function would be ideal

response.headers.append(
          "Set-Cookie",
          `${cookie.name}=${cookie.value}; Path=/; HttpOnly; Secure; SameSite=None;`,
        );

See mdn docs for more info