Fix: Loading cid: resources in WebView.

third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All
     rights reserved.
     Copyright (C) 2009 Torch Mobile Inc. http://www.torchmobile.com/
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
@@ skipping 626 matching lines @@
   if (result == kBlock)
     return ResourceForBlockedRequest(params, factory, blocked_reason);
 
   if (!params.IsSpeculativePreload()) {
     // Only log if it's not for speculative preload.
     Context().RecordLoadingActivity(identifier, resource_request,
                                     factory.GetType(),
                                     params.Options().initiator_info.name);
   }
 
-  // An URL with the "cid" scheme can only be handled by an MHTML Archive.
-  // Abort the request when there is none.
-  if (!archive_ && resource_request.Url().ProtocolIs(kContentIdScheme))
+  // A main resource request with the "cid" scheme can only be handled by an
+  // MHTML Archive. Abort the request when there is none.
+  // Note: There are some embedders of WebView that are using Content-ID
+  // URLs for sub-resources, even without any MHTMLArchive. Please see

[Torne, 2017/07/14 13:55:47]

Just a note (don't expect you to change this CL for this, but maybe a followup):
while the specific broken cases we've seen in the wild are subresources, in
general WebView embedders can do anything they please with URLs and there's no
reason to assume it's not also used for the main resource somewhere maybe. How
important is it that this check exists at all?

The only URL usage we have actually forbidden in WebView is intercepting the
chrome:// scheme, because the security assumptions around this are too deeply
embedded in chromium to work around.

[arthursonzogni, 2017/07/17 10:58:15]

Thanks for this information.

Without this check, some weird pages caused the renderer to be killed.

What I will probably do in a follow up will be what I did in the first patchset
of https://codereview.chromium.org/2834013002/#ps140001
(i.e. returning true in ShouldMakeNetworkRequestForURL(GURL("cid:xxxx")) and
modify RenderFrameImpl::DecidePolicyForNavigation() appropriately).
It will result in the removal of the part that block Content-ID URLs in the
ResourceFetcher.
I didn't do that right now in order to have a very simple CL that is easy to
merge in M60.

+  // https://crbug.com/739658.
+  if (!archive_ && factory.GetType() == Resource::kMainResource &&
+      resource_request.Url().ProtocolIs(kContentIdScheme)) {
     return nullptr;
+  }
 
   bool is_data_url = resource_request.Url().ProtocolIsData();
   bool is_static_data = is_data_url || substitute_data.IsValid() || archive_;
   if (is_static_data) {
     resource = ResourceForStaticData(params, factory, substitute_data);
     // Abort the request if the archive doesn't contain the resource, except in
     // the case of data URLs which might have resources such as fonts that need
     // to be decoded only on demand. These data URLs are allowed to be
     // processed using the normal ResourceFetcher machinery.
     if (!resource && !is_data_url && archive_)
@@ skipping 1018 matching lines @@
   visitor->Trace(archive_);
   visitor->Trace(loaders_);
   visitor->Trace(non_blocking_loaders_);
   visitor->Trace(document_resources_);
   visitor->Trace(preloads_);
   visitor->Trace(matched_preloads_);
   visitor->Trace(resource_timing_info_map_);
 }
 
 }  // namespace blink