Title: [263545] trunk/Source/WebKit
- Revision
- 263545
- Author
- [email protected]
- Date
- 2020-06-25 16:43:38 -0700 (Thu, 25 Jun 2020)
Log Message
[iOS] Network process is crashing when launching TJMaxx app due to invalid NetworkProcess::DestroySession IPC message
https://bugs.webkit.org/show_bug.cgi?id=213625
<rdar://problem/64737890>
Reviewed by Geoffrey Garen.
The app is calling [WKWebsiteDataStore init] despite the method being marked as unavailable in
WKWebsiteDataStore.h. As a result, they end up with a WKWebsiteDataStore object whose internal
_websiteDataStore is bad because its constructor was never called. When [WKWebsiteDataStore dealloc]
gets called later own, it calls the ~WebsiteDataStore() destructor for _websiteDataStore but its
m_sessionID is 0 because we never called the constructor. This causes us to send a
NetworkProcess::DestroySession IPC with a sessionID that is 0, which is not valid so the
NetworkProcess crashes.
To address the issue, we now provide an implementation of [WKWebsiteDataStore init] which returns nil
behind a linked-on-after check (since this crashes the app). To keep the app working,
[WKWebsiteDataStore init] returns the default data store until rebuilt with the new SDK. Note that
I tried returning a new ephemeral data store instead but this was getting the app in a bad state.
* UIProcess/API/Cocoa/WKWebsiteDataStore.h:
Mark "new" as unavailable, otherwise [WKWebsiteDataStore new] builds.
* UIProcess/API/Cocoa/WKWebsiteDataStore.mm:
(-[WKWebsiteDataStore init]):
Return nil with latest SDK, the default data store otherwise.
* UIProcess/Cocoa/VersionChecks.h:
Add linked-on-after check.
* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::~WebsiteDataStore):
Add a release assertion to make sure that m_sessionID is always valid when the destructor is called.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (263544 => 263545)
--- trunk/Source/WebKit/ChangeLog 2020-06-25 23:30:46 UTC (rev 263544)
+++ trunk/Source/WebKit/ChangeLog 2020-06-25 23:43:38 UTC (rev 263545)
@@ -1,3 +1,38 @@
+2020-06-25 Chris Dumez <[email protected]>
+
+ [iOS] Network process is crashing when launching TJMaxx app due to invalid NetworkProcess::DestroySession IPC message
+ https://bugs.webkit.org/show_bug.cgi?id=213625
+ <rdar://problem/64737890>
+
+ Reviewed by Geoffrey Garen.
+
+ The app is calling [WKWebsiteDataStore init] despite the method being marked as unavailable in
+ WKWebsiteDataStore.h. As a result, they end up with a WKWebsiteDataStore object whose internal
+ _websiteDataStore is bad because its constructor was never called. When [WKWebsiteDataStore dealloc]
+ gets called later own, it calls the ~WebsiteDataStore() destructor for _websiteDataStore but its
+ m_sessionID is 0 because we never called the constructor. This causes us to send a
+ NetworkProcess::DestroySession IPC with a sessionID that is 0, which is not valid so the
+ NetworkProcess crashes.
+
+ To address the issue, we now provide an implementation of [WKWebsiteDataStore init] which returns nil
+ behind a linked-on-after check (since this crashes the app). To keep the app working,
+ [WKWebsiteDataStore init] returns the default data store until rebuilt with the new SDK. Note that
+ I tried returning a new ephemeral data store instead but this was getting the app in a bad state.
+
+ * UIProcess/API/Cocoa/WKWebsiteDataStore.h:
+ Mark "new" as unavailable, otherwise [WKWebsiteDataStore new] builds.
+
+ * UIProcess/API/Cocoa/WKWebsiteDataStore.mm:
+ (-[WKWebsiteDataStore init]):
+ Return nil with latest SDK, the default data store otherwise.
+
+ * UIProcess/Cocoa/VersionChecks.h:
+ Add linked-on-after check.
+
+ * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+ (WebKit::WebsiteDataStore::~WebsiteDataStore):
+ Add a release assertion to make sure that m_sessionID is always valid when the destructor is called.
+
2020-06-25 Daniel Bates <[email protected]>
[iOS] -_requestTextInputContextsInRect cannot find empty Quip spreadsheet title
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.h (263544 => 263545)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.h 2020-06-25 23:30:46 UTC (rev 263544)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.h 2020-06-25 23:43:38 UTC (rev 263545)
@@ -47,6 +47,7 @@
*/
+ (WKWebsiteDataStore *)nonPersistentDataStore;
+- (instancetype)new NS_UNAVAILABLE;
- (instancetype)init NS_UNAVAILABLE;
/*! @abstract Whether the data store is persistent or not. */
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm (263544 => 263545)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm 2020-06-25 23:30:46 UTC (rev 263544)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm 2020-06-25 23:43:38 UTC (rev 263545)
@@ -30,6 +30,7 @@
#import "AuthenticationChallengeDispositionCocoa.h"
#import "CompletionHandlerCallChecker.h"
#import "ShouldGrandfatherStatistics.h"
+#import "VersionChecks.h"
#import "WKHTTPCookieStoreInternal.h"
#import "WKNSArray.h"
#import "WKNSURLAuthenticationChallenge.h"
@@ -115,6 +116,17 @@
return wrapper(WebKit::WebsiteDataStore::createNonPersistent());
}
+- (instancetype)init
+{
+ // This is a workaround for apps that were managing to call [WKWebsiteDataStore init].
+ // FIXME: We should eventually drop this and always return nil.
+ if (!WebKit::linkedOnOrAfter(WebKit::SDKVersion::FirstWithWKWebsiteDataStoreInitReturningNil)) {
+ RELEASE_LOG_ERROR(Process, "Application is calling [WKWebsiteDataStore init], which is not supported");
+ return [WKWebsiteDataStore defaultDataStore];
+ }
+ return nil;
+}
+
- (void)dealloc
{
_websiteDataStore->WebKit::WebsiteDataStore::~WebsiteDataStore();
Modified: trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h (263544 => 263545)
--- trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h 2020-06-25 23:30:46 UTC (rev 263544)
+++ trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h 2020-06-25 23:43:38 UTC (rev 263545)
@@ -94,6 +94,7 @@
FirstWithSessionCleanupByDefault = DYLD_IOS_VERSION_FIRST_WITH_SESSION_CLEANUP_BY_DEFAULT,
FirstThatSendsNativeMouseEvents = DYLD_IOS_VERSION_13_4,
FirstWithInitializeWebKit2MainThreadAssertion = DYLD_IOS_VERSION_14_0,
+ FirstWithWKWebsiteDataStoreInitReturningNil = DYLD_IOS_VERSION_14_0,
#elif PLATFORM(MAC)
FirstWithNetworkCache = DYLD_MACOSX_VERSION_10_11,
FirstWithExceptionsForDuplicateCompletionHandlerCalls = DYLD_MACOSX_VERSION_10_13,
@@ -107,6 +108,7 @@
FirstThatRestrictsBaseURLSchemes = DYLD_MACOSX_VERSION_10_15_4,
FirstWithSessionCleanupByDefault = DYLD_MACOS_VERSION_FIRST_WITH_SESSION_CLEANUP_BY_DEFAULT,
FirstWithInitializeWebKit2MainThreadAssertion = DYLD_MACOSX_VERSION_10_16,
+ FirstWithWKWebsiteDataStoreInitReturningNil = DYLD_MACOSX_VERSION_10_16,
#endif
};
Modified: trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp (263544 => 263545)
--- trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2020-06-25 23:30:46 UTC (rev 263544)
+++ trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp 2020-06-25 23:43:38 UTC (rev 263545)
@@ -124,6 +124,7 @@
WebsiteDataStore::~WebsiteDataStore()
{
ASSERT(RunLoop::isMain());
+ RELEASE_ASSERT(m_sessionID.isValid());
platformDestroy();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
