Commit abd5f7e4 authored by arthursonzogni's avatar arthursonzogni Committed by Commit Bot

Followup: "Fix loading cid: resources in WebView".

This is a followup of https://codereview.chromium.org/2975623002/.
It prevents the renderer from ignoring main resource requests with the "cid"
scheme and adds tests that check an Android Webview can intercept any requests
with this scheme.

BUG=739658

Change-Id: I03e039f5e84f2be8638c46812f2216803a0f1d4d
Reviewed-on: https://chromium-review.googlesource.com/574591Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Reviewed-by: default avatarNate Chapin <japhet@chromium.org>
Reviewed-by: default avatarCamille Lamy <clamy@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488330}
parent bd4e1a2e
...@@ -883,4 +883,37 @@ public class AwContentsClientShouldInterceptRequestTest extends AwTestBase { ...@@ -883,4 +883,37 @@ public class AwContentsClientShouldInterceptRequestTest extends AwTestBase {
executeJavaScriptAndWaitForResult(mAwContents, client, "1+1"); executeJavaScriptAndWaitForResult(mAwContents, client, "1+1");
signalAfterSendingIpc.countDown(); signalAfterSendingIpc.countDown();
} }
// Webview must be able to intercept requests with the content-id scheme.
// See https://crbug.com/739658.
@SmallTest
@Feature({"AndroidWebView"})
public void testContentIdImage() throws Throwable {
final String imageContentIdUrl = "cid://intercept-me";
final String pageUrl = addPageToTestServer(mWebServer, "/main.html",
CommonResources.makeHtmlPageFrom("", "<img src=\'" + imageContentIdUrl + "\'>"));
int callCount = mShouldInterceptRequestHelper.getCallCount();
loadUrlAsync(mAwContents, pageUrl);
mShouldInterceptRequestHelper.waitForCallback(callCount, 2);
assertEquals(2, mShouldInterceptRequestHelper.getUrls().size());
assertEquals(imageContentIdUrl, mShouldInterceptRequestHelper.getUrls().get(1));
}
// Webview must be able to intercept requests with the content-id scheme.
// See https://crbug.com/739658.
@SmallTest
@Feature({"AndroidWebView"})
public void testContentIdIframe() throws Throwable {
final String iframeContentIdUrl = "cid://intercept-me";
final String pageUrl = addPageToTestServer(mWebServer, "/main.html",
CommonResources.makeHtmlPageFrom(
"", "<iframe src=\'" + iframeContentIdUrl + "\'></iframe>"));
int callCount = mShouldInterceptRequestHelper.getCallCount();
loadUrlAsync(mAwContents, pageUrl);
mShouldInterceptRequestHelper.waitForCallback(callCount, 2);
assertEquals(2, mShouldInterceptRequestHelper.getUrls().size());
assertEquals(iframeContentIdUrl, mShouldInterceptRequestHelper.getUrls().get(1));
}
} }
...@@ -24,7 +24,7 @@ bool ShouldMakeNetworkRequestForURL(const GURL& url) { ...@@ -24,7 +24,7 @@ bool ShouldMakeNetworkRequestForURL(const GURL& url) {
// Javascript URLs, srcdoc, schemes that don't load data should not send a // Javascript URLs, srcdoc, schemes that don't load data should not send a
// request to the network stack. // request to the network stack.
if (url.SchemeIs(url::kJavaScriptScheme) || url.is_empty() || if (url.SchemeIs(url::kJavaScriptScheme) || url.is_empty() ||
url.SchemeIs(url::kContentIDScheme) || url == content::kAboutSrcDocURL) { url == content::kAboutSrcDocURL) {
return false; return false;
} }
......
...@@ -17,11 +17,11 @@ TEST(NavigationParamsTest, ShouldMakeNetworkRequestForURL) { ...@@ -17,11 +17,11 @@ TEST(NavigationParamsTest, ShouldMakeNetworkRequestForURL) {
EXPECT_TRUE(ShouldMakeNetworkRequestForURL(GURL("http://foo/bar.html"))); EXPECT_TRUE(ShouldMakeNetworkRequestForURL(GURL("http://foo/bar.html")));
EXPECT_TRUE(ShouldMakeNetworkRequestForURL(GURL("https://foo/bar.html"))); EXPECT_TRUE(ShouldMakeNetworkRequestForURL(GURL("https://foo/bar.html")));
EXPECT_TRUE(ShouldMakeNetworkRequestForURL(GURL("data://foo"))); EXPECT_TRUE(ShouldMakeNetworkRequestForURL(GURL("data://foo")));
EXPECT_TRUE(ShouldMakeNetworkRequestForURL(GURL("cid:foo@bar")));
EXPECT_FALSE(ShouldMakeNetworkRequestForURL(GURL("about:blank"))); EXPECT_FALSE(ShouldMakeNetworkRequestForURL(GURL("about:blank")));
EXPECT_FALSE(ShouldMakeNetworkRequestForURL(GURL("about:srcdoc"))); EXPECT_FALSE(ShouldMakeNetworkRequestForURL(GURL("about:srcdoc")));
EXPECT_FALSE(ShouldMakeNetworkRequestForURL(GURL("javascript://foo.js"))); EXPECT_FALSE(ShouldMakeNetworkRequestForURL(GURL("javascript://foo.js")));
EXPECT_FALSE(ShouldMakeNetworkRequestForURL(GURL("cid:foo@bar")));
EXPECT_FALSE(ShouldMakeNetworkRequestForURL(GURL())); EXPECT_FALSE(ShouldMakeNetworkRequestForURL(GURL()));
} }
......
...@@ -633,16 +633,6 @@ Resource* ResourceFetcher::RequestResource( ...@@ -633,16 +633,6 @@ Resource* ResourceFetcher::RequestResource(
params.Options().initiator_info.name); params.Options().initiator_info.name);
} }
// 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
// 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_data_url = resource_request.Url().ProtocolIsData();
bool is_static_data = is_data_url || substitute_data.IsValid() || archive_; bool is_static_data = is_data_url || substitute_data.IsValid() || archive_;
if (is_static_data) { if (is_static_data) {
......
...@@ -735,6 +735,11 @@ TEST_F(ResourceFetcherTest, ContentTypeDataURL) { ...@@ -735,6 +735,11 @@ TEST_F(ResourceFetcherTest, ContentTypeDataURL) {
EXPECT_EQ("text/testmimetype", resource->GetResponse().HttpContentType()); EXPECT_EQ("text/testmimetype", resource->GetResponse().HttpContentType());
} }
// Request with the Content-ID scheme must not be canceled, even if there is no
// MHTMLArchive to serve them.
// Note: Not blocking it is important because there are some embedders of
// Android WebView that are intercepting Content-ID URLs and serve their own
// resources. Please see https://crbug.com/739658.
TEST_F(ResourceFetcherTest, ContentIdURL) { TEST_F(ResourceFetcherTest, ContentIdURL) {
KURL url(kParsedURLString, "cid:0123456789@example.com"); KURL url(kParsedURLString, "cid:0123456789@example.com");
ResourceResponse response; ResourceResponse response;
...@@ -744,8 +749,7 @@ TEST_F(ResourceFetcherTest, ContentIdURL) { ...@@ -744,8 +749,7 @@ TEST_F(ResourceFetcherTest, ContentIdURL) {
ResourceFetcher* fetcher = ResourceFetcher::Create(Context()); ResourceFetcher* fetcher = ResourceFetcher::Create(Context());
// Fetching a main resource with the Content-ID scheme must be canceled if // Main resource case.
// there is no MHTMLArchive.
{ {
ResourceRequest resource_request(url); ResourceRequest resource_request(url);
resource_request.SetRequestContext(WebURLRequest::kRequestContextIframe); resource_request.SetRequestContext(WebURLRequest::kRequestContextIframe);
...@@ -753,13 +757,10 @@ TEST_F(ResourceFetcherTest, ContentIdURL) { ...@@ -753,13 +757,10 @@ TEST_F(ResourceFetcherTest, ContentIdURL) {
FetchParameters fetch_params(resource_request); FetchParameters fetch_params(resource_request);
RawResource* resource = RawResource* resource =
RawResource::FetchMainResource(fetch_params, fetcher, SubstituteData()); RawResource::FetchMainResource(fetch_params, fetcher, SubstituteData());
EXPECT_EQ(nullptr, resource); EXPECT_NE(nullptr, resource);
} }
// For all the other resource type, it must not be canceled. // Subresource case.
// Note: It is important not to cancel them because there are some embedders
// of WebView that are using Content-ID URLs for sub-resources, even without
// any MHTMLArchive. Please see https://crbug.com/739658.
{ {
ResourceRequest resource_request(url); ResourceRequest resource_request(url);
resource_request.SetRequestContext(WebURLRequest::kRequestContextVideo); resource_request.SetRequestContext(WebURLRequest::kRequestContextVideo);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment