New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 901383 link

Starred by 8 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 9
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-11-09
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

MediaElementAudioSource CORS access restriction - incorrect behavior

Project Member Reported by hongchan@chromium.org, Nov 2

Issue description

Go 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"



 
Cc: yhirano@chromium.org
https://cs.chromium.org/chromium/src/third_party/blink/renderer/modules/webaudio/media_element_audio_source_node.cc?sq=package:chromium&g=0&l=90

This is the only place where PrintCORSMessage() gets called, so WouldTaintOrigin() is giving 'true' for the resource from "rawgit.com".

yhirano@ I am not familiar with CORS check. Could you take a look at what we're doing is correct?
Cc: -yhirano@chromium.org hongchan@chromium.org
Labels: -Pri-2 Pri-1
Owner: yhirano@chromium.org
https://chromium-review.googlesource.com/c/chromium/src/+/1238098
Cc: hubbe@chromium.org yhirano@chromium.org
Labels: -Type-Bug Type-Bug-Regression
Owner: hongchan@chromium.org
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?


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
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?
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.
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...)
yhirano@ I uploaded a test CL so you can take a look.
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.
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.
Status: Started (was: Assigned)
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.
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.
Owner: yhirano@chromium.org
OK, https://chromium-review.googlesource.com/c/chromium/src/+/1320851/3 is not failing. I will make it pass.
s/not/now/

Sorry!
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!
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.
Labels: ReleaseBlock-Stable M-71
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.
Cc: gov...@chromium.org
Cc: swarnasree.mukkala@chromium.org rtoy@chromium.org hoch@chromium.org
 Issue 899745  has been merged into this issue.
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.
Labels: OS-Android OS-Chrome OS-Linux OS-Windows
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
Cc: -hoch@chromium.org
<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
Project Member

Comment 25 by bugdroid1@chromium.org, 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

Labels: Merge-Request-71
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.
Project Member

Comment 27 by sheriffbot@chromium.org, Nov 9

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
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
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.
NextAction: 2018-11-09
Pls update bug with canary result tomorrow. 
The NextAction date has arrived: 2018-11-09
Status: Verified (was: Started)
govind@

Just confirmed the fix on 72.0.3606.0. It plays the audio correctly now.
Labels: -Merge-Review-71 Merge-Approved-71
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.
Project Member

Comment 33 by bugdroid1@chromium.org, Nov 9

Labels: -merge-approved-71 merge-merged-3578
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

Labels: Merge-Merged-71-3578
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}
Cc: kainino@chromium.org jiajia....@intel.com kbr@chromium.org vamshi.kommuri@chromium.org japhet@chromium.org
 Issue 901629  has been merged into this issue.
Can confirm it's now working again. Thanks for the fast turn around!

Sign in to add a comment