Skip to content
Permalink
Browse files
Re-send connection configuration if webpushd dies
https://bugs.webkit.org/show_bug.cgi?id=240286

Reviewed by Geoffrey Garen.

If webpushd dies, all future communication with it from NetworkProcess fails because we
don't re-send the connection configuration to the daemon after the connection is
interrupted. This manifests itself as an "invalid sender" AbortError from various
PushManager methods because webpushd doesn't know the bundle identifier of the UIProcess it
is working on behalf of.

To fix this, I moved the responsibility for sending the configuration from
NetworkNotificationManager to WebPushD::Connection, and WebPushD::Connection re-sends the
configuration every time a new XPC connection is created.

Covered by a new API test.

Source/WebKit:

* NetworkProcess/NetworkSession.cpp:
(WebKit::NetworkSession::NetworkSession):
* NetworkProcess/NetworkSession.h:
(WebKit::NetworkSession::webPushDaemonUsesMockBundlesForTesting const): Deleted.
* NetworkProcess/Notifications/Cocoa/WebPushDaemonConnectionCocoa.mm:
(WebKit::WebPushD::Connection::newConnectionWasInitialized const):
* NetworkProcess/Notifications/NetworkNotificationManager.cpp:
(WebKit::NetworkNotificationManager::NetworkNotificationManager):
(WebKit::NetworkNotificationManager::sendMessage const):
(WebKit::NetworkNotificationManager::sendMessageWithReply const):
(WebKit::NetworkNotificationManager::maybeSendConnectionConfiguration const): Deleted.
* NetworkProcess/Notifications/NetworkNotificationManager.h:
* NetworkProcess/Notifications/WebPushDaemonConnection.cpp:
(WebKit::WebPushD::Connection::Connection):
* NetworkProcess/Notifications/WebPushDaemonConnection.h:
* NetworkProcess/cocoa/NetworkSessionCocoa.h:
* NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(WebKit::NetworkSessionCocoa::NetworkSessionCocoa):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm:
(TestWebKitAPI::testWebPushDaemonPList):
(TestWebKitAPI::setUpTestWebPushD):
(TestWebKitAPI::restartTestWebPushD):
(TestWebKitAPI::WebPushDTest::WebPushDTest):
* TestWebKitAPI/cocoa/DaemonTestUtilities.h:
* TestWebKitAPI/cocoa/DaemonTestUtilities.mm:
(TestWebKitAPI::restartService):

Canonical link: https://commits.webkit.org/250475@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294084 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
bnham committed May 12, 2022
1 parent 1d75ec6 commit 18ac12a87cd3d1a8545247631e8e814a0e6da424
@@ -1,3 +1,41 @@
2022-05-10 Ben Nham <nham@apple.com>

Re-send connection configuration if webpushd dies
https://bugs.webkit.org/show_bug.cgi?id=240286

Reviewed by Geoffrey Garen.

If webpushd dies, all future communication with it from NetworkProcess fails because we
don't re-send the connection configuration to the daemon after the connection is
interrupted. This manifests itself as an "invalid sender" AbortError from various
PushManager methods because webpushd doesn't know the bundle identifier of the UIProcess it
is working on behalf of.

To fix this, I moved the responsibility for sending the configuration from
NetworkNotificationManager to WebPushD::Connection, and WebPushD::Connection re-sends the
configuration every time a new XPC connection is created.

Covered by a new API test.

* NetworkProcess/NetworkSession.cpp:
(WebKit::NetworkSession::NetworkSession):
* NetworkProcess/NetworkSession.h:
(WebKit::NetworkSession::webPushDaemonUsesMockBundlesForTesting const): Deleted.
* NetworkProcess/Notifications/Cocoa/WebPushDaemonConnectionCocoa.mm:
(WebKit::WebPushD::Connection::newConnectionWasInitialized const):
* NetworkProcess/Notifications/NetworkNotificationManager.cpp:
(WebKit::NetworkNotificationManager::NetworkNotificationManager):
(WebKit::NetworkNotificationManager::sendMessage const):
(WebKit::NetworkNotificationManager::sendMessageWithReply const):
(WebKit::NetworkNotificationManager::maybeSendConnectionConfiguration const): Deleted.
* NetworkProcess/Notifications/NetworkNotificationManager.h:
* NetworkProcess/Notifications/WebPushDaemonConnection.cpp:
(WebKit::WebPushD::Connection::Connection):
* NetworkProcess/Notifications/WebPushDaemonConnection.h:
* NetworkProcess/cocoa/NetworkSessionCocoa.h:
* NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(WebKit::NetworkSessionCocoa::NetworkSessionCocoa):

2022-05-11 Yury Semikhatsky <yurys@chromium.org>

[WinCairo] Support file downloads
@@ -145,7 +145,7 @@ NetworkSession::NetworkSession(NetworkProcess& networkProcess, const NetworkSess
, m_overrideServiceWorkerRegistrationCountTestingValue(parameters.overrideServiceWorkerRegistrationCountTestingValue)
, m_storageManager(createNetworkStorageManager(networkProcess.parentProcessConnection(), parameters))
#if ENABLE(BUILT_IN_NOTIFICATIONS)
, m_notificationManager(*this, parameters.webPushMachServiceName)
, m_notificationManager(*this, parameters.webPushMachServiceName, WebPushD::WebPushDaemonConnectionConfiguration { parameters.webPushDaemonConnectionConfiguration })
#endif
#if !HAVE(NSURLSESSION_WEBSOCKET)
, m_shouldAcceptInsecureCertificatesForWebSockets(parameters.shouldAcceptInsecureCertificatesForWebSockets)
@@ -147,8 +147,6 @@ class NetworkSession : public CanMakeWeakPtr<NetworkSession> {
virtual void clearAppBoundSession() { }
#endif

virtual bool webPushDaemonUsesMockBundlesForTesting() const { return false; }

void storePrivateClickMeasurement(WebCore::PrivateClickMeasurement&&);
virtual void donateToSKAdNetwork(WebCore::PrivateClickMeasurement&&) { }
void handlePrivateClickMeasurementConversion(WebCore::PrivateClickMeasurement::AttributionTriggerData&&, const URL& requestURL, const WebCore::ResourceRequest& redirectRequest, String&& attributedBundleIdentifier);
@@ -52,7 +52,9 @@ static void addVersionAndEncodedMessageToDictionary(Vector<uint8_t>&& message, x
if (networkSession().sessionID().isEphemeral())
return;

// FIXME: Track connection state
Daemon::Encoder encoder;
encoder.encode(m_configuration);
Daemon::Connection::send(dictionaryFromMessage(WebPushD::MessageType::UpdateConnectionConfiguration, encoder.takeBuffer()).get());
}

namespace MessageInfo {
@@ -39,33 +39,22 @@
namespace WebKit {
using namespace WebCore;

NetworkNotificationManager::NetworkNotificationManager(NetworkSession& networkSession, const String& webPushMachServiceName)
NetworkNotificationManager::NetworkNotificationManager(NetworkSession& networkSession, const String& webPushMachServiceName, WebPushD::WebPushDaemonConnectionConfiguration&& configuration)
: m_networkSession(networkSession)
{
if (!m_networkSession.sessionID().isEphemeral() && !webPushMachServiceName.isEmpty())
m_connection = makeUnique<WebPushD::Connection>(webPushMachServiceName.utf8(), *this);
}

void NetworkNotificationManager::maybeSendConnectionConfiguration() const
{
if (m_sentConnectionConfiguration)
return;
m_sentConnectionConfiguration = true;

WebPushD::WebPushDaemonConnectionConfiguration configuration;
configuration.useMockBundlesForTesting = m_networkSession.webPushDaemonUsesMockBundlesForTesting();

if (!m_networkSession.sessionID().isEphemeral() && !webPushMachServiceName.isEmpty()) {
#if PLATFORM(COCOA)
auto token = m_networkSession.networkProcess().parentProcessConnection()->getAuditToken();
if (token) {
Vector<uint8_t> auditTokenData;
auditTokenData.resize(sizeof(*token));
memcpy(auditTokenData.data(), &(*token), sizeof(*token));
configuration.hostAppAuditTokenData = WTFMove(auditTokenData);
}
auto token = m_networkSession.networkProcess().parentProcessConnection()->getAuditToken();
if (token) {
Vector<uint8_t> auditTokenData;
auditTokenData.resize(sizeof(*token));
memcpy(auditTokenData.data(), &(*token), sizeof(*token));
configuration.hostAppAuditTokenData = WTFMove(auditTokenData);
}
#endif

sendMessage<WebPushD::MessageType::UpdateConnectionConfiguration>(configuration);
m_connection = makeUnique<WebPushD::Connection>(webPushMachServiceName.utf8(), *this, WTFMove(configuration));
}
}

void NetworkNotificationManager::requestSystemNotificationPermission(const String& originString, CompletionHandler<void(bool)>&& completionHandler)
@@ -210,8 +199,6 @@ void NetworkNotificationManager::sendMessage(Args&&... args) const
{
RELEASE_ASSERT(m_connection);

maybeSendConnectionConfiguration();

Daemon::Encoder encoder;
encoder.encode(std::forward<Args>(args)...);
m_connection->send(messageType, encoder.takeBuffer());
@@ -348,8 +335,6 @@ void NetworkNotificationManager::sendMessageWithReply(CompletionHandler<void(Rep
{
RELEASE_ASSERT(m_connection);

maybeSendConnectionConfiguration();

Daemon::Encoder encoder;
encoder.encode(std::forward<Args>(args)...);
m_connection->sendWithReply(messageType, encoder.takeBuffer(), [completionHandler = WTFMove(completionHandler)] (auto replyBuffer) mutable {
@@ -29,6 +29,7 @@

#include "NotificationManagerMessageHandler.h"
#include "WebPushDaemonConnection.h"
#include "WebPushDaemonConnectionConfiguration.h"
#include "WebPushMessage.h"
#include <WebCore/ExceptionData.h>
#include <WebCore/NotificationDirection.h>
@@ -66,19 +67,16 @@ class NetworkNotificationManager : public NotificationManagerMessageHandler {
void removePushSubscriptionsForOrigin(WebCore::SecurityOriginData&&, CompletionHandler<void(unsigned)>&&);

private:
NetworkNotificationManager(NetworkSession&, const String& webPushMachServiceName);
NetworkNotificationManager(NetworkSession&, const String& webPushMachServiceName, WebPushD::WebPushDaemonConnectionConfiguration&&);

void requestSystemNotificationPermission(const String& originString, CompletionHandler<void(bool)>&&) final;
void showNotification(IPC::Connection&, const WebCore::NotificationData&) final;
void cancelNotification(const UUID& notificationID) final;
void clearNotifications(const Vector<UUID>& notificationIDs) final;
void didDestroyNotification(const UUID& notificationID) final;

void maybeSendConnectionConfiguration() const;

NetworkSession& m_networkSession;
std::unique_ptr<WebPushD::Connection> m_connection;
mutable bool m_sentConnectionConfiguration { false };

template<WebPushD::MessageType messageType, typename... Args>
void sendMessage(Args&&...) const;
@@ -34,9 +34,10 @@

namespace WebKit::WebPushD {

Connection::Connection(CString&& machServiceName, NetworkNotificationManager& manager)
Connection::Connection(CString&& machServiceName, NetworkNotificationManager& manager, WebPushDaemonConnectionConfiguration&& configuration)
: Daemon::ConnectionToMachService<ConnectionTraits>(WTFMove(machServiceName))
, m_notificationManager(manager)
, m_configuration(WTFMove(configuration))
{
LOG(Push, "Creating WebPushD connection to mach service: %s", this->machServiceName().data());
}
@@ -28,6 +28,7 @@
#if ENABLE(BUILT_IN_NOTIFICATIONS)

#include "DaemonConnection.h"
#include "WebPushDaemonConnectionConfiguration.h"
#include "WebPushDaemonConstants.h"

namespace WebKit {
@@ -47,9 +48,10 @@ struct ConnectionTraits {
class Connection : public Daemon::ConnectionToMachService<ConnectionTraits> {
WTF_MAKE_FAST_ALLOCATED;
public:
Connection(CString&& machServiceName, NetworkNotificationManager&);
Connection(CString&& machServiceName, NetworkNotificationManager&, WebPushDaemonConnectionConfiguration&&);

void debugMessage(const String&);
void setConfiguration(WebPushDaemonConnectionConfiguration&&);

private:
void newConnectionWasInitialized() const final;
@@ -62,6 +64,7 @@ class Connection : public Daemon::ConnectionToMachService<ConnectionTraits> {
NetworkSession& networkSession() const;

NetworkNotificationManager& m_notificationManager;
WebPushDaemonConnectionConfiguration m_configuration;

template<MessageType messageType, typename... Args>
void sendMessage(Args&&...) const;
@@ -121,7 +121,6 @@ class NetworkSessionCocoa final : public NetworkSession {
bool fastServerTrustEvaluationEnabled() const { return m_fastServerTrustEvaluationEnabled; }
bool deviceManagementRestrictionsEnabled() const { return m_deviceManagementRestrictionsEnabled; }
bool allLoadsBlockedByDeviceManagementRestrictionsForTesting() const { return m_allLoadsBlockedByDeviceManagementRestrictionsForTesting; }
bool webPushDaemonUsesMockBundlesForTesting() const final { return m_webPushDaemonUsesMockBundlesForTesting; }

DMFWebsitePolicyMonitor *deviceManagementPolicyMonitor();

@@ -188,7 +187,6 @@ class NetworkSessionCocoa final : public NetworkSession {
RetainPtr<DMFWebsitePolicyMonitor> m_deviceManagementPolicyMonitor;
bool m_deviceManagementRestrictionsEnabled { false };
bool m_allLoadsBlockedByDeviceManagementRestrictionsForTesting { false };
bool m_webPushDaemonUsesMockBundlesForTesting { false };
bool m_shouldLogCookieInformation { false };
bool m_fastServerTrustEvaluationEnabled { false };
String m_dataConnectionServiceType;
@@ -1332,7 +1332,6 @@ static void activateSessionCleanup(NetworkSessionCocoa& session, const NetworkSe

m_deviceManagementRestrictionsEnabled = parameters.deviceManagementRestrictionsEnabled;
m_allLoadsBlockedByDeviceManagementRestrictionsForTesting = parameters.allLoadsBlockedByDeviceManagementRestrictionsForTesting;
m_webPushDaemonUsesMockBundlesForTesting = parameters.webPushDaemonConnectionConfiguration.useMockBundlesForTesting;

#if ENABLE(APP_BOUND_DOMAINS)
if (m_resourceLoadStatistics && !parameters.resourceLoadStatisticsParameters.appBoundDomains.isEmpty())
@@ -1,3 +1,21 @@
2022-05-10 Ben Nham <nham@apple.com>

Re-send connection configuration if webpushd dies
https://bugs.webkit.org/show_bug.cgi?id=240286

Reviewed by Geoffrey Garen.

Add a test to make sure that we re-send the connection configuration after webpushd dies.

* TestWebKitAPI/Tests/WebKitCocoa/WebPushDaemon.mm:
(TestWebKitAPI::testWebPushDaemonPList):
(TestWebKitAPI::setUpTestWebPushD):
(TestWebKitAPI::restartTestWebPushD):
(TestWebKitAPI::WebPushDTest::WebPushDTest):
* TestWebKitAPI/cocoa/DaemonTestUtilities.h:
* TestWebKitAPI/cocoa/DaemonTestUtilities.mm:
(TestWebKitAPI::restartService):

2022-05-10 Wenson Hsieh <wenson_hsieh@apple.com>

[Webpage Translation] Avoid removing elements with no children during text manipulation
@@ -96,11 +96,14 @@ - (void)userContentController:(WKUserContentController *)userContentController d
return [currentExecutableDirectory() URLByAppendingPathComponent:@"webpushd" isDirectory:NO];
}

static NSDictionary<NSString *, id> *testWebPushDaemonPList(NSURL *storageLocation)
enum LaunchOnlyOnce : BOOL { No, Yes };

static NSDictionary<NSString *, id> *testWebPushDaemonPList(NSURL *storageLocation, LaunchOnlyOnce launchOnlyOnce)
{
return @{
@"Label" : @"org.webkit.webpushtestdaemon",
@"LaunchOnlyOnce" : @YES,
@"LaunchOnlyOnce" : @(static_cast<BOOL>(launchOnlyOnce)),
@"ThrottleInterval" : @(1),
@"StandardErrorPath" : [storageLocation URLByAppendingPathComponent:@"daemon_stderr"].path,
@"EnvironmentVariables" : @{ @"DYLD_FRAMEWORK_PATH" : currentExecutableDirectory().get().path },
@"MachServices" : @{ @"org.webkit.webpushtestdaemon.service" : @YES },
@@ -126,7 +129,7 @@ static bool shouldSetupWebPushD()
return shouldSetup;
}

static NSURL *setUpTestWebPushD()
static NSURL *setUpTestWebPushD(LaunchOnlyOnce launchOnlyOnce = LaunchOnlyOnce::Yes)
{
if (!shouldSetupWebPushD())
return nil;
@@ -140,11 +143,17 @@ static bool shouldSetupWebPushD()

killFirstInstanceOfDaemon(@"webpushd");

registerPlistWithLaunchD(testWebPushDaemonPList(tempDir), tempDir);
registerPlistWithLaunchD(testWebPushDaemonPList(tempDir, launchOnlyOnce), tempDir);

return tempDir;
}

// Only works if the test daemon was registered with LaunchOnlyOnce::No.
static BOOL restartTestWebPushD()
{
return restartService(@"org.webkit.webpushtestdaemon", @"webpushd");
}

static void cleanUpTestWebPushD(NSURL *tempDir)
{
if (!shouldSetupWebPushD())
@@ -385,11 +394,11 @@ static void clearWebsiteDataStore(WKWebsiteDataStore *store)

class WebPushDTest : public ::testing::Test {
protected:
WebPushDTest()
WebPushDTest(LaunchOnlyOnce launchOnlyOnce = LaunchOnlyOnce::Yes)
{
[WKWebsiteDataStore _allowWebsiteDataRecordsForAllOrigins];

m_tempDirectory = retainPtr(setUpTestWebPushD());
m_tempDirectory = retainPtr(setUpTestWebPushD(launchOnlyOnce));

auto dataStoreConfiguration = adoptNS([_WKWebsiteDataStoreConfiguration new]);
dataStoreConfiguration.get().webPushMachServiceName = @"org.webkit.webpushtestdaemon.service";
@@ -500,6 +509,14 @@ bool hasPushSubscription()
void runTest(NSString *expectedMessage, NSDictionary *pushUserInfo);
};

class WebPushDMultipleLaunchTest : public WebPushDTest {
public:
WebPushDMultipleLaunchTest()
: WebPushDTest(LaunchOnlyOnce::No)
{
}
};

void WebPushDInjectedPushTest::runTest(NSString *expectedMessage, NSDictionary *pushUserInfo)
{
static constexpr auto htmlSource = R"SWRESOURCE(
@@ -1159,6 +1176,59 @@ function getSubscription()
ASSERT_TRUE([message isEqual:[NSNull null]]);
}

TEST_F(WebPushDMultipleLaunchTest, GetPushSubscriptionAfterDaemonRelaunch)
{
static constexpr auto htmlSource = R"HTML(
<script src="/constants.js"></script>
<script>
let postNoteMessage = window.webkit.messageHandlers.note.postMessage.bind(window.webkit.messageHandlers.note);
let getPushManager =
navigator.serviceWorker.register('/sw.js')
.then(() => navigator.serviceWorker.ready)
.then(registration => registration.pushManager);
function getSubscription()
{
getPushManager
.then(pushManager => pushManager.getSubscription())
.then(subscription => subscription ? subscription.toJSON() : null)
.then(postNoteMessage)
.catch(e => postNoteMessage(e.toString()));
}
postNoteMessage('Ready');
</script>
)HTML"_s;

__block RetainPtr<id> message = nil;
__block bool gotMessage = false;
[m_notificationMessageHandler setMessageHandler:^(id receivedMessage) {
message = receivedMessage;
gotMessage = true;
}];

loadRequest(htmlSource, ""_s);
TestWebKitAPI::Util::run(&gotMessage);
ASSERT_TRUE([message isEqualToString:@"Ready"]);

message = nil;
gotMessage = false;
[m_webView evaluateJavaScript:@"getSubscription()" completionHandler:^(id, NSError*) { }];
TestWebKitAPI::Util::run(&gotMessage);
ASSERT_TRUE([message isEqual:[NSNull null]]);

ASSERT_TRUE(restartTestWebPushD());

// Make sure that getSubscription works after killing webpushd. Previously, this didn't work and
// would fail with an AbortError because we didn't re-send the connection configuration after
// the daemon relaunched.
message = nil;
gotMessage = false;
[m_webView evaluateJavaScript:@"getSubscription()" completionHandler:^(id, NSError*) { }];
TestWebKitAPI::Util::run(&gotMessage);
ASSERT_TRUE([message isEqual:[NSNull null]]);
}

} // namespace TestWebKitAPI

#endif // PLATFORM(MAC) || PLATFORM(IOS)
@@ -39,6 +39,8 @@ RetainPtr<NSURL> currentExecutableDirectory();

void killFirstInstanceOfDaemon(NSString *daemonExecutableName);

BOOL restartService(NSString *serviceName, NSString *daemonExecutableName);

void registerPlistWithLaunchD(NSDictionary<NSString *, id> *plist, NSURL *tempDir);

} // namespace TestWebKitAPI

0 comments on commit 18ac12a

Please sign in to comment.