Issue metadata
Sign in to add a comment
|
MediaElementAudioSource CORS access restriction - incorrect behavior |
||||||||||||||||||||||
Issue descriptionGo to: https://rawgit.com/GoogleChrome/omnitone/master/examples/foa-renderer.html Press "Play" - The current stable (70.0.3538.77) produces the audio correctly. - Chrome Canary (72.0.3599.0) produces an error below. "MediaElementAudioSource outputs zeroes due to CORS access restrictions for https://rawgit.com/GoogleChrome/omnitone/master/examples/resources/4ch_B_FuMaNorm_FuMaOrd_speech.wav"
,
Nov 3
https://chromium-review.googlesource.com/c/chromium/src/+/1238098
,
Nov 5
Sorry, this issue is a regression introduced by https://chromium-review.googlesource.com/c/chromium/src/+/1238098. I will fix it, but I don't know how to write proper webaudio layout tests or web platform tests. hongchan@, can you write a failing wpt or layout test?
,
Nov 5
I'll write a layout test and share it with you. Would this be the same issue as well? https://bugs.chromium.org/p/chromium/issues/detail?id=899745
,
Nov 5
yhirano@ I did a little digging on the issue, and it's not reproducible on run_blink_wptserve.py server. From the test server, the audio is being played without any problem. I think this is only reproducible from the real-world server configuration. Do you have a POC fix? I would like to build locally and test it against Omnitone and Resonance Audio. If that confirms the fix, perhaps we can land the fix without a layout test?
,
Nov 6
I guess you can verify the input buffer like https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-audio-tainting.https.html. That is a "tainted" case, so for the test of this issue the expectation would be contrary.
,
Nov 6
I know how to test/verify this case, and I ran my test case with blink_wptserve.py. However, I couldn't reproduce the same error with our test server. FWIW, the test you pointed out is pretty similar except for service worker and iframe. Let me see what I can do with it. (FYI, ScriptProcessorNode is deprecated from the spec, so we should not use it in the test...)
,
Nov 6
yhirano@ I uploaded a test CL so you can take a look.
,
Nov 7
Hm, I see "Uncaught RangeError: Source is too large" in the trybot output, and I see it when I run the test via the browser as well. I think cross origin redirect is critical for this issue, but I don't see it in the test.
,
Nov 7
Yes, I think that's why I can't reproduce it with the test. How do I "redirect" in the test on the wptserver? I will take a look at the range error.
,
Nov 7
This issue seems to be affecting many sites using MediaElement with WebAudio. I might have to remove the WouldTaintOrigin() check if we can't find the fix soon.
,
Nov 8
No, simply removing the check will be a significant security problem. You can create a redirect using wpt/fetch/api/resources/redirect.py. See wpt/fetch/api/cors/cors-redirect.any.js for example.
,
Nov 8
OK, https://chromium-review.googlesource.com/c/chromium/src/+/1320851/3 is not failing. I will make it pass.
,
Nov 8
s/not/now/ Sorry!
,
Nov 8
Well, now I am even more confused. The current test doesn't do redirection. I thought the root cause of this issue is the incorrect behavior of "WouldTaintOrigin" on redirected URLs. Was I wrong? In any case, hope we can fix this soon!
,
Nov 8
audioElement.src = host_info.HTTPS_REMOTE_ORIGIN + '/fetch/api/resources/redirect.py?location=' + encodeURIComponent(host_info.HTTPS_ORIGIN + '/webaudio/resources/4ch-440.wav?pipe=header(access-control-allow-origin,*)'); performs redirect.
,
Nov 8
I'm adding ReleaseBlock-Stable because of #11, "This issue seems to be affecting many sites using MediaElement with WebAudio". hongchan@, feel free to remove the label if you don't think this issue should block stable. A fix is proposed at https://chromium-review.googlesource.com/c/chromium/src/+/1325281.
,
Nov 8
,
Nov 8
Issue 899745 has been merged into this issue.
,
Nov 8
M71 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.
,
Nov 8
,
Nov 8
This is also causing an issue on SoundCloud's public API, which uses a 302 redirect. https://github.com/soundcloud/soundcloud-javascript/issues/91#issuecomment-436758591
,
Nov 8
,
Nov 8
<Bulk edit> Reminder M71 Stable is approaching. Please review this bug and assess if this is indeed a RBS. If not, please remove the RBS label. If so, please make sure any planned work will be tested in Beta and verified before the Stable date. Thanks
,
Nov 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/657a055a66492e996b296d7bb999e3c8551eed23 commit 657a055a66492e996b296d7bb999e3c8551eed23 Author: Yutaka Hirano <yhirano@chromium.org> Date: Fri Nov 09 00:52:55 2018 [media] Treat cross-origin redirect as TAINTED only for no-cors requests With https://crrev.com/a9cbaa7a40e2b2723cfc2f266c42f4980038a949, WebMediaPlayer blindly treats a resource experiencing cross-origin redirects as TAINTED. In fact, it should be treated as TAINTED only when its request mode is "no-cors". The added tests are provided by hongchan@chromium.org. Bug: 899745 , 901383 Change-Id: Idb66407552085b053818f3e4a9d8d5ff3ddeaf45 Reviewed-on: https://chromium-review.googlesource.com/c/1325281 Reviewed-by: Hongchan Choi <hongchan@chromium.org> Reviewed-by: Fredrik Hubinette <hubbe@chromium.org> Commit-Queue: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/heads/master@{#606681} [modify] https://crrev.com/657a055a66492e996b296d7bb999e3c8551eed23/media/blink/multibuffer_data_source.cc [modify] https://crrev.com/657a055a66492e996b296d7bb999e3c8551eed23/media/blink/multibuffer_data_source.h [modify] https://crrev.com/657a055a66492e996b296d7bb999e3c8551eed23/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/657a055a66492e996b296d7bb999e3c8551eed23/media/blink/webmediaplayer_impl.h [add] https://crrev.com/657a055a66492e996b296d7bb999e3c8551eed23/third_party/WebKit/LayoutTests/external/wpt/webaudio/js/worklet-recorder.js [add] https://crrev.com/657a055a66492e996b296d7bb999e3c8551eed23/third_party/WebKit/LayoutTests/external/wpt/webaudio/resources/4ch-440.wav [add] https://crrev.com/657a055a66492e996b296d7bb999e3c8551eed23/third_party/WebKit/LayoutTests/external/wpt/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/cors-check.https.html [add] https://crrev.com/657a055a66492e996b296d7bb999e3c8551eed23/third_party/WebKit/LayoutTests/external/wpt/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/no-cors.https.html
,
Nov 9
Although the fix has just been landed, I'm requesting merge for M71 before baking the change in Canary and/or Dev. I can wait, but I don't know the release schedule, so I'd like to hear release manager's opinion. The bug is a regression of CORS redirect for media resources (i.e., video and audio). I cannot exactly assess the impact, but SoundCloud is reportedly affected by the bug (see #22). hongchan@ may be able to assess the impact more correctly. The fix is relatively small and safe to merge.
,
Nov 9
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 9
yhirano@ If ToT fixes the no-audio problem in the repro case (in the description), I second the merge request as well. I will check with tomorrow's Canary. The impact on SoundCloud is confirmed (#22). Also Omnitone and Resonance Audio SDK are also broken. Many VR audio demos rely on this feature.
,
Nov 9
Pls update bug with canary result tomorrow.
,
Nov 9
The NextAction date has arrived: 2018-11-09
,
Nov 9
govind@ Just confirmed the fix on 72.0.3606.0. It plays the audio correctly now.
,
Nov 9
Approving merge to M71 branch 3578 based on comment #26, #28 and #31. Please merge ASAP. And mark the bug as fixed if nothing else is pending after the merge. Thank you.
,
Nov 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c3aa434fe1fc3c69fd86b03267c7cae747c9440b commit c3aa434fe1fc3c69fd86b03267c7cae747c9440b Author: Yutaka Hirano <yhirano@chromium.org> Date: Fri Nov 09 16:47:20 2018 [media] Treat cross-origin redirect as TAINTED only for no-cors requests With https://crrev.com/a9cbaa7a40e2b2723cfc2f266c42f4980038a949, WebMediaPlayer blindly treats a resource experiencing cross-origin redirects as TAINTED. In fact, it should be treated as TAINTED only when its request mode is "no-cors". The added tests are provided by hongchan@chromium.org. TBR=yhirano@chromium.org (cherry picked from commit 657a055a66492e996b296d7bb999e3c8551eed23) Bug: 899745 , 901383 Change-Id: Idb66407552085b053818f3e4a9d8d5ff3ddeaf45 Reviewed-on: https://chromium-review.googlesource.com/c/1325281 Reviewed-by: Hongchan Choi <hongchan@chromium.org> Reviewed-by: Fredrik Hubinette <hubbe@chromium.org> Commit-Queue: Yutaka Hirano <yhirano@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#606681} Reviewed-on: https://chromium-review.googlesource.com/c/1329801 Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#613} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/c3aa434fe1fc3c69fd86b03267c7cae747c9440b/media/blink/multibuffer_data_source.cc [modify] https://crrev.com/c3aa434fe1fc3c69fd86b03267c7cae747c9440b/media/blink/multibuffer_data_source.h [modify] https://crrev.com/c3aa434fe1fc3c69fd86b03267c7cae747c9440b/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/c3aa434fe1fc3c69fd86b03267c7cae747c9440b/media/blink/webmediaplayer_impl.h [add] https://crrev.com/c3aa434fe1fc3c69fd86b03267c7cae747c9440b/third_party/WebKit/LayoutTests/external/wpt/webaudio/js/worklet-recorder.js [add] https://crrev.com/c3aa434fe1fc3c69fd86b03267c7cae747c9440b/third_party/WebKit/LayoutTests/external/wpt/webaudio/resources/4ch-440.wav [add] https://crrev.com/c3aa434fe1fc3c69fd86b03267c7cae747c9440b/third_party/WebKit/LayoutTests/external/wpt/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/cors-check.https.html [add] https://crrev.com/c3aa434fe1fc3c69fd86b03267c7cae747c9440b/third_party/WebKit/LayoutTests/external/wpt/webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/no-cors.https.html
,
Nov 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c3aa434fe1fc3c69fd86b03267c7cae747c9440b Commit: c3aa434fe1fc3c69fd86b03267c7cae747c9440b Author: yhirano@chromium.org Commiter: yhirano@chromium.org Date: 2018-11-09 16:47:20 +0000 UTC [media] Treat cross-origin redirect as TAINTED only for no-cors requests With https://crrev.com/a9cbaa7a40e2b2723cfc2f266c42f4980038a949, WebMediaPlayer blindly treats a resource experiencing cross-origin redirects as TAINTED. In fact, it should be treated as TAINTED only when its request mode is "no-cors". The added tests are provided by hongchan@chromium.org. TBR=yhirano@chromium.org (cherry picked from commit 657a055a66492e996b296d7bb999e3c8551eed23) Bug: 899745 , 901383 Change-Id: Idb66407552085b053818f3e4a9d8d5ff3ddeaf45 Reviewed-on: https://chromium-review.googlesource.com/c/1325281 Reviewed-by: Hongchan Choi <hongchan@chromium.org> Reviewed-by: Fredrik Hubinette <hubbe@chromium.org> Commit-Queue: Yutaka Hirano <yhirano@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#606681} Reviewed-on: https://chromium-review.googlesource.com/c/1329801 Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#613} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 9
Issue 901629 has been merged into this issue.
,
Nov 12
Can confirm it's now working again. Thanks for the fast turn around! |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by hongchan@chromium.org
, Nov 2