New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 914261

Blocking:
issue 553535
issue 678905



Sign in to add a comment
link

Issue 911974: CSS should use the response URL as the base URL

Reported by falken@chromium.org, Dec 5 Project Member

Issue description

With service workers, the response URL might not be the same as the request URL. The response URL should be used.

chromestatus: https://www.chromestatus.com/feature/5642183499579392
 

Comment 1 by falken@chromium.org, Dec 5

Summary: CSS should use the response URL as the base URL (was: CSS should use the response URl as the base URL)

Comment 2 by bugdroid1@chromium.org, Dec 6

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5dc21f8d3cf6b02771aa45895b245c59e061f636

commit 5dc21f8d3cf6b02771aa45895b245c59e061f636
Author: Matt Falkenhagen <falken@chromium.org>
Date: Thu Dec 06 05:40:06 2018

WPT: service worker: fix test for CSS base URL.

The existing test was asserting the opposite of the standard,
which is to use the response URL. See
https://github.com/whatwg/fetch/pull/146.

This CL does the following:
- Tests that a CSS stylesheet fetched via respondWith(fetch(responseUrl)
  uses responseUrl as its base URL.
- Tests that a CSS stylesheet fetched via respondWith(new Response())
  uses the response URL (which is the request URL) as its base URL.
- Changes the test to not test cross-origin stylesheets. That is more
  complex than needed for this test, and there is talk of making
  subresource requests from opaque stylesheets skip the service worker,
  which would render the test ineffective for testing base URL.
- Changes the test to use waitUntil() in the message event to try
  to ensure the service worker stays alive between the message and
  fetch events.

Bug:  911974 
Change-Id: I167dfe86986ec718a50d512f862f1eb49889608b
Reviewed-on: https://chromium-review.googlesource.com/c/1362776
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614277}
[add] https://crrev.com/5dc21f8d3cf6b02771aa45895b245c59e061f636/third_party/blink/web_tests/external/wpt/service-workers/service-worker/fetch-request-css-base-url.https-expected.txt
[modify] https://crrev.com/5dc21f8d3cf6b02771aa45895b245c59e061f636/third_party/blink/web_tests/external/wpt/service-workers/service-worker/fetch-request-css-base-url.https.html
[modify] https://crrev.com/5dc21f8d3cf6b02771aa45895b245c59e061f636/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/fetch-request-css-base-url-iframe.html
[modify] https://crrev.com/5dc21f8d3cf6b02771aa45895b245c59e061f636/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/fetch-request-css-base-url-worker.js

Comment 3 by falken@chromium.org, Dec 7

Blocking: 678905

Comment 4 by falken@chromium.org, Dec 7

Description: Show this description

Comment 5 by bugdroid1@chromium.org, Dec 7

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

commit a7e0aee027dd136299552e5aa24d73aa29904f63
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Dec 07 22:50:33 2018

Clean up cross-origin check in StyleSheetContents.

The code was using a combination of the request URL and response URL.
It can just use ResourceResponse::IsCorsSameOrigin().

This is covered by the existing WPT test:
service-workers/service-worker/fetch-request-css-cross-origin.https.html

Bug:  911974 
Change-Id: I3a136fe8a0f99507610ccc8ae49eab7828a96f4f
Reviewed-on: https://chromium-review.googlesource.com/c/1366461
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614853}
[modify] https://crrev.com/a7e0aee027dd136299552e5aa24d73aa29904f63/third_party/blink/renderer/core/css/style_sheet_contents.cc

Comment 6 by bugdroid1@chromium.org, Dec 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/02d7d52f7396cb7a5e17c83e1da77fb4877047f7

commit 02d7d52f7396cb7a5e17c83e1da77fb4877047f7
Author: Matt Falkenhagen <falken@chromium.org>
Date: Tue Dec 11 10:14:41 2018

WPT: CSS: Add cross-origin redirect tests.

This adds tests that stylesheets that result from requests that were
redirected cross-origin are considered cross-origin.

Note that A->B->A redirects, which redirect from cross-origin to
same-origin, are considered cross-origin. See
https://github.com/whatwg/fetch/issues/737 and
https://github.com/whatwg/fetch/pull/834.

In Blink, we have redirect tests at
http/tests/security/cannot-read-cssrules-redirect.html. This WPT
addition will supersede that test, but I won't yet remove it since
it asserts the opposite for the A->B->A case. I can remove the test
when Blink changes to pass this WPT test.

Bug:  911974 
Change-Id: Ie015c0390829299de7c29cff6685ddfcd774c66f
Reviewed-on: https://chromium-review.googlesource.com/c/1370162
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615475}
[add] https://crrev.com/02d7d52f7396cb7a5e17c83e1da77fb4877047f7/third_party/blink/web_tests/external/wpt/css/cssom/stylesheet-same-origin.sub-expected.txt
[modify] https://crrev.com/02d7d52f7396cb7a5e17c83e1da77fb4877047f7/third_party/blink/web_tests/external/wpt/css/cssom/stylesheet-same-origin.sub.html

Comment 7 by bugdroid1@chromium.org, Dec 11

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

commit f2128a1506a602ec711696169ade34aa9f771997
Author: Matt Falkenhagen <falken@chromium.org>
Date: Tue Dec 11 10:25:14 2018

WPT: CSS: Add same-origin tests for loading error stylesheets.

This adds a test that a stylesheet that failed to load is considered
cross-origin. That is, accessing |styleSheet.rules| throws a
SecurityError.  This aligns with the specifications:
- cssRules checks the `origin-clean` flag:
  https://drafts.csswg.org/cssom/#dom-cssstylesheet-cssrules
- This is set to true iff CORS-same-origin:
  https://html.spec.whatwg.org/multipage/links.html#link-type-stylesheet
- CORS-same-origin is false for "error" responses:
  https://html.spec.whatwg.org/multipage/urls-and-fetching.html#cors-same-origin

Bug:  911974 
Change-Id: I558e6d3dfa564b15284c393ffc35b80f54a9bc4e
Reviewed-on: https://chromium-review.googlesource.com/c/1371307
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615479}
[modify] https://crrev.com/f2128a1506a602ec711696169ade34aa9f771997/third_party/blink/web_tests/external/wpt/css/cssom/stylesheet-same-origin.sub-expected.txt
[modify] https://crrev.com/f2128a1506a602ec711696169ade34aa9f771997/third_party/blink/web_tests/external/wpt/css/cssom/stylesheet-same-origin.sub.html
[add] https://crrev.com/f2128a1506a602ec711696169ade34aa9f771997/third_party/blink/web_tests/external/wpt/css/cssom/support/malformed-http-response.asis

Comment 8 by falken@chromium.org, Dec 11

Blockedon: 914135

Comment 9 by falken@chromium.org, Dec 12

Blockedon: -914135

Comment 10 by falken@chromium.org, Dec 13

Blockedon: 914261

Comment 11 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b30f526e1c760773511f48faa80c0347e49234c

commit 8b30f526e1c760773511f48faa80c0347e49234c
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Dec 14 04:00:31 2018

CSS: Use the response URL for base URL and type for security decisions.

Use ResourceResponse::ResponseUrl() to set the base URL, and use
ResponseResponse::GetType() to determine whether the resonse is
CORS-same-origin.

This CL has three web-exposed changes.

1. Use the response URL rather than the last request URL as the base URL
   of the stylesheet. This aligns with the standard. See
   https://github.com/whatwg/fetch/pull/146 and WPT results indicate
   Firefox, Edge, and Safari use the response URL. This only matters if
   the response came from a service worker, as the URLs only differ
   when the service worker intercepts the request and responds with a
   different URL via respondWith(fetch(other_url)).

   This is covered by the WPT:
   service-workers/service-worker/fetch-request-css-base-url.https.html

   The test doesn't completely pass yet because the search query part of
   the URL gets chopped off for FetchEvent.request.referrer, but the base
   URL is correct.

   Chrome Status: https://www.chromestatus.com/feature/5642183499579392

2. Consider A->B->A redirects to be cross-origin rather than
   same-origin. Previously, this was considered same-origin. See the
   discussion in https://github.com/whatwg/fetch/issues/737 and change
   https://github.com/whatwg/fetch/pull/834.

   This change makes the following WPT test pass:
   css/cssom/stylesheet-same-origin.sub.html

   It also affects the web test:
   http/tests/security/cannot-read-cssrules-redirect.html

   This test is updated to match the behavior change. It can be removed
   later since it is redundant with the WPT test.

3. Consider load failures to be cross-origin rather than same-origin.
   That is, accessing |styleSheet.rules| throws a SecurityError if the
   load failed.  This aligns with the specification:
   - cssRules checks the `origin-clean` flag:
     https://drafts.csswg.org/cssom/#dom-cssstylesheet-cssrules
   - This is set to true iff CORS-same-origin:
     https://html.spec.whatwg.org/multipage/links.html#link-type-stylesheet
   - CORS-same-origin is false on kError:
     https://html.spec.whatwg.org/multipage/urls-and-fetching.html#cors-same-origin

   This change makes the following WPT test pass:
   css/cssom/stylesheet-same-origin.sub.html

   It also affects the web tests:
   register-bypassing-scheme-partial.html
   require-sri-for-style-blocked.php

   These tests are updated to match the behavior change.

Chrome Status: https://www.chromestatus.com/feature/5642183499579392
Intent to Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/7OSy00oxVpk/siufiQVBBwAJ

Bug:  911974 
Change-Id: I9add3162596963eee66f60f339cfd9911bc151cd
Reviewed-on: https://chromium-review.googlesource.com/c/1367331
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616580}
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/css/css_style_sheet.cc
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/css/css_style_sheet.h
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/css/parser/css_parser_context.cc
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/css/parser/css_parser_context.h
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/css/selector_query.cc
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/css/selector_query_test.cc
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/css/style_rule_import.cc
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/css/style_sheet_contents.h
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/dom/processing_instruction.cc
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/renderer/core/html/link_style.cc
[delete] https://crrev.com/9f9392fa73c44640e4d1270d8d004f34a84badbf/third_party/blink/web_tests/external/wpt/css/cssom/stylesheet-same-origin.sub-expected.txt
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/web_tests/external/wpt/service-workers/service-worker/fetch-request-css-base-url.https-expected.txt
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/web_tests/http/tests/security/cannot-read-cssrules-redirect.html
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/register-bypassing-scheme-partial.html
[modify] https://crrev.com/8b30f526e1c760773511f48faa80c0347e49234c/third_party/blink/web_tests/http/tests/security/contentSecurityPolicy/require-sri-for/require-sri-for-style-blocked.php

Comment 12 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62e18616965872c50e58bfd6b28cc4aebfa035db

commit 62e18616965872c50e58bfd6b28cc4aebfa035db
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Dec 14 04:56:45 2018

CSS: Cleanup unused code and redundant test.

* Remove allow_rule_access_from_origin_ which is no longer used after
  https://chromium-review.googlesource.com/c/chromium/src/+/1367331.
* Remove http/tests/security/cannot-read-cssrules-redirect.html which is
  redundant with the WPT test
  css/cssom/stylesheet-same-origin.sub.html, which is passing after
  https://chromium-review.googlesource.com/c/chromium/src/+/1367331.

Bug:  911974 
Change-Id: I5047589f84ab371680390433f2c3d1c430d6cf6b
Reviewed-on: https://chromium-review.googlesource.com/c/1372989
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616586}
[modify] https://crrev.com/62e18616965872c50e58bfd6b28cc4aebfa035db/third_party/blink/renderer/core/css/css_style_sheet.cc
[modify] https://crrev.com/62e18616965872c50e58bfd6b28cc4aebfa035db/third_party/blink/renderer/core/css/css_style_sheet.h
[modify] https://crrev.com/62e18616965872c50e58bfd6b28cc4aebfa035db/third_party/blink/renderer/core/html/link_style.cc
[modify] https://crrev.com/62e18616965872c50e58bfd6b28cc4aebfa035db/third_party/blink/renderer/core/html/link_style.h
[delete] https://crrev.com/dac7ea529bceadbc8a97679abba0995999250194/third_party/blink/web_tests/http/tests/security/cannot-read-cssrules-redirect.html

Comment 13 by falken@chromium.org, Dec 14

Labels: M-73
Status: Fixed (was: Started)

Sign in to add a comment