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

There should be an adapter for sandboxed SSR. #5231

Closed
AlbertMarashi opened this issue Jun 21, 2022 · 11 comments
Closed

There should be an adapter for sandboxed SSR. #5231

AlbertMarashi opened this issue Jun 21, 2022 · 11 comments
Labels
feature / enhancement New feature or request
Milestone

Comments

@AlbertMarashi
Copy link

Describe the problem

Related discussion: #4339

File-based stores in sveltekit are basically unusable since they leak state across SSR requests. This is a major flaw in the design of SvelteKit and makes it practically unusable compared to other frameworks.

Describe the proposed solution

I have experience playing around with Vue's sand-boxing system that they use to isolate module state from each other using node's vm features. I would be happy to collaborate with someone, with the goal of creating a svelte kit adapter to achieve this.

Using stores like normal JavaScript I find is generally the most convenient and not needing to worry about potential security vulnerabilities between shared user state is a big plus.

image

Many other developers have faced similar issues and there seems to be a consensus that this is needed.

Alternatives considered

People have considered using $page.stuff and contexts but they clear during client-side navigation, destroying state.

Importance

i cannot use SvelteKit without it

Additional Information

No response

@Lenard-0
Copy link

Agreed. I think stores are extremely limited at the moment.

@Romeo-V1
Copy link

Romeo-V1 commented Jun 21, 2022

Yes.

@giray123
Copy link

I see no value on global stores. Looking forward to this.

@dummdidumm dummdidumm added the feature / enhancement New feature or request label Jun 21, 2022
@dummdidumm
Copy link
Member

dummdidumm commented Jun 25, 2022

You said there are sandboxing features in Vue (Edit: Said Vite before, my bad), I didn't find anything about that in their docs though. Could you add details/links?

@DevOfManyThings
Copy link
Contributor

DevOfManyThings commented Jun 25, 2022

@dummdidumm
They said their experience was with Vue's sandboxing, which uses node's vm features

@dummdidumm
Copy link
Member

I did type Vite instead of Vue accidentally, oops. I have read up on these vm features a little, but I didn't find anything in the Vue documentation that they use that to sandbox requests, that's why I'm asking for details.

@AlbertMarashi
Copy link
Author

Looks like they have stopped supporting it, their old docs had a whole section about it and they gave you the option of creating a new runtime context each request

The benefits:

  • no stress about security and leaked state
  • simplified file import stores

The cons:

  • Slight performance overhead

(Personally I preferred the performance overhead, and i'm sure others would too)

Look, it's not the end of the world if we don't have this feature, but I think stores in svelte-kit need some serious work in usability. (idk who decided stuff was a good name)

Here's another suggestion that could potentially be worth exploring:

  • the scripts could run in a different "global" context (but same instance of modules)
  • in each global context, we inject a map which will contain the stores for this request
  • creating, and subscribing to stores involves searching the map for the given stores symbol or ID (and can be used outside of component initialization)
  • each request is using the same modules, but the window/global context isolates the stores from eachother

@AlbertMarashi
Copy link
Author

This is an example of using the VM contexts to provide each request a custom global context to isolate requests from each other, this does not involve creating and compiling new instances of modules and code, so it is guaranteed to be much faster.

It slightly involves changing stores to take in a function that initializes a value, rather than taking in a value as an argument (although deep cloning could potentially allow us to pass in an object type, without needing to change the syntax of stores)

These could be also alternatively provided as SSR safe stores, that are specifically designed for the purposes of avoiding cross-request data leaks

let context = {
	stores: new Map()
}

// create data is just a function that initializes the store per request (when the request subscribes to a store)
function writable(create_data) {
	let store = {
		symbol: Symbol(),
		create_data
	}
	
	store.subscribe = cb => {
		// check if the store is set, otherwise initialise with create data
                // in this example we will just update the store
                // we can store all the subscribers for this particular store in the global context
		context.stores.set(store.symbol, store.create_data())
		cb(context.stores.get(store.symbol)) 
	}
	
	context.stores.set(store.symbol)
	
	return store
}

let store = writable(() => 123)

let new_value = 200;

store.subscribe(val => new_value = val)

console.log(new_value)

@dummdidumm
Copy link
Member

I'm not sure if this is better, because now you have to be super careful not to subscribe, unsubscribe and subscribe again - you would lose the current state. You also have to be careful not to call subscribe in a global file where it's called only once and not per request.

@AlbertMarashi
Copy link
Author

AlbertMarashi commented Jun 28, 2022

I'm not sure if this is better, because now you have to be super careful not to subscribe, unsubscribe and subscribe again - you would lose the current state. You also have to be careful not to call subscribe in a global file where it's called only once and not per request.

Sorry if the example was poor, but in the actual code we would not clear the state. Subscribing multiple times would reuse the same state in the same request, but would isolate requests from one another

@Rich-Harris Rich-Harris added this to the post-1.0 milestone Sep 20, 2022
@AlbertMarashi
Copy link
Author

Closed in favor of sveltejs/svelte#12947

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants