New issue
Advanced search Search tips

Issue 767606 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Nov 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 678905



Sign in to add a comment

Pass the web-platform-test: registration-mime-types.https.html

Project Member Reported by falken@chromium.org, Sep 21 2017

Issue description

Comment 1 by falken@chromium.org, Sep 21 2017

Cc: nhiroki@chromium.org
Owner: falken@chromium.org
Status: Started (was: Assigned)
Will just take this.
Thank you! :)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 22 2017

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

commit 0095b65a53519b27f83f75fc15625d64865341ac
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Sep 22 05:33:06 2017

service worker: Recognize all JavaScript MIME types during registration.

register(scriptURL) throws if the script response does not
have a JavaScript MIME type. But before this patch we only recognize
a few types. For spec conformance, recognize all. The service worker spec
points to HTML spec's definition of "JavaScript MIME type" which is
implemented by blink::IsJavascriptMIMEType():
https://html.spec.whatwg.org/multipage/scripting.html#javascript-mime-type

We still don't check MIME type for importScripts() which is left for future
work. I'm not sure the WPT test for that is testing what the spec is saying.

Bug:  767606 
Change-Id: I5a7033ecbcb938081fe501848652dbda254c8076
Reviewed-on: https://chromium-review.googlesource.com/677894
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503667}
[modify] https://crrev.com/0095b65a53519b27f83f75fc15625d64865341ac/content/browser/service_worker/service_worker_write_to_cache_job.cc
[modify] https://crrev.com/0095b65a53519b27f83f75fc15625d64865341ac/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-mime-types.https-expected.txt
[modify] https://crrev.com/0095b65a53519b27f83f75fc15625d64865341ac/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/registration-mime-types.https-expected.txt
[delete] https://crrev.com/03c7274a902e0e563927ac3246613aa5ca9c4eda/third_party/WebKit/LayoutTests/platform/linux/virtual/mojo-blobs/external/wpt/service-workers/service-worker/link-element-register-mime-types.https-expected.txt
[delete] https://crrev.com/03c7274a902e0e563927ac3246613aa5ca9c4eda/third_party/WebKit/LayoutTests/platform/linux/virtual/mojo-blobs/external/wpt/service-workers/service-worker/registration-mime-types.https-expected.txt
[delete] https://crrev.com/03c7274a902e0e563927ac3246613aa5ca9c4eda/third_party/WebKit/LayoutTests/platform/linux/virtual/service-worker-script-streaming/external/wpt/service-workers/service-worker/link-element-register-mime-types.https-expected.txt
[delete] https://crrev.com/03c7274a902e0e563927ac3246613aa5ca9c4eda/third_party/WebKit/LayoutTests/platform/linux/virtual/service-worker-script-streaming/external/wpt/service-workers/service-worker/registration-mime-types.https-expected.txt
[delete] https://crrev.com/03c7274a902e0e563927ac3246613aa5ca9c4eda/third_party/WebKit/LayoutTests/platform/mac/virtual/mojo-blobs/external/wpt/service-workers/service-worker/link-element-register-mime-types.https-expected.txt
[delete] https://crrev.com/03c7274a902e0e563927ac3246613aa5ca9c4eda/third_party/WebKit/LayoutTests/platform/mac/virtual/mojo-blobs/external/wpt/service-workers/service-worker/registration-mime-types.https-expected.txt
[delete] https://crrev.com/03c7274a902e0e563927ac3246613aa5ca9c4eda/third_party/WebKit/LayoutTests/platform/mac/virtual/service-worker-script-streaming/external/wpt/service-workers/service-worker/link-element-register-mime-types.https-expected.txt
[delete] https://crrev.com/03c7274a902e0e563927ac3246613aa5ca9c4eda/third_party/WebKit/LayoutTests/platform/mac/virtual/service-worker-script-streaming/external/wpt/service-workers/service-worker/registration-mime-types.https-expected.txt

Comment 4 by falken@chromium.org, Sep 29 2017

Remaining work is to apply the MIME check to importScripts.

Comment 5 by falken@chromium.org, May 14 2018

Owner: ----
Status: Available (was: Started)
Freeing this up.
Hi, I did some investigation to enable the mime check on importScripts() and think out a possible solution to make this case pass, but met with some difficulty:

This case expected the promise of ServiceWorkerContainer.register() to be rejected with SecurityError if imported script’s mime type check failed. It actually requires to notify more error details happened on service worker thread to the thread triggering the registration instead of just knowing success or failure.

We could do two things for this requirement:

1. Throw SecurityError upon failure of mime type check during importScripts() on the worker thread. Then after evaluation of the service worker script, extract the error code from exception. We could also support more other exceptions if needed.

2. Transfer the error code to browser process to notify ServiceWorkerRegisterJob to reject the register promise.

For the first thing, we could throw a SecurityError. We are not sure whether Javascript code will catch the error or not, so we must check it after the evaluation of the script, and if not caught by Javascript, we could extract its error code and process it further. 

If we don’t do it this way, we can store the error info somewhere manually and process it later. But that could be error-prone to manage the error state, and it’s not generic either. Throwing exception and check the exception seems more generic and could cover other types of errors like network errors too.

I found the difficulty is how to extract error code from exception: in WorkerOrWorkletScriptController::Evaluate(), I didn’t find a way to extract error code from the exception (a blink::ScriptValue). It maybe a DOMException, ES errors or even other types.

Do someone have some suggestion on this? Thanks a lot in advance.

Cc: falken@chromium.org shimazu@chromium.org
CCed falken@ and shimazu@, any insights about c#6? :)
In the browser process, currently there is a code to retrieve a failure message when registering a worker only for a main script.
https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_register_job.cc?type=cs&sq=package:chromium&g=0&l=449

I guess we can do something similar by attaching the MimeType error at SWNewScriptLoader, which loads scripts and checks the mime type like this:
https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_new_script_loader.cc?type=cs&g=0&l=218

When it's a main script, currently the URLLoaderCompletionStatus and its message are delivered there via ServiceWorkerScriptCacheMap::NotifyFinished(). I guess we can do it for the latest subresource errors.
https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_new_script_loader.cc?type=cs&g=0&l=568

Thanks for the suggestion, I made a try with SWNewScriptLoader and it worked! Checking it in browser process instead of in renderer process have no such difficulty I mentioned in c#6. thanks for help :). I think I can continue with the left work for this issue. shimazu@
This case passes after a new import of wpt tests in CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1310474

The test was updated due to not strictly complies with the spec, I checked it, it did pass now. So this can be closed. 

Then my change is not necessary, thanks for all.

Status: Fixed (was: Available)
Oh, thanks for working on this and sorry for my bad suggestion. I misread the spec.
Throwing SecurityError in the service worker context and TypeError in the context calling .register() seems correct.
Let's close this issue since the test currently passes.
Labels: M-71
For the record, Chrome 71 changed to do stricter MIME checking in importScripts().  I think in:
https://bugs.chromium.org/p/chromium/issues/detail?id=794548#c5

Then the WPT tests were updated at https://github.com/web-platform-tests/wpt/commit/44dd29fb0d51be4569ece8cad2d8e94aeb24f335 and we passed them.


Sign in to add a comment