WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(3.91 KB, patch)
2020-11-19 12:42 PST
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2020-11-19 09:55:17 PST
Created
attachment 414597
[details]
Patch
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
Created
attachment 414611
[details]
Patch
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
<
rdar://problem/71598074
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug