New issue
Advanced search Search tips

Issue 858975 link

Starred by 3 users
Status: Started
Owner:
shimazu@chromium.org
Cc:
falken@chromium.org
kinuko@chromium.org
bashi@chromium.org
dxie@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 715632



Sign in to add a comment

AppCache with NetworkService doesn't do mime sniffing

Project Member Reported by falken@chromium.org, Jun 29 2018 
Chrome Version: 69.0.3472.3 (Official Build) dev (64-bit)

What steps will reproduce the problem?
(1) Enable Network Service from chrome://flags or --enable-features=NetworkService.
(2) Go to https://crbug858975.glitch.me/

This is a basic site with an appcache manifest. You might need to first visit this site once, then reload.

What is the expected result?

Navigation.

What happens instead?

Download.


I'm guessing MIME sniffing doesn't apply to the responses from AppCache.

Not sure whether this blocks the Network Service Canary, leaving Untriaged.

Service worker has a similar issue:  issue 771118

Comment 1 by falken@chromium.org, Jun 29 2018

Description: Show this description

Comment 2 by falken@chromium.org, Jun 29 2018

Description: Show this description

Comment 3 by dxie@chromium.org, Jul 11

Labels: Proj-Servicification-Canary
Status: Available (was: Untriaged)

Comment 4 by jam@chromium.org, Jul 12

Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows

Comment 5 by jam@chromium.org, Jul 12

Labels: -Pri-3 Pri-1
Owner: kinuko@chromium.org
Status: Assigned (was: Available)
Kinuko: Daniel mentioned that someone from Tokyo is going to own AppCache now; if so can you assign this to them? Thanks

(bumping up priority since AppCache needs to work for canary trial)

Comment 6 by jam@chromium.org, Jul 12

Summary: AppCache with NetworkService doesn't do mime sniffing (was: Page is downloaded instead of navigated to when served from AppCache with NetworkService)
Also wonder if we need this with SW as well?

Comment 7 by falken@chromium.org, Jul 12 

FWIW, we decided not to block NetS13nServiceWorker Canary on the same SW issue ( issue 771118 ). We are proceeding and seeing if people complain. So far people haven't on Canary.

However I was a bit surprised how easily it was to hit this issue while testing, so I'm suspecting people will complain.

Comment 8 by kinuko@chromium.org, Jul 13

Cc: falken@chromium.org kinuko@chromium.org bashi@chromium.org
Owner: shimazu@chromium.org
Okay, let us take a look given that we have a similar issue we'll likely need to fix in SW. Assigning this to shimazu@ as he said he's interested in (and Matt is fine with it)-- thanks!

Comment 9 by jam@chromium.org, Jul 16 

Re comment 7, that's a great data point. I'm fine with removing canary-blocker on this as well for network service.

Comment 10 by jam@chromium.org, Jul 16

Cc: dxie@chromium.org
Labels: -Proj-Servicification-Canary Hotlist-KnownIssue

Comment 11 by shimazu@chromium.org, Jul 20

Status: Started (was: Assigned)
I'm now trying to apply mime sniffing by injecting URLLoader at ThrottlingURLLoader at NavigationURLLoaderImpl or ResourceDispatcher.
I'm expecting that  issue 771118  will be solved together in that approach. (Please correct me if I'm wrong. I'm not super familiar with the path.)
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 30 

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

commit 35be1bb5de7323013f3bc67e59f15be0d501dff0
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Mon Jul 30 03:11:29 2018

Add MimeSniffingThrottle for navigation and requests from renderer

This CL adds MimeSniffingThrottle which can intercept the response in
ThrottlingURLLoader. Typically mime sniffing happens in network::URLLoader and
MimeSniffingThrottle is skipped in this case. If a request goes to a service
worker or other interceptors, the MimeSniffngThrottle intercepts the response
when a set of the mime type and the url is eligible to sniff the mime
type. Sniffable mime types are defined in net::ShouldSniffMimeType(). When a
mime type for performance sensitive components like "text/html",
"text/javascript",or "text/css" is provided, it doesn't sniff.

Bug: 858975
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ib6e5b29867494fc0c0876952b4fec2a141288b36
Reviewed-on: https://chromium-review.googlesource.com/1141744
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578971}
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/browser/devtools/devtools_url_loader_interceptor.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/browser/file_url_loader_factory.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/browser/fileapi/file_system_url_loader_factory.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/browser/fileapi/file_system_url_loader_factory_browsertest.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/browser/loader/mime_sniffing_resource_handler.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/browser/loader/mime_sniffing_resource_handler_unittest.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/BUILD.gn
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/DEPS
[add] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/mime_sniffing_throttle.cc
[add] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/mime_sniffing_throttle.h
[add] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/mime_sniffing_throttle_unittest.cc
[add] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/mime_sniffing_url_loader.cc
[add] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/mime_sniffing_url_loader.h
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/service_worker/service_worker_loader_helpers.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/throttling_url_loader.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/common/throttling_url_loader.h
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/public/common/url_loader_throttle.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/public/common/url_loader_throttle.h
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/content/test/BUILD.gn
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/services/network/public/cpp/network_ipc_param_traits.h
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/services/network/public/cpp/resource_response.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/services/network/public/cpp/resource_response_info.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/services/network/public/cpp/resource_response_info.h
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/services/network/url_loader.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/services/network/url_loader_unittest.cc
[modify] https://crrev.com/35be1bb5de7323013f3bc67e59f15be0d501dff0/third_party/WebKit/LayoutTests/TestExpectations
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 31 

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

commit b0f5b21b078c6382136d80d11f477822e6395e28
Author: Peter Boström <pbos@chromium.org>
Date: Tue Jul 31 00:42:46 2018

Revert "Add MimeSniffingThrottle for navigation and requests from renderer"

This reverts commit 35be1bb5de7323013f3bc67e59f15be0d501dff0.

Reason for revert: Causing test flakes.

Bug:  chromium:869225 

Original change's description:
> Add MimeSniffingThrottle for navigation and requests from renderer
> 
> This CL adds MimeSniffingThrottle which can intercept the response in
> ThrottlingURLLoader. Typically mime sniffing happens in network::URLLoader and
> MimeSniffingThrottle is skipped in this case. If a request goes to a service
> worker or other interceptors, the MimeSniffngThrottle intercepts the response
> when a set of the mime type and the url is eligible to sniff the mime
> type. Sniffable mime types are defined in net::ShouldSniffMimeType(). When a
> mime type for performance sensitive components like "text/html",
> "text/javascript",or "text/css" is provided, it doesn't sniff.
> 
> Bug: 858975
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: Ib6e5b29867494fc0c0876952b4fec2a141288b36
> Reviewed-on: https://chromium-review.googlesource.com/1141744
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578971}

TBR=kinuko@chromium.org,shimazu@chromium.org

Change-Id: I88721b279e5f38685b96d2d6f2813e423ff49540
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 858975
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1155501
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579259}
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/browser/devtools/devtools_url_loader_interceptor.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/browser/file_url_loader_factory.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/browser/fileapi/file_system_url_loader_factory.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/browser/fileapi/file_system_url_loader_factory_browsertest.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/browser/loader/mime_sniffing_resource_handler.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/browser/loader/mime_sniffing_resource_handler_unittest.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/common/BUILD.gn
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/common/DEPS
[delete] https://crrev.com/d35bcee05bb7136bd7a69d8e89d6fa2181185283/content/common/mime_sniffing_throttle.cc
[delete] https://crrev.com/d35bcee05bb7136bd7a69d8e89d6fa2181185283/content/common/mime_sniffing_throttle.h
[delete] https://crrev.com/d35bcee05bb7136bd7a69d8e89d6fa2181185283/content/common/mime_sniffing_throttle_unittest.cc
[delete] https://crrev.com/d35bcee05bb7136bd7a69d8e89d6fa2181185283/content/common/mime_sniffing_url_loader.cc
[delete] https://crrev.com/d35bcee05bb7136bd7a69d8e89d6fa2181185283/content/common/mime_sniffing_url_loader.h
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/common/service_worker/service_worker_loader_helpers.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/common/throttling_url_loader.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/common/throttling_url_loader.h
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/public/common/url_loader_throttle.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/public/common/url_loader_throttle.h
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/content/test/BUILD.gn
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/services/network/public/cpp/network_ipc_param_traits.h
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/services/network/public/cpp/resource_response.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/services/network/public/cpp/resource_response_info.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/services/network/public/cpp/resource_response_info.h
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/services/network/url_loader.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/services/network/url_loader_unittest.cc
[modify] https://crrev.com/b0f5b21b078c6382136d80d11f477822e6395e28/third_party/WebKit/LayoutTests/TestExpectations
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 2 

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

commit 21950e8178c9398c40a6055a683adcea54f77660
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Thu Aug 02 12:37:14 2018

Reland "Add MimeSniffingThrottle for navigation and requests from renderer"

This reverts commit b0f5b21b078c6382136d80d11f477822e6395e28.

The difference from the original patch is a kAborted state in
MimeSniffingURLLoader. DCHECKs will capture if something wrong. Also, Abort()
stops to read the body and receive messages from the source URLLoader. We can
ensure that the loader is stopped to work immediately in this way.

Original description:

> This CL adds MimeSniffingThrottle which can intercept the response in
> ThrottlingURLLoader. Typically mime sniffing happens in network::URLLoader and
> MimeSniffingThrottle is skipped in this case. If a request goes to a service
> worker or other interceptors, the MimeSniffngThrottle intercepts the response
> when a set of the mime type and the url is eligible to sniff the mime
> type. Sniffable mime types are defined in net::ShouldSniffMimeType(). When a
> mime type for performance sensitive components like "text/html",
> "text/javascript",or "text/css" is provided, it doesn't sniff.

Bug: 858975
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I51648def32e147eb127ec6efbb59ed2353cfba56
Reviewed-on: https://chromium-review.googlesource.com/1160133
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580143}
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/browser/devtools/devtools_url_loader_interceptor.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/browser/file_url_loader_factory.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/browser/fileapi/file_system_url_loader_factory.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/browser/fileapi/file_system_url_loader_factory_browsertest.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/browser/loader/mime_sniffing_resource_handler.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/browser/loader/mime_sniffing_resource_handler_unittest.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/BUILD.gn
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/DEPS
[add] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/mime_sniffing_throttle.cc
[add] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/mime_sniffing_throttle.h
[add] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/mime_sniffing_throttle_unittest.cc
[add] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/mime_sniffing_url_loader.cc
[add] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/mime_sniffing_url_loader.h
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/service_worker/service_worker_loader_helpers.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/throttling_url_loader.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/common/throttling_url_loader.h
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/public/common/url_loader_throttle.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/public/common/url_loader_throttle.h
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/content/test/BUILD.gn
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/services/network/public/cpp/network_ipc_param_traits.h
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/services/network/public/cpp/resource_response.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/services/network/public/cpp/resource_response_info.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/services/network/public/cpp/resource_response_info.h
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/services/network/url_loader.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/services/network/url_loader_unittest.cc
[modify] https://crrev.com/21950e8178c9398c40a6055a683adcea54f77660/third_party/WebKit/LayoutTests/TestExpectations

Comment 15 by shimazu@chromium.org, Aug 6

Labels: -Pri-1 Pri-2
I could manually confirm https://crbug858975.glitch.me/ works well.
We still need to add a wpt test for it, but let me de-prioritize this since the bug has been solved.
►

Sign in to add a comment