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

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug


Sign in to add a comment
link

Issue 797712: SWs and ES6 Modules don't work in extensions if HKEY_CLASSES_ROOT\js\Content Type registry is set

Reported by hirosh...@chromium.org, Dec 27 2017 Project Member

Issue description

If HKEY_CLASSES_ROOT\js\Content Type registry is set (to a non-JavaScript mime type such as text/plain), then the MIME type for .js files in extensions is set to the registry value, and thus the scripts are blocked as service worker scripts and module scripts.

The root cause was originally found in:
https://bugs.chromium.org/p/chromium/issues/detail?id=762483#c18

More recent discussion is:
https://bugs.chromium.org/p/chromium/issues/detail?id=728377#c24 -- 30
and suggested solution is to make .js MIME type in extensions not to be affected by the local registry configuration.

Probably, the problems reported in  Issue 769012  and  Issue 763673  after they are closed as fixed are also due to this issue.

I filed this issue separately in order to isolate root causes for module script failures in extensions (if there're still many).

Steps to reproduce:
1. Open regedit.exe and navigate to HKEY_CLASSES_ROOT\js.
2. Add "Content Type" string value and set it to something non-Javascript (e.g. "foo/bar").
3. Load the attached unpacked extension (this is the same as https://codesearch.chromium.org/chromium/src/chrome/test/data/extensions/api_test/modules/).
4. Observe the error via chrome://extensions/.
5. Remove the "Content Type" key added in Step 2.
6. Reload the extension and observe the error again via chrome://extensions/.

Expected behavior:
Console log "PASS" is shown for Steps 4 and 6.

Actual behavior:
Failed to load module script: The server responded with a non-JavaScript MIME type of "foo/bar". Strict MIME type checking is enforced for module scripts per HTML spec.
is shown for Step 4.
Console log "PASS" is shown for Step 6 (i.e. module script works if the registry key doesn't exist, and thus  Issue 769012  was closed as fixed).
 
testModuleScriptsInExtension.zip
1.2 KB Download

Comment 1 by hirosh...@chromium.org, Dec 27 2017

Cc: hirosh...@chromium.org asanka@chromium.org rdevlin....@chromium.org mkwst@chromium.org rsleevi@chromium.org

Comment 2 by rsleevi@chromium.org, Dec 27 2017

Labels: -Pri-1 Pri-2
Interesting! It most likely means we're taking a sniffing pass, which shouldn't be happening.

Even without a Step 1, you should be able to trace by looking at net::GetMimeTypeFromExtension(), which dispatches to net::PlatformMimeUtil::GetPlatformMimeTypeFromExtension if the extension isn't in kPrimaryMappings (and it isn't - we only have the media types there). I'd also add a check to net::PlatformMimeUtil's function as well, just to make sure it's comprehensively covering the interactions.

Neither of those should be called, though, if the mime type is set correctly. Finding out where this guessing is coming from, I suspect, is key to tracing it down.

I don't think the correct solution is to add the JS mimetype to kPrimaryMappings - it's already in kSecondaryMappings, which is consistently deprioritized compared to the platform list. This also matches what other browsers do - https://cs.chromium.org/chromium/src/net/base/mime_util.cc?rcl=ed6f5c83b64a0e7792de626e64e99239d34f4ca2&l=241 - specifically, mirroring Firefox behaviour here.

Comment 3 by elawrence@chromium.org, Dec 27 2017

Re #2: The reason the MIME type is missing "isn't set correctly" is that the MIME type isn't provided by the chrome-extension:// protocol except via the new code added for https://bugs.chromium.org/p/chromium/issues/detail?id=769012#c1 which uses the GetMimeTypeFromExtension function.

I'm not in any way an owner of the MIME determination code, but given the level of breakage caused by a system misconfiguration, moving .js to kPrimaryMappings seems very reasonable to me.

Comment 4 by rsleevi@chromium.org, Dec 27 2017

As noted in https://cs.chromium.org/chromium/src/net/base/mime_util.cc?rcl=ed6f5c83b64a0e7792de626e64e99239d34f4ca2&l=241 , the design choice to excluding it from kPrimaryMappings is to allow the OS to override this type. To that end, it's WAI - but that doesn't mean we can't change.

To that end, I'm wondering whether https://chromium.googlesource.com/chromium/src/+/6449de840aec46116c234a5865c4ba120a0e38d9%5E%21/#F6 should instead be bypassing GetMimeType() for resources it knows the mimetypes of, relative to extensions. Is there a reason we may not want to do that?

Comment 5 by elawrence@chromium.org, Dec 27 2017

Re #4: If we put the special case in for the chrome-extension:// protocol, we'd presumably also want to put in the same special case for the file protocol (to ensure web developers aren't broken), and probably anything else that uses GetMimeTypeFromExtension. 

It's hard to imagine a scenario whereby we'd really want a critical web content type to be overridden by a user's system configuration.

Comment 6 by elawrence@chromium.org, Dec 27 2017

With regard to Firefox behavior, is our comment current? It appears that https://dxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#2675 checks a baked-in primary list before looking to the registry, and JavaScript is on that list.

Comment 7 by rsleevi@chromium.org, Dec 27 2017

I disagree that it would be appropriate to place in the file:// protocol. Could you explain why you believe the file:// protocol should not respect what the mime type the user has configured their system to report such files as?

I really believe it's specific to chrome-extension://, and I have trouble seeing both the slippery slope and how it extends. Given that .js files can have other mime types attached (e.g. wscript), it does not at all seem unreasonable or incorrect to keep the current behaviour.

Comment 8 by elawrence@chromium.org, Dec 27 2017

I contend that ~no users expect the behavior of a page loaded from the file protocol via 

   <script src="file.js" /> 

...should vary based on any local configuration settings. That's especially true because in all of the cases I've seen in the last thirteen years of working on browsers, all such configuration changes were performed either without the user's knowledge (e.g. by installers of other software) or in an attempt to achieve some other goal (e.g. display directly-navigated files as plaintext instead of having them download).

Users' expectations are, of course, not readily measurable, and given the fact that we don't especially /like/ the use of file://, allowing its behavior to remain unpredictable/unreliable wouldn't be the end of the world.

Comment 9 by rsleevi@chromium.org, Dec 27 2017

Firefox changed the behaviour 9 months ago ( https://bugzilla.mozilla.org/show_bug.cgi?id=1352684 ). This was a preemptive optimization based on perf concerns around Firefox Fennec (Android) lookups, as expanded in https://bugzilla.mozilla.org/show_bug.cgi?id=632490

They originally went the route proposed in Comment #4, but wanted to optimize further, as explained in https://bugzilla.mozilla.org/show_bug.cgi?id=1352684#c0

That is - this was not about correctness, it was an optimization. To be explicit my concern - I'm concerned that attempting to deduce the mime type (GetMimeTypeFromExtension) is a form of mime sniffing, and we should be exceptionally judicious with those calls - ideally, avoiding all-together. I'm particularly concerned with it being used for loading, rather than saving or uploading, and the care needed to reason about those implications.

Comment 10 by elawrence@chromium.org, Dec 27 2017

>I'm concerned that attempting to deduce the mime type (GetMimeTypeFromExtension) is a form of mime sniffing

I contend that it's the least dangerous form of sniffing, and the same one used by every major web server.

>we should be exceptionally judicious with those calls

If the proposal were anything other than "Ensure that GetMimeTypeFromExtension('.js') returns 'application/javascript' 100% of the time instead of just >99.99% of the time" then I'd agree that extreme care is warranted.

Comment 11 by kouhei@chromium.org, Dec 28 2017

Owner: rsleevi@chromium.org
rsleevi, elawrence: Thanks for looking into this. Would you mind owning this bug?

Comment 12 by rsleevi@chromium.org, Dec 28 2017

Owner: rdevlin....@chromium.org
Assigning to rdevlin.cronin instead for their take. I'm not sure I agree with Comment #10's conclusions - that is, I'm concretely concerned about unconditionally treating the 'js' extension as application/javascript being a good thing, in general. That Firefox recently changed to unconditionally treat it as application/javascript doesn't particularly inspire me with confidence, given both the difference in coding patterns and the rationale.

I'd curious for Mike's take as well, based on work like  Issue 419383  or  Issue 788936 . My gut instinct is that I think it's reasonable for extensions (when loaded from extensions) to hardcode the mimetype, I'm nervous about it for file:// URLs, and I'm not sure I understand enough about the SW case (unless it's just "in the context of extensions", in which case, it should be addressed by the above extension loading case). I'm also curious if there's progress on making file:// URLs distinct origins, and what impact that would have (e.g. service workers would no longer matter, would they, since they'd be distinct origins and thus not match the page if loaded over file://?)

Comment 13 by rdevlin....@chromium.org, Jan 12 2018

Naively, I agree with elawrence@ that really, there shouldn't be expectations around js files being anything but JavaScript, and that if it's set to something else, it's much more likely a bug than a feature.  That said, I have approximately no experience at the net layer, and am well out of my depth. :)

I think that at a minimum, we should expect this for extensions.  To me (again naively), it seems preferable to do this globally, rather than have a one-off assumption in extensions code that overrides the mime type retrieved from the net layer.  But if there are *legitimate* cases for that (and we think that they are common enough that they are worth preserving), I'm fine with the extension-specific solution.

Is there any data we could gather that would help inform this decision?  I can't think of any, but, again, net/ layer is out of my jurisdiction. :)

Comment 14 by asanka@chromium.org, Jan 17 2018

On Content-Type determination in file:// vs extension://
-----------------------------------------------------

I agree with #12's argument that file:// and extension:// resolvers shouldn't use the same logic for determining content type.

Given what file:// URLs are used for, it makes sense for these URL fetches to infer the content type of a resource in the same manner in which the underlying platform infers it. I.e. use the platform MIME map whenever one is available.

extension:// URLs, on the other hand, shouldn't use the platform MIME map at all. The local configuration was not a party to how resources packaged with an extension were named. Hence the local configuration shouldn't be a party to how those resources are interpreted.

The current behavior WRT extension:// URLs is a side-effect of URLRequestExtensionJob inheriting from net::URLRequestFileJob. While extension:// URL fetches follow the same logic as file:// for fetching the underlying resource, it shouldn't IMHO inherit the content type determination logic. The change isn't very complicated either[2]. FTR, file:// uses net::GetMimeTypeFromFile() whereas extension:// can switch to using net::GetWellKnownMimeTypeFromExtension() contingent on the well-known mappings knowing about .js.

It's possible that some extensions rely on the current behavior. E.g. to sniff out which applications are installed locally based on telltale file associations. These may break if extension:// switches to well-known mappings only.

On adding '.js' to primary MT mappings vs. secondary mappings
----------------------------------------------------------

There hasn't been a consistent philosophy around the filetype <-> content type where net's hardcoded settings differ from platform settings. Most[1] of the primary mappings are consistent with what was needed to isolate file:// URL loads from local configuration in an era before ES6 modules and SWs.

Adding .js to the primary mappings is, I believe, consistent with how our MIME mapping logic has worked historically. .js is just as critical as .html and .css for basic web functionality and the file type has sufficiently ossified that a different interpretation is likely not intentional as #8 and #13 points out. That //net unfortunately bears the burden of preserving web platform compatibility for file:// is a historical wart that should probably be addressed at some point.

How about this:

  * Restrict URLRequestExtensionJob to use the well-known extension set. I.e. use net::GetWellKnownMimeTypeFromExtension().

  * Add '.js' to primary mappings.

  * Add '.json' to the secondary mappings list as per  issue 799045 . The mapping for .json isn't quite as ossified as .js. Keeping it in the secondary mappings would isolate extensions from local configuration while allowing file:// to respect local system settings.


[1]: There are exceptions. E.g. the primary mapping for .mht/.mhtml was added to account for a disagreement in what Windows thought its content type was ("message/rfc822") and what RFC 2557 says it should be ("multipart/related"). IMHO this is the wrong layering to add such compat logic. But there it is.

[2]: There's also some refactoring that needs to happen to allow URLRequestExtensionJob to override the content type logic. Nothing is that easy.

Comment 15 by asanka@chromium.org, Jan 17 2018

Cc: viswatej...@techmahindra.com sc00335...@techmahindra.com vamshi.k...@techmahindra.com
 Issue 799045  has been merged into this issue.

Comment 16 by rsleevi@chromium.org, Jan 24 2018

The plan in comment #14 sounds good to me.

Comment 17 by bugdroid1@chromium.org, Jan 26 2018

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

commit aed0e5425bf2021fac85173daac10145f6a58a2a
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Jan 26 22:05:29 2018

[Extensions] Change Mime-Type deducation in URLRequestExtensionJob

Modify URLRequestExtensionJob to determine the mime type of a requested
file through net::GetWellKnownMimeTypeFromExtension(), rather than
URLRequestFileJob::GetMimeType(). The difference here is that the latter
will include platform-specific modifications. This means that if another
application on the users machine specified a different mime type for an
extension, that type would be used.

This causes problems with modules in extensions, in the cases where a
different application has set a different mime type for .js files, since
modules have strict mime type requirements.

Use GetWellKnownMimeTypeFromExtension(), and add .js to the list of
primary mappings in mime_util.cc.

Bug:  797712 

Change-Id: I27fea0479268520abd08fb62a501d213a3a04f52
Reviewed-on: https://chromium-review.googlesource.com/885401
Reviewed-by: Asanka Herath <asanka@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532070}
[modify] https://crrev.com/aed0e5425bf2021fac85173daac10145f6a58a2a/extensions/browser/extension_protocols.cc
[modify] https://crrev.com/aed0e5425bf2021fac85173daac10145f6a58a2a/net/base/mime_util.cc

Comment 18 by bugdroid1@chromium.org, Jan 30 2018

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

commit c3f88077f497376eb5565031decf89860435d97a
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue Jan 30 02:10:11 2018

[Net] Add JSON to kSecondaryMappings in mime_util

Add JSON as a known mime type in kSecondaryMappings in mime_util.
It's not quite common enough to put in kPrimaryMappings;
kSecondaryMappings will still allow local configurations to
apply on file:// urls.

Bug:  797712 
Change-Id: Ia4c8ab8930a2d14da51ad781094df27d9a21afb5
Reviewed-on: https://chromium-review.googlesource.com/889915
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532733}
[modify] https://crrev.com/c3f88077f497376eb5565031decf89860435d97a/chrome/browser/extensions/extension_protocols_unittest.cc
[modify] https://crrev.com/c3f88077f497376eb5565031decf89860435d97a/net/base/mime_util.cc
[modify] https://crrev.com/c3f88077f497376eb5565031decf89860435d97a/net/base/mime_util_unittest.cc

Comment 19 by rdevlin....@chromium.org, Feb 8 2018

Status: Fixed (was: Assigned)
This should be fixed.

Comment 20 by liuhao...@gmail.com, Mar 7 2018

I can reproduce on chrome 65.0.3325.146 (正式版本) (64 位) (cohort: 65_win_146).

Is chrome 65 include this patch?

Comment 21 by rdevlin....@chromium.org, Mar 7 2018

Nope, looks like the patch landed in M66:

https://chromium.googlesource.com/chromium/src/+/c3f88077f497376eb5565031decf89860435d97a/chrome/VERSION

It looks like Dev and Canary should each include this by now, if you wanted to test there.

Comment 22 by aki.maki...@gmail.com, Mar 21 2018

Is there any chance of .js files in file:// also receiving a default mime type at some point to make ES6 modules work?

I have found that ES6 modules also cause issues in cordova hybrid apps while developing/testing on android device.

Example of this same problem by someone else in Stackoverflow: https://stackoverflow.com/questions/48113359

Comment 23 by rdevlin....@chromium.org, Mar 21 2018

@22: I'll defer to asanka@ and rsleevi@.  I think it should be possible, but I'm not sure what else (if anything) it may break.  It may be worth filing a new bug to track that.

Comment 24 by elawrence@chromium.org, Mar 21 2018

Re #22/#23: We should definitely use a new issue to track that, but my understanding was that .js would default to the proper application/javascript MIME type (via the secondaryMappings) unless the operating system was configured to return a different type.

Comment 25 by aki.maki...@gmail.com, Mar 22 2018

While this problem (for me) is while running the code on android webview, a quick test shows that I also get it when trying to load on windows 7.

I created a new issue 824651 concerning this.

Sign in to add a comment