elysiajs / elysia

Ergonomic Framework for Humans
https://elysiajs.com
MIT License
9.23k stars 198 forks source link

Unable to set more than 1 cookie when using redirect() #637

Closed doroved closed 2 months ago

doroved commented 2 months ago

What version of Elysia.JS is running?

^1.0.18

What platform is your computer?

Darwin 23.4.0 arm64 arm

What steps can reproduce the bug?

If you set more than 1 cookie, it will not be set.

import { Elysia, redirect } from "elysia";

new Elysia()
  .get("/", () => "yay")
  .get("/redirect", ({ cookie: { name, name2 } }) => {
    name.value = 'value'
    name2.value = 'value2'
    return redirect("https://youtu.be/whpVWVWBW4U?&t=8");
  })
  .listen(8080);

What is the expected behavior?

>1 cookies must be successfully set when redirect() is used

What do you see instead?

This is what the server headers look like when 1 or 2 cookies are set image

Additional information

UPD: It turns out that if you use set.redirect, there are no problems.

import { Elysia } from "elysia";

new Elysia()
  .get("/", () => "yay")
  .get("/redirect", ({ cookie: { name, name2 }, set }) => {
    name.value = 'value'
    name2.value = 'value2'

    set.redirect = 'https://youtu.be/whpVWVWBW4U?&t=8'
  })
  .listen(8080);

image

bogeychan commented 2 months ago

@SaltyAom, the main cause is that this function returns a Headers object:

https://github.com/elysiajs/elysia/blob/968650becd19850e4f33dca273d9e5838c937d70/src/handler.ts#L68

https://github.com/elysiajs/elysia/blob/968650becd19850e4f33dca273d9e5838c937d70/src/handler.ts#L152-L153

which cannot be used in such a spreading operation:

https://github.com/elysiajs/elysia/blob/968650becd19850e4f33dca273d9e5838c937d70/src/handler.ts#L201

inherits is allways {}


Is this parsing step even needed?

bogeychan commented 2 months ago

In addition to redirecting, this bug also occurs when returning raw Response objects:

import { Elysia } from "elysia";

new Elysia()
  .get("/", () => "yay")
  .get("/redirect", ({ cookie: { name, name2 }, set }) => {
    name.value = 'value'
    name2.value = 'value2'

    return new Response('yay')
  })
  .listen(8080);
SaltyAom commented 2 months ago

Fixed on https://github.com/elysiajs/elysia/commit/a4d74d5968f461d528a0d682a773cca2f7df0758, published on 1.0.19

doroved commented 2 months ago

Fixed on a4d74d5, published on 1.0.19

I checked the solution to my problem + https://github.com/elysiajs/elysia/issues/637#issuecomment-2107568630, it's ok, thanks