RESOLVED FIXED 219166
Unable to fetch an audio worklet module using a data URL
https://bugs.webkit.org/show_bug.cgi?id=219166
Summary Unable to fetch an audio worklet module using a data URL
Chris Dumez
Reported 2020-11-19 09:53:59 PST
Unable to fetch an audio worklet module using a data URL.
Attachments
Patch (3.92 KB, patch)
2020-11-19 09:55 PST, Chris Dumez
no flags
Patch (3.91 KB, patch)
2020-11-19 12:42 PST, Chris Dumez
no flags
Chris Dumez
Comment 1 2020-11-19 09:55:17 PST
Geoffrey Garen
Comment 2 2020-11-19 10:06:12 PST
Comment on attachment 414597 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414597&action=review > Source/WebCore/workers/WorkerScriptLoader.cpp:133 > + if (fetchOptions.destination != FetchOptions::Destination::Serviceworker && fetchOptions.destination != FetchOptions::Destination::Worker) > + options.sameOriginDataURLFlag = SameOriginDataURLFlag::Set; I tried to think through why the same origin restriction is only required for ServiceWorker and Worker, but I couldn't figure it out. What's the distinction being made here?
Chris Dumez
Comment 3 2020-11-19 10:11:03 PST
(In reply to Geoffrey Garen from comment #2) > Comment on attachment 414597 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=414597&action=review > > > Source/WebCore/workers/WorkerScriptLoader.cpp:133 > > + if (fetchOptions.destination != FetchOptions::Destination::Serviceworker && fetchOptions.destination != FetchOptions::Destination::Worker) > > + options.sameOriginDataURLFlag = SameOriginDataURLFlag::Set; > > I tried to think through why the same origin restriction is only required > for ServiceWorker and Worker, but I couldn't figure it out. What's the > distinction being made here? I am not sure this is correct. I am not sure what's the best condition to use and I asked Youenn for feedback on Slack (He said he would look at it shortly). The flag in question has been dropped from the spec: https://github.com/whatwg/fetch/issues/381
youenn fablet
Comment 4 2020-11-19 11:05:54 PST
Comment on attachment 414597 [details] Patch r=me once we verify there is no issue with the origin of audio worklet for data URLs. View in context: https://bugs.webkit.org/attachment.cgi?id=414597&action=review >>> Source/WebCore/workers/WorkerScriptLoader.cpp:133 >>> + options.sameOriginDataURLFlag = SameOriginDataURLFlag::Set; >> >> I tried to think through why the same origin restriction is only required for ServiceWorker and Worker, but I couldn't figure it out. What's the distinction being made here? > > I am not sure this is correct. I am not sure what's the best condition to use and I asked Youenn for feedback on Slack (He said he would look at it shortly). > > The flag in question has been dropped from the spec: > https://github.com/whatwg/fetch/issues/381 Ideally, we should remove this flag and have specific handling for navigation loads and for dedicated workers. For dedicated workers, the current approach is to do the following as per spec: set worker global scope's cross-origin isolated capability to false. I am not sure what is the rationale here. For audio worklet, how do we compute the origin of the audio worklet? For service workers, there is no point in setting the same origin flag, we should fail any data: load whose destination is service worker. In fact, we should not even hit that code path since the register method should reject on data: URL schemes. For this specific patch, we could just keep the if (fetchOptions.destination != FetchOptions::Destination::Worker) for now with a FIXME stating to remove this flag and implement the spec.
Chris Dumez
Comment 5 2020-11-19 11:09:16 PST
(In reply to youenn fablet from comment #4) > Comment on attachment 414597 [details] > Patch > > r=me once we verify there is no issue with the origin of audio worklet for > data URLs. Could you clarify what you mean here? Why does the origin of an audio worklet matter? Per the WPT test, we should be able to load an audio worklet over a data URL, which is what I was trying to achieve. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=414597&action=review > > >>> Source/WebCore/workers/WorkerScriptLoader.cpp:133 > >>> + options.sameOriginDataURLFlag = SameOriginDataURLFlag::Set; > >> > >> I tried to think through why the same origin restriction is only required for ServiceWorker and Worker, but I couldn't figure it out. What's the distinction being made here? > > > > I am not sure this is correct. I am not sure what's the best condition to use and I asked Youenn for feedback on Slack (He said he would look at it shortly). > > > > The flag in question has been dropped from the spec: > > https://github.com/whatwg/fetch/issues/381 > > Ideally, we should remove this flag and have specific handling for > navigation loads and for dedicated workers. > For dedicated workers, the current approach is to do the following as per > spec: set worker global scope's cross-origin isolated capability to false. > I am not sure what is the rationale here. For audio worklet, how do we > compute the origin of the audio worklet? > > For service workers, there is no point in setting the same origin flag, we > should fail any data: load whose destination is service worker. > In fact, we should not even hit that code path since the register method > should reject on data: URL schemes. > > For this specific patch, we could just keep the if (fetchOptions.destination > != FetchOptions::Destination::Worker) for now with a FIXME stating to remove > this flag and implement the spec.
youenn fablet
Comment 6 2020-11-19 11:18:06 PST
(In reply to Chris Dumez from comment #5) > (In reply to youenn fablet from comment #4) > > Comment on attachment 414597 [details] > > Patch > > > > r=me once we verify there is no issue with the origin of audio worklet for > > data URLs. > > Could you clarify what you mean here? Why does the origin of an audio > worklet matter? Per the WPT test, we should be able to load an audio worklet > over a data URL, which is what I was trying to achieve. If we consider data URL to be same origin, the audio worklet could be considered as having the origin of its frame. That will play if the worklet does a load (fetch, addModule). It could also have an opaque origin, which will impact how the worklet is doing fetch. For instance, is service worker intercepting audio worklet loads (HTTP audio worklet, data URL worklets). It would be nice to validate this behavior and that this aligns with spec and other browsers.
Chris Dumez
Comment 7 2020-11-19 11:32:30 PST
(In reply to youenn fablet from comment #6) > (In reply to Chris Dumez from comment #5) > > (In reply to youenn fablet from comment #4) > > > Comment on attachment 414597 [details] > > > Patch > > > > > > r=me once we verify there is no issue with the origin of audio worklet for > > > data URLs. > > > > Could you clarify what you mean here? Why does the origin of an audio > > worklet matter? Per the WPT test, we should be able to load an audio worklet > > over a data URL, which is what I was trying to achieve. > > If we consider data URL to be same origin, the audio worklet could be > considered as having the origin of its frame. That will play if the worklet > does a load (fetch, addModule). > It could also have an opaque origin, which will impact how the worklet is > doing fetch. > > For instance, is service worker intercepting audio worklet loads (HTTP audio > worklet, data URL worklets). > > It would be nice to validate this behavior and that this aligns with spec > and other browsers. At the moment, this is not something we support. audio worklets cannot do any loads. In the future, we should support recursive addModule() but this is not something we currently support in workers.
Chris Dumez
Comment 8 2020-11-19 12:42:13 PST
Chris Dumez
Comment 9 2020-11-19 12:52:00 PST
Comment on attachment 414611 [details] Patch Clearing flags on attachment: 414611 Committed r270046: <https://trac.webkit.org/changeset/270046>
Chris Dumez
Comment 10 2020-11-19 12:52:02 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 11 2020-11-19 12:53:36 PST
Note You need to log in before you can comment on or make changes to this bug.