Skip to content
Permalink
Browse files
Fix webpushd crash when removing notification permissions
https://bugs.webkit.org/show_bug.cgi?id=240737

Reviewed by Chris Dumez and Geoffrey Garen.

When removing notification permissions for an origin, webpushd crashes because of a RELEASE_ASSERT
that fires in AppBundleRequest::start when INSTALL_COORDINATION_BUNDLES isn't enabled and mock
bundles aren't enabled.

Fix this by removing the calls into AppBundleRequest when INSTALL_COORDINATION_BUNDLES isn't enabled.
Those requests are only meant to run when that feature is enabled.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm: Only run the permission management test
  when INSTALL_COORDINATION_BUNDLES is enabled (e.g. on iOS); that is the only time it makes sense
  to test that codepath.
* Source/WebKit/webpushd/WebPushDaemon.h:
* Source/WebKit/webpushd/WebPushDaemon.mm:
(WebPushD::Daemon::deletePushRegistration):
(WebPushD::Daemon::deletePushAndNotificationRegistration):

Canonical link: https://commits.webkit.org/250904@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294726 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
bnham committed May 24, 2022
1 parent 7e0c90e commit cfafb5d1e3765cd7ea99e7f380d9b67c0815a390
Showing 3 changed files with 84 additions and 70 deletions.
@@ -74,6 +74,7 @@ class Daemon {
void echoTwice(ClientConnection*, const String&, CompletionHandler<void(const String&)>&& replySender);
void requestSystemNotificationPermission(ClientConnection*, const String&, CompletionHandler<void(bool)>&& replySender);
void getOriginsWithPushAndNotificationPermissions(ClientConnection*, CompletionHandler<void(const Vector<String>&)>&& replySender);
void deletePushRegistration(const String&, const String&, CompletionHandler<void()>&&);
void deletePushAndNotificationRegistration(ClientConnection*, const String& originString, CompletionHandler<void(const String&)>&& replySender);
void setDebugModeIsEnabled(ClientConnection*, bool);
void updateConnectionConfiguration(ClientConnection*, const WebPushDaemonConnectionConfiguration&);
@@ -505,25 +505,38 @@ void handleWebPushDMessage(ClientConnection* connection, Span<const uint8_t> enc
#endif
}

void Daemon::deletePushRegistration(const String& bundleIdentifier, const String& originString, CompletionHandler<void()>&& callback)
{
runAfterStartingPushService([this, bundleIdentifier, originString, callback = WTFMove(callback)]() mutable {
if (!m_pushService) {
callback();
return;
}

m_pushService->removeRecordsForBundleIdentifierAndOrigin(bundleIdentifier, originString, [callback = WTFMove(callback)](auto&&) mutable {
callback();
});
});
}

void Daemon::deletePushAndNotificationRegistration(ClientConnection* connection, const String& originString, CompletionHandler<void(const String&)>&& replySender)
{
if (!canRegisterForNotifications(*connection)) {
replySender("Could not delete push and notification registrations for connection: Unknown host application code signing identifier"_s);
return;
}

#if ENABLE(INSTALL_COORDINATION_BUNDLES)
connection->enqueueAppBundleRequest(makeUnique<AppBundleDeletionRequest>(*connection, originString, [this, originString = String { originString }, replySender = WTFMove(replySender), bundleIdentifier = connection->hostAppCodeSigningIdentifier()](auto result) mutable {
runAfterStartingPushService([this, bundleIdentifier = WTFMove(bundleIdentifier), originString = WTFMove(originString), replySender = WTFMove(replySender), result]() mutable {
if (!m_pushService) {
replySender(result);
return;
}

m_pushService->removeRecordsForBundleIdentifierAndOrigin(bundleIdentifier, originString, [replySender = WTFMove(replySender), result](auto&&) mutable {
replySender(result);
});
deletePushRegistration(bundleIdentifier, originString, [replySender = WTFMove(replySender), result]() mutable {
replySender(result);
});
}));
#else
deletePushRegistration(connection->hostAppCodeSigningIdentifier(), originString, [replySender = WTFMove(replySender)]() mutable {
replySender(emptyString());
});
#endif
}

void Daemon::setDebugModeIsEnabled(ClientConnection* clientConnection, bool enabled)
@@ -322,67 +322,6 @@ static void sendConfigurationWithAuditToken(xpc_connection_t connection)
cleanUpTestWebPushD(tempDir);
}

TEST(WebPushD, PermissionManagement)
{
NSURL *tempDirectory = setUpTestWebPushD();

auto dataStoreConfiguration = adoptNS([_WKWebsiteDataStoreConfiguration new]);
dataStoreConfiguration.get().webPushMachServiceName = @"org.webkit.webpushtestdaemon.service";
dataStoreConfiguration.get().webPushDaemonUsesMockBundlesForTesting = YES;
auto dataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:dataStoreConfiguration.get()]);

auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
configuration.get().websiteDataStore = dataStore.get();
[configuration.get().preferences _setNotificationsEnabled:YES];
for (_WKExperimentalFeature *feature in [WKPreferences _experimentalFeatures]) {
if ([feature.key isEqualToString:@"BuiltInNotificationsEnabled"])
[[configuration preferences] _setEnabled:YES forFeature:feature];
}

auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:configuration.get()]);
auto uiDelegate = adoptNS([[NotificationPermissionDelegate alloc] init]);
[webView setUIDelegate:uiDelegate.get()];
[webView synchronouslyLoadHTMLString:@"" baseURL:[NSURL URLWithString:@"https://example.org"]];
[webView evaluateJavaScript:@"Notification.requestPermission().then(() => { alert('done') })" completionHandler:nil];
TestWebKitAPI::Util::run(&alertReceived);

static bool originOperationDone = false;
static RetainPtr<WKSecurityOrigin> origin;
[dataStore _getOriginsWithPushAndNotificationPermissions:^(NSSet<WKSecurityOrigin *> *origins) {
EXPECT_EQ([origins count], 1u);
origin = [origins anyObject];
originOperationDone = true;
}];

TestWebKitAPI::Util::run(&originOperationDone);

EXPECT_WK_STREQ(origin.get().protocol, "https");
EXPECT_WK_STREQ(origin.get().host, "example.org");

// If we failed to retrieve an expected origin, we will have failed the above checks
if (!origin) {
cleanUpTestWebPushD(tempDirectory);
return;
}

originOperationDone = false;
[dataStore _deletePushAndNotificationRegistration:origin.get() completionHandler:^(NSError *error) {
EXPECT_FALSE(!!error);
originOperationDone = true;
}];

TestWebKitAPI::Util::run(&originOperationDone);

originOperationDone = false;
[dataStore _getOriginsWithPushAndNotificationPermissions:^(NSSet<WKSecurityOrigin *> *origins) {
EXPECT_EQ([origins count], 0u);
originOperationDone = true;
}];
TestWebKitAPI::Util::run(&originOperationDone);

cleanUpTestWebPushD(tempDirectory);
}

static void clearWebsiteDataStore(WKWebsiteDataStore *store)
{
__block bool clearedStore = false;
@@ -938,6 +877,67 @@ function log(msg)

#if ENABLE(INSTALL_COORDINATION_BUNDLES)
#if USE(APPLE_INTERNAL_SDK)
TEST(WebPushD, PermissionManagement)
{
NSURL *tempDirectory = setUpTestWebPushD();

auto dataStoreConfiguration = adoptNS([_WKWebsiteDataStoreConfiguration new]);
dataStoreConfiguration.get().webPushMachServiceName = @"org.webkit.webpushtestdaemon.service";
dataStoreConfiguration.get().webPushDaemonUsesMockBundlesForTesting = YES;
auto dataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:dataStoreConfiguration.get()]);

auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
configuration.get().websiteDataStore = dataStore.get();
[configuration.get().preferences _setNotificationsEnabled:YES];
for (_WKExperimentalFeature *feature in [WKPreferences _experimentalFeatures]) {
if ([feature.key isEqualToString:@"BuiltInNotificationsEnabled"])
[[configuration preferences] _setEnabled:YES forFeature:feature];
}

auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:configuration.get()]);
auto uiDelegate = adoptNS([[NotificationPermissionDelegate alloc] init]);
[webView setUIDelegate:uiDelegate.get()];
[webView synchronouslyLoadHTMLString:@"" baseURL:[NSURL URLWithString:@"https://example.org"]];
[webView evaluateJavaScript:@"Notification.requestPermission().then(() => { alert('done') })" completionHandler:nil];
TestWebKitAPI::Util::run(&alertReceived);

static bool originOperationDone = false;
static RetainPtr<WKSecurityOrigin> origin;
[dataStore _getOriginsWithPushAndNotificationPermissions:^(NSSet<WKSecurityOrigin *> *origins) {
EXPECT_EQ([origins count], 1u);
origin = [origins anyObject];
originOperationDone = true;
}];

TestWebKitAPI::Util::run(&originOperationDone);

EXPECT_WK_STREQ(origin.get().protocol, "https");
EXPECT_WK_STREQ(origin.get().host, "example.org");

// If we failed to retrieve an expected origin, we will have failed the above checks
if (!origin) {
cleanUpTestWebPushD(tempDirectory);
return;
}

originOperationDone = false;
[dataStore _deletePushAndNotificationRegistration:origin.get() completionHandler:^(NSError *error) {
EXPECT_FALSE(!!error);
originOperationDone = true;
}];

TestWebKitAPI::Util::run(&originOperationDone);

originOperationDone = false;
[dataStore _getOriginsWithPushAndNotificationPermissions:^(NSSet<WKSecurityOrigin *> *origins) {
EXPECT_EQ([origins count], 0u);
originOperationDone = true;
}];
TestWebKitAPI::Util::run(&originOperationDone);

cleanUpTestWebPushD(tempDirectory);
}

static void deleteAllRegistrationsForDataStore(WKWebsiteDataStore *dataStore)
{
__block bool originOperationDone = false;

0 comments on commit cfafb5d

Please sign in to comment.