From c2b752b1d03355b2e9bbde897731b616e9f70883 Mon Sep 17 00:00:00 2001 From: Daniel Cheng Date: Wed, 28 Nov 2018 00:03:29 +0000 Subject: [PATCH] //url: Allow Android WebView to use origins with non-standard schemes This adds an escape hatch so that Android WebView can restore the old behavior before https://crrev.com/c/1208811/. Bug: 896059 Change-Id: I90cc51fe5c6ddaa95281a51a5b89795e8a3958fe Reviewed-on: https://chromium-review.googlesource.com/c/1338660 Reviewed-by: Dmitry Gozman Reviewed-by: Changwan Ryu Reviewed-by: Tom Sepez Commit-Queue: Daniel Cheng Cr-Commit-Position: refs/heads/master@{#611436} --- android_webview/common/aw_content_client.cc | 1 + .../test/LoadDataWithBaseUrlTest.java | 6 ++---- content/common/url_schemes.cc | 6 ++++++ content/public/common/content_client.h | 5 +++++ .../platform/weborigin/security_origin.cc | 21 ++++++++++++------- .../weborigin/security_origin_test.cc | 17 +++++++++++++++ url/origin.cc | 3 ++- url/origin_unittest.cc | 14 +++++++++++++ url/scheme_host_port.cc | 12 +++++++++-- url/url_util.cc | 11 ++++++++++ url/url_util.h | 11 ++++++++++ 11 files changed, 93 insertions(+), 14 deletions(-) diff --git a/android_webview/common/aw_content_client.cc b/android_webview/common/aw_content_client.cc index 1514c9440f6562..babb0fe0c8600c 100644 --- a/android_webview/common/aw_content_client.cc +++ b/android_webview/common/aw_content_client.cc @@ -45,6 +45,7 @@ void AwContentClient::AddAdditionalSchemes(Schemes* schemes) { schemes->local_schemes.push_back(url::kContentScheme); schemes->secure_schemes.push_back( android_webview::kAndroidWebViewVideoPosterScheme); + schemes->allow_non_standard_schemes_in_origins = true; } std::string AwContentClient::GetProduct() const { diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java index 4d60425893a5e1..4d1380f30fbda9 100644 --- a/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java +++ b/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java @@ -570,8 +570,7 @@ public void testWindowOriginForCustomSchemeUrl() throws Throwable { AwSettings contentSettings = mActivityTestRule.getAwSettingsOnUiThread(mAwContents); contentSettings.setJavaScriptEnabled(true); loadDataWithBaseUrlSync("", "text/html", false, baseUri, null); - // TODO(dcheng): https://crbug.com/896059 this should be fixed as "x-thread://". - Assert.assertEquals("\"null\"", + Assert.assertEquals("\"x-thread://\"", mActivityTestRule.executeJavaScriptAndWaitForResult( mAwContents, mContentsClient, "window.origin;")); } @@ -588,8 +587,7 @@ public void testXhrForHttpSchemeUrl() throws Throwable { @Feature({"AndroidWebView"}) // https://crbug.com/900528 public void testXhrForCustomSchemeUrl() throws Throwable { - // TODO(dcheng): this should be fixed by https://crbug.com/900528. - Assert.assertFalse(verifyXhrForUrls("myscheme://mydomain/1", "myscheme://mydomain/2")); + Assert.assertTrue(verifyXhrForUrls("myscheme://mydomain/1", "myscheme://mydomain/2")); } /** diff --git a/content/common/url_schemes.cc b/content/common/url_schemes.cc index c922d9755135bf..e532406b0a2243 100644 --- a/content/common/url_schemes.cc +++ b/content/common/url_schemes.cc @@ -10,6 +10,7 @@ #include "base/no_destructor.h" #include "base/strings/string_util.h" +#include "build/build_config.h" #include "content/public/common/content_client.h" #include "content/public/common/url_constants.h" #include "url/url_util.h" @@ -89,6 +90,11 @@ void RegisterContentSchemes(bool lock_schemes) { for (auto& scheme : schemes.empty_document_schemes) url::AddEmptyDocumentScheme(scheme.c_str()); +#if defined(OS_ANDROID) + if (schemes.allow_non_standard_schemes_in_origins) + url::EnableNonStandardSchemesForAndroidWebView(); +#endif + // Prevent future modification of the scheme lists. This is to prevent // accidental creation of data races in the program. Add*Scheme aren't // threadsafe so must be called when GURL isn't used on any other thread. This diff --git a/content/public/common/content_client.h b/content/public/common/content_client.h index b939bc2a9edf8e..618a328b4767b2 100644 --- a/content/public/common/content_client.h +++ b/content/public/common/content_client.h @@ -139,6 +139,11 @@ class CONTENT_EXPORT ContentClient { // Registers a URL scheme as strictly empty documents, allowing them to // commit synchronously. std::vector empty_document_schemes; +#if defined(OS_ANDROID) + // Normally, non-standard schemes canonicalize to opaque origins. However, + // Android WebView requires non-standard schemes to still be preserved. + bool allow_non_standard_schemes_in_origins = false; +#endif }; virtual void AddAdditionalSchemes(Schemes* schemes) {} diff --git a/third_party/blink/renderer/platform/weborigin/security_origin.cc b/third_party/blink/renderer/platform/weborigin/security_origin.cc index 98500fba454b5e..81b8fa13fb8e9a 100644 --- a/third_party/blink/renderer/platform/weborigin/security_origin.cc +++ b/third_party/blink/renderer/platform/weborigin/security_origin.cc @@ -47,6 +47,7 @@ #include "third_party/blink/renderer/platform/wtf/wtf.h" #include "url/url_canon.h" #include "url/url_canon_ip.h" +#include "url/url_util.h" namespace blink { @@ -121,13 +122,19 @@ static bool ShouldTreatAsOpaqueOrigin(const KURL& url) { return true; // Nonstandard schemes and unregistered schemes aren't known to contain hosts - // and/or ports, so they'll usually be placed in opaque origins. An exception - // is made for non-standard local schemes. - // TODO: Migrate "content:" and "externalfile:" to be standard schemes, and - // remove the local scheme exception. - if (!relevant_url.CanSetHostOrPort() && - !SchemeRegistry::ShouldTreatURLSchemeAsLocal(relevant_url.Protocol())) { - return true; + // and/or ports, so they'll usually be placed in opaque origins. + if (!relevant_url.CanSetHostOrPort()) { + // A temporary exception is made for non-standard local schemes. + // TODO: Migrate "content:" and "externalfile:" to be standard schemes, and + // remove the local scheme exception. + if (SchemeRegistry::ShouldTreatURLSchemeAsLocal(relevant_url.Protocol())) + return false; + + // Otherwise, treat non-standard origins as opaque, unless the Android + // WebView workaround is enabled. If the workaround is enabled, return false + // so that the scheme is retained, to avoid breaking XHRs on custom schemes, + // et cetera. + return !url::AllowNonStandardSchemesForAndroidWebView(); } // This is the common case. diff --git a/third_party/blink/renderer/platform/weborigin/security_origin_test.cc b/third_party/blink/renderer/platform/weborigin/security_origin_test.cc index 8a95d1e967a410..e77bd9220af4f1 100644 --- a/third_party/blink/renderer/platform/weborigin/security_origin_test.cc +++ b/third_party/blink/renderer/platform/weborigin/security_origin_test.cc @@ -790,4 +790,21 @@ TEST_F(SecurityOriginTest, ToTokenForFastCheck) { } } +TEST_F(SecurityOriginTest, NonStandardScheme) { + scoped_refptr origin = + SecurityOrigin::CreateFromString("cow://"); + EXPECT_TRUE(origin->IsOpaque()); +} + +TEST_F(SecurityOriginTest, NonStandardSchemeWithAndroidWebViewHack) { + url::EnableNonStandardSchemesForAndroidWebView(); + scoped_refptr origin = + SecurityOrigin::CreateFromString("cow://"); + EXPECT_FALSE(origin->IsOpaque()); + EXPECT_EQ("cow", origin->Protocol()); + EXPECT_EQ("", origin->Host()); + EXPECT_EQ(0, origin->Port()); + url::Shutdown(); +} + } // namespace blink diff --git a/url/origin.cc b/url/origin.cc index 1c78bc954b3d0a..f6403f58b6dc78 100644 --- a/url/origin.cc +++ b/url/origin.cc @@ -42,7 +42,8 @@ Origin Origin::Create(const GURL& url) { // It's SchemeHostPort's responsibility to filter out unrecognized schemes; // sanity check that this is happening. DCHECK(tuple.IsInvalid() || url.IsStandard() || - base::ContainsValue(GetLocalSchemes(), url.scheme_piece())); + base::ContainsValue(GetLocalSchemes(), url.scheme_piece()) || + AllowNonStandardSchemesForAndroidWebView()); } if (tuple.IsInvalid()) diff --git a/url/origin_unittest.cc b/url/origin_unittest.cc index 0b57da088ac3b6..5f889c09a142f6 100644 --- a/url/origin_unittest.cc +++ b/url/origin_unittest.cc @@ -654,4 +654,18 @@ TEST_F(OriginTest, DebugAlias) { EXPECT_STREQ("https://foo.com", origin1_debug_alias); } +TEST_F(OriginTest, NonStandardScheme) { + Origin origin = Origin::Create(GURL("cow://")); + EXPECT_TRUE(origin.opaque()); +} +TEST_F(OriginTest, NonStandardSchemeWithAndroidWebViewHack) { + EnableNonStandardSchemesForAndroidWebView(); + Origin origin = Origin::Create(GURL("cow://")); + EXPECT_FALSE(origin.opaque()); + EXPECT_EQ("cow", origin.scheme()); + EXPECT_EQ("", origin.host()); + EXPECT_EQ(0, origin.port()); + Shutdown(); +} + } // namespace url diff --git a/url/scheme_host_port.cc b/url/scheme_host_port.cc index 0d681f139183fd..e588aece42ff5c 100644 --- a/url/scheme_host_port.cc +++ b/url/scheme_host_port.cc @@ -52,6 +52,10 @@ bool IsValidInput(const base::StringPiece& scheme, const base::StringPiece& host, uint16_t port, SchemeHostPort::ConstructPolicy policy) { + // Empty schemes are never valid. + if (scheme.empty()) + return false; + SchemeType scheme_type = SCHEME_WITH_HOST_PORT_AND_USER_INFORMATION; bool is_standard = GetStandardSchemeType( scheme.data(), @@ -67,7 +71,10 @@ bool IsValidInput(const base::StringPiece& scheme, if (base::ContainsValue(GetLocalSchemes(), scheme) && host.empty() && port == 0) return true; - return false; + + // Otherwise, allow non-standard schemes only if the Android WebView + // workaround is enabled. + return AllowNonStandardSchemesForAndroidWebView(); } switch (scheme_type) { @@ -135,7 +142,8 @@ SchemeHostPort::SchemeHostPort(std::string scheme, scheme_ = std::move(scheme); host_ = std::move(host); port_ = port; - DCHECK(!IsInvalid()); + DCHECK(!IsInvalid()) << "Scheme: " << scheme_ << " Host: " << host_ + << " Port: " << port; } SchemeHostPort::SchemeHostPort(base::StringPiece scheme, diff --git a/url/url_util.cc b/url/url_util.cc index a515231f555fad..29cebf96899c52 100644 --- a/url/url_util.cc +++ b/url/url_util.cc @@ -20,6 +20,8 @@ namespace url { namespace { +bool g_allow_non_standard_schemes = false; + // Pass this enum through for methods which would like to know if whitespace // removal is necessary. enum WhitespaceRemovalPolicy { @@ -530,6 +532,7 @@ void Initialize() { void Shutdown() { initialized = false; + g_allow_non_standard_schemes = false; delete standard_schemes; standard_schemes = nullptr; delete referrer_schemes; @@ -550,6 +553,14 @@ void Shutdown() { empty_document_schemes = nullptr; } +void EnableNonStandardSchemesForAndroidWebView() { + g_allow_non_standard_schemes = true; +} + +bool AllowNonStandardSchemesForAndroidWebView() { + return g_allow_non_standard_schemes; +} + void AddStandardScheme(const char* new_scheme, SchemeType type) { Initialize(); DoAddSchemeWithType(new_scheme, type, standard_schemes); diff --git a/url/url_util.h b/url/url_util.h index d4b0d77358920a..ee456a02cc09c0 100644 --- a/url/url_util.h +++ b/url/url_util.h @@ -39,6 +39,17 @@ URL_EXPORT void Shutdown(); // Schemes --------------------------------------------------------------------- +// Changes the behavior of SchemeHostPort / Origin to allow non-standard schemes +// to be specified, instead of canonicalizing them to an invalid SchemeHostPort +// or opaque Origin, respectively. This is used for Android WebView backwards +// compatibility, which allows the use of custom schemes: content hosted in +// Android WebView assumes that one URL with a non-standard scheme will be +// same-origin to another URL with the same non-standard scheme. +URL_EXPORT void EnableNonStandardSchemesForAndroidWebView(); + +// Whether or not SchemeHostPort and Origin allow non-standard schemes. +URL_EXPORT bool AllowNonStandardSchemesForAndroidWebView(); + // A pair for representing a standard scheme name and the SchemeType for it. struct URL_EXPORT SchemeWithType { const char* scheme;