WIP CL @ https://crrev.com/c/1042820 (using the suggestion from creis@ to stop blocking responses if a HTML comment within the response body also contains Javascript-styke comments).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c73ae935d43b69827a0dd34055ffc24538bf2a29

commit c73ae935d43b69827a0dd34055ffc24538bf2a29
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri May 04 01:30:27 2018

Handle some html/js polyglots in CORB confirmation sniffing.

Cross-Origin Read Blocking (CORB) tries to protect certain resource
types (e.g. text/html).  To be resilient against HTTP responses
mislabeled with an incorrect Content-Type, CORB sniffs the response body
to confirm if it truly is the protected type.

Before this CL the confirmation sniffing logic decided to block
resources that are both a valid html and a valid javascript.
Blocking of such resources is undesirable, because it is disruptive to
existing websites that use such polyglot responses in <script> tags.

After this CL, a HTML comment that contains a Javascript comment will
cause the confirmation sniffing to decide that the response is not
really a HTML document (this will prevent CORB blocking).

Bug:  839425 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ie790a81c2742513aed9fda45edd0bb2976bd0fc6
Reviewed-on: https://chromium-review.googlesource.com/1042820
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555951}
[modify] https://crrev.com/c73ae935d43b69827a0dd34055ffc24538bf2a29/content/browser/loader/cross_site_document_blocking_browsertest.cc
[add] https://crrev.com/c73ae935d43b69827a0dd34055ffc24538bf2a29/content/test/data/site_isolation/js-html-polyglot.html
[modify] https://crrev.com/c73ae935d43b69827a0dd34055ffc24538bf2a29/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/c73ae935d43b69827a0dd34055ffc24538bf2a29/services/network/cross_origin_read_blocking_explainer.md
[modify] https://crrev.com/c73ae935d43b69827a0dd34055ffc24538bf2a29/services/network/cross_origin_read_blocking_unittest.cc
[add] https://crrev.com/c73ae935d43b69827a0dd34055ffc24538bf2a29/third_party/WebKit/LayoutTests/external/wpt/fetch/corb/resources/html-js-polyglot.js
[add] https://crrev.com/c73ae935d43b69827a0dd34055ffc24538bf2a29/third_party/WebKit/LayoutTests/external/wpt/fetch/corb/resources/html-js-polyglot.js.headers
[add] https://crrev.com/c73ae935d43b69827a0dd34055ffc24538bf2a29/third_party/WebKit/LayoutTests/external/wpt/fetch/corb/script-html-js-polyglot.sub.html

Commit c73ae935... initially landed in 68.0.3419.0.  I've looked at crashes in this version and nothing worrisome caught my attention.

This fix is important to merge back into M67, as otherwise Site Isolation can disrupt some existing websites.

The fix is of medium complexity, but in M67 it should only affect Site Isolation (CORB has been turned on by default in M68, but is still gated behind Site Isolation in M67) and only affect CORB/HTML sniffing.

This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Hmmm... for some reason my Windows canary won't upgrade above: Version 68.0.3418.3 (Official Build) canary-dcheck (32-bit)... This makes it difficult to verify the fix on the original website that exhibited the problem.

I can confirm the fix in Mac Canary 68.0.3419.0.  CORB no longer blocks http://ads.pubmatic.com/AdServer/js/showad.js?, which can be interpreted as either HTML or JS.  That means it's working as intended.

However, there appear to be other polyglots in use on that page, with one example below.  Lukasz will file a bug to investigate that one.

I still think this is worth merging to M67.  It fixes a known case where CORB is blocking a script response, which could be problematic for Site Isolation users on M67 (and all users on M68).  We may follow up with a fix for the other polyglot case.

govind@: Does it make sense to merge this now and continue with investigating the second fix, or wait and merge both at once?  I'm inclined to get extra bake time for this first fix and go ahead with it.


Second polyglot:
https://clarium.global.ssl.fastly.net/?wrapper=bbdvOAJnqH-Idffgn_02C2Cyx_E&tpid=YmJkdk9BSm5xSC1JZGZmZ25fMDJDMkN5eF9FL2Fkc3BhcmM6MzAweDYwMA%3D%3D&d=eyJ3aCI6IlltSmtkazlCU201eFNDMUpaR1ptWjI1Zk1ESkRNa041ZUY5RkwyRmtjM0JoY21NNk16QXdlRFl3TUE9PSIsIndkIjp7ImsiOnsiaGJfYmlkZGVyIjpbImFkc3BhcmMiXSwiaGJfc2l6ZSI6WyIzMDB4NjAwIl19fSwid3IiOjB9

<!-- confiant --> <script type='text/javascript'>
//<![CDATA[
// (script code here)
//]]>--></script>

Approving merge for cl listed at #5 to M67 branch 3396 based on comments #12 and per offline chat with creis@. Please merge ASAP. Thank you.

Pls file a new bug and request a merge  to M67 when readyfor follow up with a fix for the other polyglot case. Thank you.

Thanks!  The followup fix will be tracked in  issue 839945 .

This almost merges cleanly, but not quite - I had to resolve some conflicts in services/network/cross_origin_read_blocking.cc.  I've built the merge on 3396 branch and run some CORB tests and everything looked good, so let me proceed with the merge.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5471caa18e89ed6a78535d88fd132bc5d0c0f741

commit 5471caa18e89ed6a78535d88fd132bc5d0c0f741
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri May 04 21:10:30 2018

Handle some html/js polyglots in CORB confirmation sniffing.

Cross-Origin Read Blocking (CORB) tries to protect certain resource
types (e.g. text/html).  To be resilient against HTTP responses
mislabeled with an incorrect Content-Type, CORB sniffs the response body
to confirm if it truly is the protected type.

Before this CL the confirmation sniffing logic decided to block
resources that are both a valid html and a valid javascript.
Blocking of such resources is undesirable, because it is disruptive to
existing websites that use such polyglot responses in <script> tags.

After this CL, a HTML comment that contains a Javascript comment will
cause the confirmation sniffing to decide that the response is not
really a HTML document (this will prevent CORB blocking).

TBR=lukasza@chromium.org

(cherry picked from commit c73ae935d43b69827a0dd34055ffc24538bf2a29)

Bug:  839425 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ie790a81c2742513aed9fda45edd0bb2976bd0fc6
Reviewed-on: https://chromium-review.googlesource.com/1042820
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#555951}
Reviewed-on: https://chromium-review.googlesource.com/1045135
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#489}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/5471caa18e89ed6a78535d88fd132bc5d0c0f741/content/browser/loader/cross_site_document_blocking_browsertest.cc
[add] https://crrev.com/5471caa18e89ed6a78535d88fd132bc5d0c0f741/content/test/data/site_isolation/js-html-polyglot.html
[modify] https://crrev.com/5471caa18e89ed6a78535d88fd132bc5d0c0f741/services/network/cross_origin_read_blocking.cc
[modify] https://crrev.com/5471caa18e89ed6a78535d88fd132bc5d0c0f741/services/network/cross_origin_read_blocking_explainer.md
[modify] https://crrev.com/5471caa18e89ed6a78535d88fd132bc5d0c0f741/services/network/cross_origin_read_blocking_unittest.cc
[add] https://crrev.com/5471caa18e89ed6a78535d88fd132bc5d0c0f741/third_party/WebKit/LayoutTests/external/wpt/fetch/corb/resources/html-js-polyglot.js
[add] https://crrev.com/5471caa18e89ed6a78535d88fd132bc5d0c0f741/third_party/WebKit/LayoutTests/external/wpt/fetch/corb/resources/html-js-polyglot.js.headers
[add] https://crrev.com/5471caa18e89ed6a78535d88fd132bc5d0c0f741/third_party/WebKit/LayoutTests/external/wpt/fetch/corb/script-html-js-polyglot.sub.html

Thanks!  Marking as fixed.