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

Astro actions cookies broken, too large sometimes #11675

Closed
1 task
michaelpayne02 opened this issue Aug 12, 2024 · 4 comments · Fixed by #11689
Closed
1 task

Astro actions cookies broken, too large sometimes #11675

michaelpayne02 opened this issue Aug 12, 2024 · 4 comments · Fixed by #11689
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: actions Related to Astro actions (scope)

Comments

@michaelpayne02
Copy link

michaelpayne02 commented Aug 12, 2024

Astro Info

Astro                    v4.13.3
Node                     v20.9.0
System                   macOS (x64)
Package Manager          bun
Output                   hybrid
Adapter                  @astrojs/node
Integrations             @astrojs/sitemap
                         pagefind
                         @astrojs/mdx
                         @astrojs/tailwind

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

When using astro form actions with more than two or three fields being validated, the large url-encoded cookie becomes too big to store in the browser and exceeds the 4096 character limit. It also contains an entire stacktrace. This means the Astro.getActionResult() does not return anything on the source/fallback page (the one in the Referrer header with the original form).

Screenshot 2024-08-12 at 12 10 16 AM

I also get strange 'deleted' cookies that break the entire project permenantly until the stackblitz is restarted.

Screenshot 2024-08-12 at 12 43 04 AM

And this associated stacktrace:

SyntaxError: Unexpected token 'd', "deleted" is not valid JSON
    at JSON.parse (<anonymous>)
    at AstroCookie.json (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/cookies/cookies.js:29:17)
    at eval (/home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/actions/runtime/middleware.js:23:103)
    at applyHandle (/home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:20:22)
    at eval (/home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:17:12)
    at applyHandle (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:31:22)
    at eval (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:50:18)
    at payload (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/i18n/middleware.js:26:34)
    at applyHandle (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:31:22)
    at eval (file:///home/projects/github-g63nyt-zhjdjc/node_modules/astro/dist/core/middleware/sequence.js:28:12)

What's the expected result?

The client should be correctly redirected to the original page with the correct cookies/error messages. It should also work if both the confirmation page needs data and the originating page needs errors.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-g63nyt-zhjdjc?file=README.md

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Aug 12, 2024
@michaelpayne02 michaelpayne02 changed the title Astro actions cookies break Astro actions cookies broken, too large sometimes Aug 12, 2024
@bholmesdev bholmesdev self-assigned this Aug 12, 2024
@bholmesdev bholmesdev added - P4: important Violate documented behavior or significantly impacts performance (priority) and removed needs triage Issue needs to be triaged labels Aug 12, 2024
@bholmesdev
Copy link
Contributor

Thanks for reporting! We can probably find a smaller encoding to cut down on size. The stack is only present in development as well, so we can investigate storing that in-memory or with a separate cookie if needed

@bholmesdev bholmesdev added the feat: actions Related to Astro actions (scope) label Aug 12, 2024
@michaelpayne02
Copy link
Author

Thanks Ben, I appreciate all your work. There's some potential solutions I thought of but I have no idea how feasible they would be to implement:

  • Split the action payload inputErrors into multiple cookies for each field, with errors thrown from the handler also separated. This would cut down on the amount of characters needed to URL encode & double-escape some of the backslashes for the deeply-nested payload object.
  • Inject a script into the page that contains the payload. This is probably more complex of a change and would hinder the NoJS experience.

@ematipico ematipico assigned ematipico and unassigned bholmesdev Aug 13, 2024
@ematipico ematipico added - P3: minor bug An edge case that only affects very specific usage (priority) and removed - P4: important Violate documented behavior or significantly impacts performance (priority) labels Aug 13, 2024
@gaytanmisael
Copy link

gaytanmisael commented Sep 10, 2024

Was this fixed? I'm having problem with the cookie header when I use forms with more than 3 fields running locally where and I'm running Astro 14.5.4......

@bholmesdev
Copy link
Contributor

This should have been fixed in the latest, yes @gaytanmisael. If you're having an problem, feel free to open a new issue with a repro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: actions Related to Astro actions (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants