WICG / shared-storage

Explainer for proposed web platform Shared Storage API
Other
84 stars 18 forks source link

Spec: Allow cross-origin script in addModule & align createWorklet #161

Open pythagoraskitty opened 2 weeks ago

pythagoraskitty commented 2 weeks ago

We make the spec changes to accompany #158 .

In particular, we make it possible for sharedStorage.worklet.addModule() to create a worklet with cross-origin worklet script. We also align the behavior of sharedStorage.createWorklet() with addmodule() so that it uses the invoking context's origin as the data partition origin by default, while adding a mechanism to use the script's origin instead if specified.

For a more detailed description, please see #158 .


Preview | Diff

pythagoraskitty commented 2 weeks ago

Thanks! Added a few nit comments. However, I think the following two sections also need to be updated:

Updated, could you please take another look? Thanks

pythagoraskitty commented 1 week ago

Looks good % I'm not entirely convinced that relying on destination "sharedstorageworkletcontextorigin" is ideal (also as noted in my Gerrit comment). I recommend getting input from a spec owner.

Thanks, will do. I will either need to add the additional worklet destination type, or else a new field to request, so that I can distinguish requests that we need to check for the "Shared-Storage-Cross-Origin-Worklet-Allowed" header from ones that we don't. As we don't currently have access to the data origin itself from the request, we either need to pass that, or some bit denoting whether it's cross-origin or not.

wanderview commented 5 days ago

Looks good % I'm not entirely convinced that relying on destination "sharedstorageworkletcontextorigin" is ideal (also as noted in my Gerrit comment). I recommend getting input from a spec owner.

Thanks, will do. I will either need to add the additional worklet destination type, or else a new field to request, so that I can distinguish requests that we need to check for the "Shared-Storage-Cross-Origin-Worklet-Allowed" header from ones that we don't. As we don't currently have access to the data origin itself from the request, we either need to pass that, or some bit denoting whether it's cross-origin or not.

I've reviewed this pull request in regards to the destination question. I think it would be preferable to monkey-patch an internal property on request instead.

My reasoning is that "how do we know when to add the header" is an internal implementation detail. In contrast, changing the destination is a change that results in different behavior for web sites. The destination value is exposed to sites a few different ways. So changing the destination for this purpose would require all sites checking the destination to now have to understand an additional enumerated value. In theory this should not be necessary given the information is covered in the newly exposed header.

In addition, using an internal request property is consistent with how the fetch spec tracks whether various headers need to be added.

Another option you could consider, though, is sending the worklet data origin explicitly in a request header. This would maybe benefit the server receiving the request and also give you a comparison you can make to know to add the other header. (Or do you need the other header if you have this?) But I can see reasons why maybe not to do that.

wanderview commented 5 days ago

Also, just for my curiosity, what is the use case for using the origin of the addWorklet URL as the data origin? The linked explainer pull request https://github.com/WICG/shared-storage/pull/158 says cross-origin worklet scripts are motivated by CDN usage. I don't understand a scenario where someone would want data partitioned according to CDN origin.

Is there another use case that isn't documented here?

xyaoinum commented 5 days ago

Also, just for my curiosity, what is the use case for using the origin of the addWorklet URL as the data origin?

The use case for allowing the origin of the script URL as the dataOrigin is to enable third-party scripts to load worklets directly, without requiring an iframe. This is a pre-existing use case (https://github.com/WICG/shared-storage/pull/131) and isn't related to CDN. That is, an ad script could call createWorklet('https://ad.com/script.js') in any context. Without this API, they'd need to create an https://ad.com iframe first, and inside the iframe, call addModule('script.js').

However, neither of the two approaches supports hosting the script on a CDN, as cross-origin script hasn't been allowed for addModule(). This PR removes this limitation, so that CDN script is supported, although it would definitely require running inside the ad iframe.

To align addModule() and createWorlet(), this PR also changes createWorklet()'s default dataOrigin to the calling context's origin. As a result, the network stack needs a separate flag to know cross-origin & shared-storage & script-origin case, whereas before, cross-origin & shared-storage could always imply script-origin.

xyaoinum commented 5 days ago

(@pythagoraskitty Re-requested change per Ben's comment. To monkey-patch an internal property on request, you can refer to the Topics spec)