- Revision
- 280016
- Author
- katherine_che...@apple.com
- Date
- 2021-07-16 20:24:25 -0700 (Fri, 16 Jul 2021)
Log Message
WKWebView _javascript_ injection doesn't work if app includes WKAppBoundDomains
https://bugs.webkit.org/show_bug.cgi?id=227589
<rdar://problem/80327452>
Reviewed by Brent Fulgham.
Source/WebCore:
Update service worker code to always allow workers on localhost to be
registered. Also add logic so localhost registrations aren't added
to the total count of 3 registrations.
* workers/service/server/SWServer.cpp:
(WebCore::SWServer::addRegistration):
We only want to increment the count if this registration is not
already in the mapping, otherwise it gets double counted.
(WebCore::SWServer::removeRegistration):
Ditto for removing a registration, we should only decrement the count
if we find a matching key.
(WebCore::SWServer::validateRegistrationDomain):
(WebCore::SWServer::removeFromScopeToRegistrationMap):
Ditto.
* workers/service/server/SWServer.h:
Source/WebKit:
Apps should not have to specify localhost in their Info.plist in order
to load local content in app-bound mode. This patch adds a check for
localhost or a loopback IP address and forces an app into app-bound
mode in this case.
Since all layout tests use localhost and 127.0.0.1 as test domains,
this patch also adds a check for enableInAppBrowserPrivacyForTesting()
which determines if we are running layout tests and does not trigger
this check in that case so we can test other behavior.
* UIProcess/WebPageProxy.cpp:
(WebKit::shouldTreatURLProtocolAsAppBound):
(WebKit::WebPageProxy::setIsNavigatingToAppBoundDomainAndCheckIfPermitted):
(WebKit::WebPageProxy::decidePolicyForNavigationAction):
Tools:
Added new tests. Removed localhost and 127.0.0.1 from the Info.plist
to avoid false positive tests. Replace them with other domains so we
still test the max count.
* TestWebKitAPI/Info.plist:
* TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm:
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (280015 => 280016)
--- trunk/Source/WebCore/ChangeLog 2021-07-17 01:02:06 UTC (rev 280015)
+++ trunk/Source/WebCore/ChangeLog 2021-07-17 03:24:25 UTC (rev 280016)
@@ -1,3 +1,30 @@
+2021-07-16 Kate Cheney <katherine_che...@apple.com>
+
+ WKWebView _javascript_ injection doesn't work if app includes WKAppBoundDomains
+ https://bugs.webkit.org/show_bug.cgi?id=227589
+ <rdar://problem/80327452>
+
+ Reviewed by Brent Fulgham.
+
+ Update service worker code to always allow workers on localhost to be
+ registered. Also add logic so localhost registrations aren't added
+ to the total count of 3 registrations.
+
+ * workers/service/server/SWServer.cpp:
+ (WebCore::SWServer::addRegistration):
+ We only want to increment the count if this registration is not
+ already in the mapping, otherwise it gets double counted.
+
+ (WebCore::SWServer::removeRegistration):
+ Ditto for removing a registration, we should only decrement the count
+ if we find a matching key.
+
+ (WebCore::SWServer::validateRegistrationDomain):
+ (WebCore::SWServer::removeFromScopeToRegistrationMap):
+ Ditto.
+
+ * workers/service/server/SWServer.h:
+
2021-07-16 Alex Christensen <achristen...@webkit.org>
Network access prevention SPI should prevent preconnecting, and it should first allow injected bundle to change request
Modified: trunk/Source/WebCore/workers/service/server/SWServer.cpp (280015 => 280016)
--- trunk/Source/WebCore/workers/service/server/SWServer.cpp 2021-07-17 01:02:06 UTC (rev 280015)
+++ trunk/Source/WebCore/workers/service/server/SWServer.cpp 2021-07-17 03:24:25 UTC (rev 280016)
@@ -184,6 +184,9 @@
void SWServer::addRegistration(std::unique_ptr<SWServerRegistration>&& registration)
{
+ if (!m_scopeToRegistrationMap.contains(registration->key()) && !SecurityOrigin::isLocalHostOrLoopbackIPAddress(registration->key().topOrigin().host))
+ m_uniqueRegistrationCount++;
+
m_originStore->add(registration->key().topOrigin());
auto registrationID = registration->identifier();
ASSERT(!m_scopeToRegistrationMap.contains(registration->key()));
@@ -198,8 +201,11 @@
ASSERT(registration);
auto it = m_scopeToRegistrationMap.find(registration->key());
- if (it != m_scopeToRegistrationMap.end() && it->value == registration.get())
+ if (it != m_scopeToRegistrationMap.end() && it->value == registration.get()) {
m_scopeToRegistrationMap.remove(it);
+ if (!SecurityOrigin::isLocalHostOrLoopbackIPAddress(registration->key().topOrigin().host))
+ m_uniqueRegistrationCount--;
+ }
m_originStore->remove(registration->key().topOrigin());
if (m_registrationStore)
@@ -343,16 +349,16 @@
void SWServer::validateRegistrationDomain(WebCore::RegistrableDomain domain, CompletionHandler<void(bool)>&& completionHandler)
{
if (m_hasServiceWorkerEntitlement || m_hasReceivedAppBoundDomains) {
- completionHandler(m_appBoundDomains.contains(domain) && m_scopeToRegistrationMap.keys().size() < maxRegistrationCount);
+ completionHandler(SecurityOrigin::isLocalHostOrLoopbackIPAddress(domain.string()) || (m_appBoundDomains.contains(domain) && m_uniqueRegistrationCount < maxRegistrationCount));
return;
}
-
+
m_appBoundDomainsCallback([this, weakThis = makeWeakPtr(this), domain = WTFMove(domain), completionHandler = WTFMove(completionHandler)](auto&& appBoundDomains) mutable {
if (!weakThis)
return;
m_hasReceivedAppBoundDomains = true;
m_appBoundDomains = WTFMove(appBoundDomains);
- completionHandler(m_appBoundDomains.contains(domain) && m_scopeToRegistrationMap.keys().size() < maxRegistrationCount);
+ completionHandler(SecurityOrigin::isLocalHostOrLoopbackIPAddress(domain.string()) || (m_appBoundDomains.contains(domain) && m_uniqueRegistrationCount < maxRegistrationCount));
});
}
@@ -1011,6 +1017,9 @@
void SWServer::removeFromScopeToRegistrationMap(const ServiceWorkerRegistrationKey& key)
{
+ if (m_scopeToRegistrationMap.contains(key) && !SecurityOrigin::isLocalHostOrLoopbackIPAddress(key.topOrigin().host))
+ m_uniqueRegistrationCount--;
+
m_scopeToRegistrationMap.remove(key);
}
Modified: trunk/Source/WebCore/workers/service/server/SWServer.h (280015 => 280016)
--- trunk/Source/WebCore/workers/service/server/SWServer.h 2021-07-17 01:02:06 UTC (rev 280015)
+++ trunk/Source/WebCore/workers/service/server/SWServer.h 2021-07-17 03:24:25 UTC (rev 280016)
@@ -283,6 +283,7 @@
HashSet<WebCore::RegistrableDomain> m_appBoundDomains;
bool m_hasServiceWorkerEntitlement { false };
bool m_hasReceivedAppBoundDomains { false };
+ unsigned m_uniqueRegistrationCount { 0 };
};
} // namespace WebCore
Modified: trunk/Source/WebKit/ChangeLog (280015 => 280016)
--- trunk/Source/WebKit/ChangeLog 2021-07-17 01:02:06 UTC (rev 280015)
+++ trunk/Source/WebKit/ChangeLog 2021-07-17 03:24:25 UTC (rev 280016)
@@ -1,3 +1,26 @@
+2021-07-16 Kate Cheney <katherine_che...@apple.com>
+
+ WKWebView _javascript_ injection doesn't work if app includes WKAppBoundDomains
+ https://bugs.webkit.org/show_bug.cgi?id=227589
+ <rdar://problem/80327452>
+
+ Reviewed by Brent Fulgham.
+
+ Apps should not have to specify localhost in their Info.plist in order
+ to load local content in app-bound mode. This patch adds a check for
+ localhost or a loopback IP address and forces an app into app-bound
+ mode in this case.
+
+ Since all layout tests use localhost and 127.0.0.1 as test domains,
+ this patch also adds a check for enableInAppBrowserPrivacyForTesting()
+ which determines if we are running layout tests and does not trigger
+ this check in that case so we can test other behavior.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::shouldTreatURLProtocolAsAppBound):
+ (WebKit::WebPageProxy::setIsNavigatingToAppBoundDomainAndCheckIfPermitted):
+ (WebKit::WebPageProxy::decidePolicyForNavigationAction):
+
2021-07-16 Alex Christensen <achristen...@webkit.org>
Prevent HSTS tracking mitigation for top level navigation requests
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (280015 => 280016)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2021-07-17 01:02:06 UTC (rev 280015)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2021-07-17 03:24:25 UTC (rev 280016)
@@ -3321,9 +3321,15 @@
};
#if ENABLE(APP_BOUND_DOMAINS)
-static bool shouldTreatURLProtocolAsAppBound(const URL& requestURL)
+static bool shouldTreatURLProtocolAsAppBound(const URL& requestURL, bool isRunningTest)
{
- return requestURL.protocolIsAbout() || requestURL.protocolIsData() || requestURL.protocolIsBlob() || requestURL.isLocalFile() || requestURL.protocolIsJavaScript();
+ return !isRunningTest
+ && (SecurityOrigin::isLocalHostOrLoopbackIPAddress(requestURL.host())
+ || requestURL.protocolIsAbout()
+ || requestURL.protocolIsData()
+ || requestURL.protocolIsBlob()
+ || requestURL.isLocalFile()
+ || requestURL.protocolIsJavaScript());
}
bool WebPageProxy::setIsNavigatingToAppBoundDomainAndCheckIfPermitted(bool isMainFrame, const URL& requestURL, std::optional<NavigatingToAppBoundDomain> isNavigatingToAppBoundDomain)
@@ -3341,7 +3347,7 @@
if (m_ignoresAppBoundDomains)
return true;
- if (isMainFrame && shouldTreatURLProtocolAsAppBound(requestURL)) {
+ if (isMainFrame && shouldTreatURLProtocolAsAppBound(requestURL, websiteDataStore().configuration().enableInAppBrowserPrivacyForTesting())) {
isNavigatingToAppBoundDomain = NavigatingToAppBoundDomain::Yes;
m_limitsNavigationsToAppBoundDomains = true;
}
@@ -5434,7 +5440,7 @@
if (shouldExpectSafeBrowsingResult == ShouldExpectSafeBrowsingResult::Yes)
beginSafeBrowsingCheck(request.url(), frame.isMainFrame(), listener);
#if ENABLE(APP_BOUND_DOMAINS)
- bool shouldSendSecurityOriginData = !frame.isMainFrame() && shouldTreatURLProtocolAsAppBound(request.url());
+ bool shouldSendSecurityOriginData = !frame.isMainFrame() && shouldTreatURLProtocolAsAppBound(request.url(), websiteDataStore().configuration().enableInAppBrowserPrivacyForTesting());
auto host = shouldSendSecurityOriginData ? frameInfo.securityOrigin.host : request.url().host();
auto protocol = shouldSendSecurityOriginData ? frameInfo.securityOrigin.protocol : request.url().protocol();
m_websiteDataStore->beginAppBoundDomainCheck(host.toString(), protocol.toString(), listener);
Modified: trunk/Tools/ChangeLog (280015 => 280016)
--- trunk/Tools/ChangeLog 2021-07-17 01:02:06 UTC (rev 280015)
+++ trunk/Tools/ChangeLog 2021-07-17 03:24:25 UTC (rev 280016)
@@ -1,3 +1,19 @@
+2021-07-16 Kate Cheney <katherine_che...@apple.com>
+
+ WKWebView _javascript_ injection doesn't work if app includes WKAppBoundDomains
+ https://bugs.webkit.org/show_bug.cgi?id=227589
+ <rdar://problem/80327452>
+
+ Reviewed by Brent Fulgham.
+
+ Added new tests. Removed localhost and 127.0.0.1 from the Info.plist
+ to avoid false positive tests. Replace them with other domains so we
+ still test the max count.
+
+ * TestWebKitAPI/Info.plist:
+ * TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm:
+ (TEST):
+
2021-07-16 Alex Christensen <achristen...@webkit.org>
Prevent HSTS tracking mitigation for top level navigation requests
Modified: trunk/Tools/TestWebKitAPI/Info.plist (280015 => 280016)
--- trunk/Tools/TestWebKitAPI/Info.plist 2021-07-17 01:02:06 UTC (rev 280015)
+++ trunk/Tools/TestWebKitAPI/Info.plist 2021-07-17 03:24:25 UTC (rev 280016)
@@ -11,10 +11,7 @@
<string>apple.com</string>
<string>sub.domain.webkit.org/road/to/nowhere/</string>
<string>webkit.org</string>
- <string>localhost:8000</string>
- <string>localhost/road/to/nowhere/</string>
<string>file:///some/file</string>
- <string>127.0.0.1</string>
<string></string>
<string> </string>
<string>https://sub.domain.webkit.org</string>
@@ -24,7 +21,9 @@
<string>http://www.example.com/</string>
<string>http://bar.com/</string>
<string>http://foo.com/</string>
+ <string>https://searchforlongboards.biz</string>
<string>app-bound-custom-scheme:</string>
+ <string>longboardshop.biz</string>
<string>should-not-be-included:</string>
</array>
<key>CFBundleName</key>
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm (280015 => 280016)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm 2021-07-17 01:02:06 UTC (rev 280015)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/InAppBrowserPrivacy.mm 2021-07-17 03:24:25 UTC (rev 280016)
@@ -271,7 +271,7 @@
initializeInAppBrowserPrivacyTestSettings();
isDone = false;
[[WKWebsiteDataStore defaultDataStore] _appBoundDomains:^(NSArray<NSString *> *domains) {
- NSArray *domainsToCompare = @[@"127.0.0.1", @"apple.com", @"bar.com", @"example.com", @"foo.com", @"localhost", @"testdomain1", @"webkit.org"];
+ NSArray *domainsToCompare = @[@"apple.com", @"bar.com", @"example.com", @"foo.com", @"longboardshop.biz", @"searchforlongboards.biz", @"testdomain1", @"webkit.org"];
NSArray *sortedDomains = [domains sortedArrayUsingSelector:@selector(caseInsensitiveCompare:)];
@@ -361,6 +361,60 @@
TestWebKitAPI::Util::run(&isDone);
}
+TEST(InAppBrowserPrivacy, LocalHostIsAppBound)
+{
+ initializeInAppBrowserPrivacyTestSettings();
+ isDone = false;
+
+ auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ auto schemeHandler = adoptNS([[InAppBrowserSchemeHandler alloc] init]);
+ [configuration setURLSchemeHandler:schemeHandler.get() forURLScheme:@"in-app-browser"];
+ [configuration setLimitsNavigationsToAppBoundDomains:YES];
+
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+ auto delegate = adoptNS([AppBoundDomainDelegate new]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"in-app-browser://localhost/app-bound-domain-load"]];
+ [webView loadRequest:request];
+ [delegate waitForDidFinishNavigation];
+
+ isDone = false;
+ [webView _isForcedIntoAppBoundMode:^(BOOL isForcedIntoAppBoundMode) {
+ EXPECT_TRUE(isForcedIntoAppBoundMode);
+ cleanUpInAppBrowserPrivacyTestSettings();
+ isDone = true;
+ }];
+ TestWebKitAPI::Util::run(&isDone);
+}
+
+TEST(InAppBrowserPrivacy, LoopbackIPAddressIsAppBound)
+{
+ initializeInAppBrowserPrivacyTestSettings();
+ isDone = false;
+
+ auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ auto schemeHandler = adoptNS([[InAppBrowserSchemeHandler alloc] init]);
+ [configuration setURLSchemeHandler:schemeHandler.get() forURLScheme:@"in-app-browser"];
+ [configuration setLimitsNavigationsToAppBoundDomains:YES];
+
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+ auto delegate = adoptNS([AppBoundDomainDelegate new]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"in-app-browser://127.0.0.1/app-bound-domain-load"]];
+ [webView loadRequest:request];
+ [delegate waitForDidFinishNavigation];
+
+ isDone = false;
+ [webView _isForcedIntoAppBoundMode:^(BOOL isForcedIntoAppBoundMode) {
+ EXPECT_TRUE(isForcedIntoAppBoundMode);
+ cleanUpInAppBrowserPrivacyTestSettings();
+ isDone = true;
+ }];
+ TestWebKitAPI::Util::run(&isDone);
+}
+
static NSString *styleSheetSource = @"body { background-color: green !important; }";
static NSString *backgroundColorScript = @"window.getComputedStyle(document.body, null).getPropertyValue('background-color')";
static const char* redInRGB = "rgb(255, 0, 0)";
