Title: [278995] trunk/Source/WebKit
- Revision
- 278995
- Author
- [email protected]
- Date
- 2021-06-17 10:17:36 -0700 (Thu, 17 Jun 2021)
Log Message
Fix crashes in ContentRuleListStore::lookupContentRuleList
https://bugs.webkit.org/show_bug.cgi?id=227100
<rdar://78816611>
Reviewed by Chris Dumez.
To be extra careful, instead of just assuming that moveFile will always succeed,
when moveFile fails, try to delete the legacy path (which has a ContentExtension- instead of ContentRuleList- prefix)
and fail the lookup. This will only happen in Safari, which was the only client of _WKUserContentExtensionStore.
The legacy file is not useful to us because in r275078 I incremented CurrentContentRuleListFileVersion so the bytes on disk
need to be recompiled anyways. Safari already has logic to recompile it. This was just an attempt to use what we have and
not leave anything behind.
Also, to be extra careful, try deleting a file before moving a file on top of it.
* UIProcess/API/APIContentRuleListStore.cpp:
compiledToFile:
(API::ContentRuleListStore::lookupContentRuleList):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (278994 => 278995)
--- trunk/Source/WebKit/ChangeLog 2021-06-17 17:16:54 UTC (rev 278994)
+++ trunk/Source/WebKit/ChangeLog 2021-06-17 17:17:36 UTC (rev 278995)
@@ -1,5 +1,26 @@
2021-06-17 Alex Christensen <[email protected]>
+ Fix crashes in ContentRuleListStore::lookupContentRuleList
+ https://bugs.webkit.org/show_bug.cgi?id=227100
+ <rdar://78816611>
+
+ Reviewed by Chris Dumez.
+
+ To be extra careful, instead of just assuming that moveFile will always succeed,
+ when moveFile fails, try to delete the legacy path (which has a ContentExtension- instead of ContentRuleList- prefix)
+ and fail the lookup. This will only happen in Safari, which was the only client of _WKUserContentExtensionStore.
+ The legacy file is not useful to us because in r275078 I incremented CurrentContentRuleListFileVersion so the bytes on disk
+ need to be recompiled anyways. Safari already has logic to recompile it. This was just an attempt to use what we have and
+ not leave anything behind.
+
+ Also, to be extra careful, try deleting a file before moving a file on top of it.
+
+ * UIProcess/API/APIContentRuleListStore.cpp:
+ compiledToFile:
+ (API::ContentRuleListStore::lookupContentRuleList):
+
+2021-06-17 Alex Christensen <[email protected]>
+
WKScriptMessageHandlerWithReply should raise an exception if replyHandler is called twice
https://bugs.webkit.org/show_bug.cgi?id=226863
Modified: trunk/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp (278994 => 278995)
--- trunk/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp 2021-06-17 17:16:54 UTC (rev 278994)
+++ trunk/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp 2021-06-17 17:17:36 UTC (rev 278995)
@@ -388,6 +388,10 @@
return makeUnexpected(ContentRuleListStore::Error::CompileFailed);
}
+ // Try and delete any files at the destination instead of overwriting them
+ // in case there is already a file there and it is mmapped.
+ deleteFile(finalFilePath);
+
if (!moveFile(temporaryFilePath, finalFilePath)) {
WTFLogAlways("Content Rule List compiling failed: Moving file failed.");
return makeUnexpected(ContentRuleListStore::Error::CompileFailed);
@@ -461,8 +465,20 @@
m_readQueue->dispatch([protectedThis = makeRef(*this), identifier = identifier.isolatedCopy(), storePath = m_storePath.isolatedCopy(), completionHandler = WTFMove(completionHandler)]() mutable {
auto path = constructedPath(storePath, identifier, false);
auto legacyPath = constructedPath(storePath, identifier, true);
- if (fileExists(legacyPath))
- moveFile(legacyPath, path);
+ if (fileExists(legacyPath)) {
+ // Try and delete any files at the destination instead of overwriting them
+ // in case there is already a file there and it is mmapped.
+ deleteFile(path);
+ if (!moveFile(legacyPath, path)) {
+ WTFLogAlways("Content Rule List lookup failed: Moving a legacy file failed.");
+ if (!deleteFile(legacyPath))
+ WTFLogAlways("Content Rule List lookup failed: Deleting a legacy file failed.");
+ RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), completionHandler = WTFMove(completionHandler)] () mutable {
+ completionHandler(nullptr, Error::LookupFailed);
+ });
+ return;
+ }
+ }
auto contentRuleList = openAndMapContentRuleList(path);
if (!contentRuleList) {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
