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

Islands have their props uniquely encrypted even if they have no props, leading to uncacheability #12949

Closed
1 task done
kaytwo opened this issue Jan 9, 2025 · 3 comments · Fixed by #12956
Closed
1 task done
Labels
needs triage Issue needs to be triaged

Comments

@kaytwo
Copy link
Contributor

kaytwo commented Jan 9, 2025

Astro Info

Astro                    v5.1.4
Node                     v18.20.3
System                   Linux (x64)
Package Manager          unknown
Output                   static
Adapter                  @astrojs/node
Integrations             none

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

No response

Describe the Bug

According to https://docs.astro.build/en/guides/server-islands/#caching, we should be using cache-control to enforce caching of server island responses. However, prop encryption uses a new IV for every encryption, thus even if you send the same exact props in two different renders of the same server island, they will have different encryptions of the same props, and thus won't be cacheable. See stackblitz for an example - components that don't receive ANY props receive an encryption of the JSON.stringified empty object {}. Because this gets reencrypted with a different IV each time, each GET request has a different query string.

Server islands included from prerendered pages do use the same encryption each time they get downloaded (because they only get rendered once), but that is per-route, and thus you can't cache-invalidate every instance of the same component across different routes because you don't know the right key.

You can see an example in the stackblitz - if you watch the server islands requests in devtools, you'll see that on refresh each of them get served with a new URL each time, even though NoPropIsland has no props to encrypt.

I believe the right way to fix this would be for components with no props, don't send any encrypted props, allowing for direct caching and programmatic cache invalidation.

What's the expected result?

Islands which aren't passed any props should have a deterministic URL, allowing for effective caching and cache invalidation.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-xcrq2rav?file=src%2Fpages%2Findex.astro

Participation

  • I am willing to submit a pull request for this issue.
@JannikWempe
Copy link

I had opened an issue for a similar observation: #12975

But it is not just about server island without any props. How should caching work with server islands that get passed some props, e.g.:

<MyServerIsland server:defer something="test">

This has the same issue as server islands that don't get any props passed: the p query param changes...

@kaytwo
Copy link
Contributor Author

kaytwo commented Jan 13, 2025

I had opened an issue for a similar observation: #12975

But it is not just about server island without any props. How should caching work with server islands that get passed some props, e.g.:

<MyServerIsland server:defer something="test">

This has the same issue as server islands that don't get any props passed: the p query param changes...

IMO (not an Astro team member), any island that receives props should not be cachable because it would open the encrypted props up to having their encryption attacked, whether by replay attacks or other means. I could imagine adding a feature that allows one to opt in to plaintext props for caching reasons, but it would need to be thoughtfully designed to not become a secret leaking footgun.

@JannikWempe
Copy link

I am not saying that the encrypted p should stay the same (not using a random iv). There are other options, e.g. introducing another query param that contains the hashed props (same props = same hash) which could be used for caching.

But I am not highjacking this issue here, which is about no props at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage Issue needs to be triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants