Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug?]: set-cookies from server action's response return being overwritten with flash cookie #1649

Closed
2 tasks done
samualtnorman opened this issue Oct 7, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@samualtnorman
Copy link
Contributor

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Solid Start overwrites set-cookie's from my server action's returned response with flash cookie in no JS mode.

Expected behavior 🤔

For the client to receive my set-cookie headers.

Steps to reproduce 🕹

return redirect("/foo", { headers: { "set-cookie": "foo=bar" } })

in server action

Context 🔦

I'm trying to set a cookie from a server action.

Your environment 🌎

System:
    OS: NixOS 24.05.5518.ecbc1ca8ffd6 (Uakari) x86_64
    CPU: 12th Gen Intel(R) Core(TM) i5-12500 (12) @ 4.60 GHz
Binaries:
    Node: 22.6.0
    PNPM: 9.12.0
npmPackages:
    @solidjs/meta 0.29.4
    @solidjs/router 0.14.7
    @solidjs/start 1.0.8
    solid-js 1.9.1
@samualtnorman samualtnorman added the bug Something isn't working label Oct 7, 2024
@samualtnorman
Copy link
Contributor Author

For other people running into this issue, I am currently using this patch as a workaround:

diff --git a/dist/runtime/server-handler.js b/dist/runtime/server-handler.js
index 9f86080139b32ce71dcc27103ecd26549191cdbc..9aab397f46d729b2a6bc236c6b542018b407e364 100644
--- a/dist/runtime/server-handler.js
+++ b/dist/runtime/server-handler.js
@@ -207,28 +207,28 @@ async function handleServerFunction(h3Event) {
 function handleNoJS(result, request, parsed, thrown) {
     const url = new URL(request.url);
     const isError = result instanceof Error;
-    let redirectUrl = new URL(request.headers.get("referer")).toString();
     let statusCode = 302;
-    if (result instanceof Response && result.headers.has("Location")) {
-        redirectUrl = new URL(result.headers.get("Location"), url.origin + import.meta.env.SERVER_BASE_URL).toString();
-        statusCode = getExpectedRedirectStatus(result);
+    let headers;
+    if (result instanceof Response) {
+        headers = new Headers(result.headers)
+        if (result.headers.has("Location")) {
+            headers.set(`Location`, new URL(result.headers.get("Location"), url.origin + import.meta.env.SERVER_BASE_URL).toString())
+            statusCode = getExpectedRedirectStatus(result);
+        }
+    } else
+        headers = new Headers({ Location: new URL(request.headers.get("referer")).toString() })
+    if (result) {
+        headers.append("Set-Cookie", `flash=${JSON.stringify({
+            url: url.pathname + encodeURIComponent(url.search),
+            result: isError ? result.message : result,
+            thrown: thrown,
+            error: isError,
+            input: [...parsed.slice(0, -1), [...parsed[parsed.length - 1].entries()]]
+        })}; Secure; HttpOnly;`)
     }
     return new Response(null, {
         status: statusCode,
-        headers: {
-            Location: redirectUrl,
-            ...(result
-                ? {
-                    "Set-Cookie": `flash=${JSON.stringify({
-                        url: url.pathname + encodeURIComponent(url.search),
-                        result: isError ? result.message : result,
-                        thrown: thrown,
-                        error: isError,
-                        input: [...parsed.slice(0, -1), [...parsed[parsed.length - 1].entries()]]
-                    })}; Secure; HttpOnly;`
-                }
-                : {})
-        }
+        headers
     });
 }
 let App;

@ryansolid
Copy link
Member

This looks pretty good. I will add it in, but PR is more than welcome next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants