Title: [286041] trunk
Revision
286041
Author
achristen...@apple.com
Date
2021-11-18 17:35:35 -0800 (Thu, 18 Nov 2021)

Log Message

Allow all redirect schemes when compiling a content rule list
https://bugs.webkit.org/show_bug.cgi?id=233338

Reviewed by Tim Hatcher.

Source/WebCore:

What was I thinking?  I made an SPI that gives you a set of allowed redirect schemes
from the same source as the JSON which contains those redirect schemes.  This allows
all redirect schemes.  If you want to prevent redirecting to a certain scheme, then
don't compile JSON that redirects to that scheme.  We may want a helper function that
uses the same parser to get the set of schemes being redirected to, but right now that
is not necessary because the JSON is being assembled by a program that has the ability
to check the schemes.

* contentextensions/ContentExtensionActions.cpp:
(WebCore::ContentExtensions::RedirectAction::parse):
(WebCore::ContentExtensions::RedirectAction::URLTransformAction::parse):
* contentextensions/ContentExtensionActions.h:
* contentextensions/ContentExtensionCompiler.cpp:
(WebCore::ContentExtensions::compileRuleList):
* contentextensions/ContentExtensionError.cpp:
(WebCore::ContentExtensions::contentExtensionErrorCategory):
* contentextensions/ContentExtensionError.h:
* contentextensions/ContentExtensionParser.cpp:
(WebCore::ContentExtensions::loadAction):
(WebCore::ContentExtensions::loadRule):
(WebCore::ContentExtensions::loadEncodedRules):
(WebCore::ContentExtensions::parseRuleList):
* contentextensions/ContentExtensionParser.h:

Source/WebKit:

* UIProcess/API/APIContentRuleListStore.cpp:
(API::ContentRuleListStore::lookupContentRuleList):
(API::ContentRuleListStore::compileContentRuleList):
* UIProcess/API/APIContentRuleListStore.h:
* UIProcess/API/C/WKUserContentExtensionStoreRef.cpp:
(WKUserContentExtensionStoreCompile):
* UIProcess/API/Cocoa/WKContentRuleListStore.mm:
(-[WKContentRuleListStore compileContentRuleListForIdentifier:encodedContentRuleList:completionHandler:]):
(-[WKContentRuleListStore _compileContentRuleListForIdentifier:encodedContentRuleList:allowedRedirectSchemes:completionHandler:]): Deleted.
* UIProcess/API/Cocoa/WKContentRuleListStorePrivate.h:
* UIProcess/API/glib/WebKitUserContentFilterStore.cpp:
(webkitUserContentFilterStoreSaveBytes):

Tools:

* TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
(TestWebKitAPI::InMemoryCompiledContentExtension::create):
(TestWebKitAPI::checkCompilerError):
(TestWebKitAPI::TEST_F):
* TestWebKitAPI/Tests/WebKitCocoa/WKContentExtensionStore.mm:
(compileContentRuleList):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (286040 => 286041)


--- trunk/Source/WebCore/ChangeLog	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebCore/ChangeLog	2021-11-19 01:35:35 UTC (rev 286041)
@@ -1,3 +1,34 @@
+2021-11-18  Alex Christensen  <achristen...@webkit.org>
+
+        Allow all redirect schemes when compiling a content rule list
+        https://bugs.webkit.org/show_bug.cgi?id=233338
+
+        Reviewed by Tim Hatcher.
+
+        What was I thinking?  I made an SPI that gives you a set of allowed redirect schemes
+        from the same source as the JSON which contains those redirect schemes.  This allows
+        all redirect schemes.  If you want to prevent redirecting to a certain scheme, then
+        don't compile JSON that redirects to that scheme.  We may want a helper function that
+        uses the same parser to get the set of schemes being redirected to, but right now that
+        is not necessary because the JSON is being assembled by a program that has the ability
+        to check the schemes.
+
+        * contentextensions/ContentExtensionActions.cpp:
+        (WebCore::ContentExtensions::RedirectAction::parse):
+        (WebCore::ContentExtensions::RedirectAction::URLTransformAction::parse):
+        * contentextensions/ContentExtensionActions.h:
+        * contentextensions/ContentExtensionCompiler.cpp:
+        (WebCore::ContentExtensions::compileRuleList):
+        * contentextensions/ContentExtensionError.cpp:
+        (WebCore::ContentExtensions::contentExtensionErrorCategory):
+        * contentextensions/ContentExtensionError.h:
+        * contentextensions/ContentExtensionParser.cpp:
+        (WebCore::ContentExtensions::loadAction):
+        (WebCore::ContentExtensions::loadRule):
+        (WebCore::ContentExtensions::loadEncodedRules):
+        (WebCore::ContentExtensions::parseRuleList):
+        * contentextensions/ContentExtensionParser.h:
+
 2021-11-18  Chris Dumez  <cdu...@apple.com>
 
         Unreviewed build fix.

Modified: trunk/Source/WebCore/contentextensions/ContentExtensionActions.cpp (286040 => 286041)


--- trunk/Source/WebCore/contentextensions/ContentExtensionActions.cpp	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebCore/contentextensions/ContentExtensionActions.cpp	2021-11-19 01:35:35 UTC (rev 286041)
@@ -279,7 +279,7 @@
     return deserializeLength(span, 0);
 }
 
-Expected<RedirectAction, std::error_code> RedirectAction::parse(const JSON::Object& redirectObject, const std::optional<HashSet<String>>& allowedRedirectURLSchemes)
+Expected<RedirectAction, std::error_code> RedirectAction::parse(const JSON::Object& redirectObject)
 {
     auto redirect = redirectObject.getObject("redirect");
     if (!redirect)
@@ -295,7 +295,7 @@
         return RedirectAction { RegexSubstitutionAction { WTFMove(regexSubstitution) } };
 
     if (auto transform = redirect->getObject("transform")) {
-        auto parsedTransform = URLTransformAction::parse(*transform, allowedRedirectURLSchemes);
+        auto parsedTransform = URLTransformAction::parse(*transform);
         if (!parsedTransform)
             return makeUnexpected(parsedTransform.error());
         return RedirectAction { WTFMove(*parsedTransform) };
@@ -305,8 +305,8 @@
         auto url = "" urlString);
         if (!url.isValid())
             return makeUnexpected(ContentExtensionError::JSONRedirectURLInvalid);
-        if (allowedRedirectURLSchemes && !allowedRedirectURLSchemes->contains(url.protocol().toStringWithoutCopying()))
-            return makeUnexpected(ContentExtensionError::JSONRedirectURLSchemeNotAllowed);
+        if (url.protocolIsJavaScript())
+            return makeUnexpected(ContentExtensionError::JSONRedirectToJavaScriptURL);
         return RedirectAction { URLAction { WTFMove(urlString) } };
     }
 
@@ -383,7 +383,7 @@
     }), action);
 }
 
-auto RedirectAction::URLTransformAction::parse(const JSON::Object& transform, const std::optional<HashSet<String>>& allowedRedirectURLSchemes) -> Expected<URLTransformAction, std::error_code>
+auto RedirectAction::URLTransformAction::parse(const JSON::Object& transform) -> Expected<URLTransformAction, std::error_code>
 {
     URLTransformAction action;
     if (auto fragment = transform.getString("fragment"); !!fragment) {
@@ -410,8 +410,8 @@
         auto scheme = WTF::URLParser::maybeCanonicalizeScheme(uncanonicalizedScheme);
         if (!scheme)
             return makeUnexpected(ContentExtensionError::JSONRedirectURLSchemeInvalid);
-        if (allowedRedirectURLSchemes && !allowedRedirectURLSchemes->contains(*scheme))
-            return makeUnexpected(ContentExtensionError::JSONRedirectURLSchemeNotAllowed);
+        if (scheme == "_javascript_")
+            return makeUnexpected(ContentExtensionError::JSONRedirectToJavaScriptURL);
         action.scheme = WTFMove(*scheme);
     }
     action.username = transform.getString("username");

Modified: trunk/Source/WebCore/contentextensions/ContentExtensionActions.h (286040 => 286041)


--- trunk/Source/WebCore/contentextensions/ContentExtensionActions.h	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebCore/contentextensions/ContentExtensionActions.h	2021-11-19 01:35:35 UTC (rev 286041)
@@ -207,7 +207,7 @@
         String username;
 
         uint32_t hash() const { return computeHash(fragment.hash(), host.hash(), password.hash(), path.hash(), port, VariantHasher::hash(queryTransform), scheme.hash(), username.hash()); }
-        static Expected<URLTransformAction, std::error_code> parse(const JSON::Object&, const std::optional<HashSet<String>>&);
+        static Expected<URLTransformAction, std::error_code> parse(const JSON::Object&);
         URLTransformAction isolatedCopy() const;
         bool operator==(const URLTransformAction&) const;
         void serialize(Vector<uint8_t>&) const;
@@ -238,7 +238,7 @@
     bool isDeletedValue() const { return hashTableType == HashTableType::Deleted; }
     uint32_t hash() const { return VariantHasher::hash(action); }
 
-    static Expected<RedirectAction, std::error_code> parse(const JSON::Object&, const std::optional<HashSet<String>>&);
+    static Expected<RedirectAction, std::error_code> parse(const JSON::Object&);
     RedirectAction isolatedCopy() const;
     bool operator==(const RedirectAction&) const;
     void serialize(Vector<uint8_t>&) const;

Modified: trunk/Source/WebCore/contentextensions/ContentExtensionCompiler.cpp (286040 => 286041)


--- trunk/Source/WebCore/contentextensions/ContentExtensionCompiler.cpp	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebCore/contentextensions/ContentExtensionCompiler.cpp	2021-11-19 01:35:35 UTC (rev 286041)
@@ -275,7 +275,7 @@
 {
 #if ASSERT_ENABLED
     callOnMainThread([ruleJSON = ruleJSON.isolatedCopy(), parsedRuleList = parsedRuleList.isolatedCopy()] {
-        ASSERT(parseRuleList(ruleJSON, std::nullopt).value() == parsedRuleList);
+        ASSERT(parseRuleList(ruleJSON).value() == parsedRuleList);
     });
 #endif
 

Modified: trunk/Source/WebCore/contentextensions/ContentExtensionError.cpp (286040 => 286041)


--- trunk/Source/WebCore/contentextensions/ContentExtensionError.cpp	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebCore/contentextensions/ContentExtensionError.cpp	2021-11-19 01:35:35 UTC (rev 286041)
@@ -101,12 +101,10 @@
                 return "Members of the add-or-replace-parameters array must contain a value that is a string";
             case ContentExtensionError::JSONRedirectExtensionPathDoesNotStartWithSlash:
                 return "A redirect extension path must start with a slash";
-            case ContentExtensionError::JSONRedirectURLSchemeNotAllowed:
-                return "A redirect url scheme must be in the set of allowed schemes";
             case ContentExtensionError::JSONRedirectURLSchemeInvalid:
                 return "A redirect url scheme must be a valid scheme";
-            case ContentExtensionError::JSONRedirectURLSchemesShouldNotIncludeJavascript:
-                return "The set of allowed redirect schemes shouldn't include _javascript_";
+            case ContentExtensionError::JSONRedirectToJavaScriptURL:
+                return "A redirect url can't have a scheme of _javascript_";
             case ContentExtensionError::JSONRedirectURLInvalid:
                 return "A redirect url must be valid";
             case ContentExtensionError::JSONRedirectInvalidType:

Modified: trunk/Source/WebCore/contentextensions/ContentExtensionError.h (286040 => 286041)


--- trunk/Source/WebCore/contentextensions/ContentExtensionError.h	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebCore/contentextensions/ContentExtensionError.h	2021-11-19 01:35:35 UTC (rev 286041)
@@ -61,8 +61,7 @@
     JSONRedirectMissing,
     JSONRedirectExtensionPathDoesNotStartWithSlash,
     JSONRedirectURLSchemeInvalid,
-    JSONRedirectURLSchemeNotAllowed,
-    JSONRedirectURLSchemesShouldNotIncludeJavascript,
+    JSONRedirectToJavaScriptURL,
     JSONRedirectURLInvalid,
     JSONRedirectInvalidType,
     JSONRedirectInvalidPort,

Modified: trunk/Source/WebCore/contentextensions/ContentExtensionParser.cpp (286040 => 286041)


--- trunk/Source/WebCore/contentextensions/ContentExtensionParser.cpp	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebCore/contentextensions/ContentExtensionParser.cpp	2021-11-19 01:35:35 UTC (rev 286041)
@@ -179,7 +179,7 @@
     return !!parser.parseSelector(selector);
 }
 
-static std::optional<Expected<Action, std::error_code>> loadAction(const JSON::Object& ruleObject, const std::optional<HashSet<String>>& allowedRedirectURLSchemes)
+static std::optional<Expected<Action, std::error_code>> loadAction(const JSON::Object& ruleObject)
 {
     auto actionObject = ruleObject.getObject("action");
     if (!actionObject)
@@ -210,7 +210,7 @@
         return Action { NotifyAction { { WTFMove(notification) } } };
     }
     if (actionType == "redirect") {
-        auto action = "" allowedRedirectURLSchemes);
+        auto action = ""
         if (!action)
             return makeUnexpected(action.error());
         return Action { RedirectAction { WTFMove(*action) } };
@@ -224,13 +224,13 @@
     return makeUnexpected(ContentExtensionError::JSONInvalidActionType);
 }
 
-static std::optional<Expected<ContentExtensionRule, std::error_code>> loadRule(const JSON::Object& ruleObject, const std::optional<HashSet<String>>& allowedRedirectURLSchemes)
+static std::optional<Expected<ContentExtensionRule, std::error_code>> loadRule(const JSON::Object& ruleObject)
 {
     auto trigger = loadTrigger(ruleObject);
     if (!trigger.has_value())
         return makeUnexpected(trigger.error());
 
-    auto action = "" allowedRedirectURLSchemes);
+    auto action = ""
     if (!action)
         return std::nullopt;
     if (!action->has_value())
@@ -239,11 +239,8 @@
     return { { { WTFMove(trigger.value()), WTFMove(action->value()) } } };
 }
 
-static Expected<Vector<ContentExtensionRule>, std::error_code> loadEncodedRules(const String& ruleJSON, const std::optional<HashSet<String>>& allowedRedirectURLSchemes)
+static Expected<Vector<ContentExtensionRule>, std::error_code> loadEncodedRules(const String& ruleJSON)
 {
-    if (allowedRedirectURLSchemes && allowedRedirectURLSchemes->contains("_javascript_"))
-        return makeUnexpected(ContentExtensionError::JSONRedirectURLSchemesShouldNotIncludeJavascript);
-
     auto decodedRules = JSON::Value::parseJSON(ruleJSON);
 
     if (!decodedRules)
@@ -264,7 +261,7 @@
         if (!ruleObject)
             return makeUnexpected(ContentExtensionError::JSONInvalidRule);
 
-        auto rule = loadRule(*ruleObject, allowedRedirectURLSchemes);
+        auto rule = loadRule(*ruleObject);
         if (!rule)
             continue;
         if (!rule->has_value())
@@ -275,13 +272,13 @@
     return ruleList;
 }
 
-Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(const String& ruleJSON, const std::optional<HashSet<String>>& allowedRedirectURLSchemes)
+Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(const String& ruleJSON)
 {
 #if CONTENT_EXTENSIONS_PERFORMANCE_REPORTING
     MonotonicTime loadExtensionStartTime = MonotonicTime::now();
 #endif
 
-    auto ruleList = loadEncodedRules(ruleJSON, allowedRedirectURLSchemes);
+    auto ruleList = loadEncodedRules(ruleJSON);
 
     if (!ruleList.has_value())
         return makeUnexpected(ruleList.error());

Modified: trunk/Source/WebCore/contentextensions/ContentExtensionParser.h (286040 => 286041)


--- trunk/Source/WebCore/contentextensions/ContentExtensionParser.h	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebCore/contentextensions/ContentExtensionParser.h	2021-11-19 01:35:35 UTC (rev 286041)
@@ -38,12 +38,7 @@
 
 class ContentExtensionRule;
 
-// An allowedRedirectURLSchemes of std::nullopt indicates that all URL schemes are allowed.
-// This is used when recompiling source from older versions of WKContentRuleLists on disk,
-// where we don't have the list of allowed URL schemes but assume that if it compiled
-// successfully the first time then all the schemes used must have been allowed.
-// An allowedRedirectURLSchemes of an empty HashSet indicates that no active actions are allowed.
-WEBCORE_EXPORT Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(const String&, const std::optional<HashSet<String>>& allowedRedirectURLSchemes);
+WEBCORE_EXPORT Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(const String&);
 WEBCORE_EXPORT bool isValidCSSSelector(const String&);
 
 } // namespace ContentExtensions

Modified: trunk/Source/WebKit/ChangeLog (286040 => 286041)


--- trunk/Source/WebKit/ChangeLog	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebKit/ChangeLog	2021-11-19 01:35:35 UTC (rev 286041)
@@ -1,3 +1,23 @@
+2021-11-18  Alex Christensen  <achristen...@webkit.org>
+
+        Allow all redirect schemes when compiling a content rule list
+        https://bugs.webkit.org/show_bug.cgi?id=233338
+
+        Reviewed by Tim Hatcher.
+
+        * UIProcess/API/APIContentRuleListStore.cpp:
+        (API::ContentRuleListStore::lookupContentRuleList):
+        (API::ContentRuleListStore::compileContentRuleList):
+        * UIProcess/API/APIContentRuleListStore.h:
+        * UIProcess/API/C/WKUserContentExtensionStoreRef.cpp:
+        (WKUserContentExtensionStoreCompile):
+        * UIProcess/API/Cocoa/WKContentRuleListStore.mm:
+        (-[WKContentRuleListStore compileContentRuleListForIdentifier:encodedContentRuleList:completionHandler:]):
+        (-[WKContentRuleListStore _compileContentRuleListForIdentifier:encodedContentRuleList:allowedRedirectSchemes:completionHandler:]): Deleted.
+        * UIProcess/API/Cocoa/WKContentRuleListStorePrivate.h:
+        * UIProcess/API/glib/WebKitUserContentFilterStore.cpp:
+        (webkitUserContentFilterStoreSaveBytes):
+
 2021-11-18  Per Arne Vollan <pvol...@apple.com>
 
         [macOS][GPUP] Remove access in sandbox to com.apple.audio.AudioComponentRegistrar

Modified: trunk/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp (286040 => 286041)


--- trunk/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebKit/UIProcess/API/APIContentRuleListStore.cpp	2021-11-19 01:35:35 UTC (rev 286041)
@@ -491,7 +491,7 @@
         if (contentRuleList->metaData.version != ContentRuleListStore::CurrentContentRuleListFileVersion) {
             if (auto sourceFromOldVersion = getContentRuleListSourceFromMappedFile(*contentRuleList); !sourceFromOldVersion.isEmpty()) {
                 RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), sourceFromOldVersion = sourceFromOldVersion.isolatedCopy(), identifier = identifier.isolatedCopy(), completionHandler = WTFMove(completionHandler)] () mutable {
-                    protectedThis->compileContentRuleList(identifier, WTFMove(sourceFromOldVersion), std::nullopt, WTFMove(completionHandler));
+                    protectedThis->compileContentRuleList(identifier, WTFMove(sourceFromOldVersion), WTFMove(completionHandler));
                 });
                 return;
             }
@@ -530,13 +530,13 @@
     });
 }
 
-void ContentRuleListStore::compileContentRuleList(const WTF::String& identifier, WTF::String&& json, std::optional<HashSet<WTF::String>>&& allowedRedirectSchemes, CompletionHandler<void(RefPtr<API::ContentRuleList>, std::error_code)> completionHandler)
+void ContentRuleListStore::compileContentRuleList(const WTF::String& identifier, WTF::String&& json, CompletionHandler<void(RefPtr<API::ContentRuleList>, std::error_code)> completionHandler)
 {
     ASSERT(RunLoop::isMain());
     AtomString::init();
     WebCore::QualifiedName::init();
     
-    auto parsedRules = WebCore::ContentExtensions::parseRuleList(json, WTFMove(allowedRedirectSchemes));
+    auto parsedRules = WebCore::ContentExtensions::parseRuleList(json);
     if (!parsedRules.has_value())
         return completionHandler(nullptr, parsedRules.error());
     

Modified: trunk/Source/WebKit/UIProcess/API/APIContentRuleListStore.h (286040 => 286041)


--- trunk/Source/WebKit/UIProcess/API/APIContentRuleListStore.h	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebKit/UIProcess/API/APIContentRuleListStore.h	2021-11-19 01:35:35 UTC (rev 286041)
@@ -66,7 +66,7 @@
     explicit ContentRuleListStore(const WTF::String& storePath);
     virtual ~ContentRuleListStore();
 
-    void compileContentRuleList(const WTF::String& identifier, WTF::String&& json, std::optional<HashSet<WTF::String>>&& allowedRedirectSchemes, CompletionHandler<void(RefPtr<API::ContentRuleList>, std::error_code)>);
+    void compileContentRuleList(const WTF::String& identifier, WTF::String&& json, CompletionHandler<void(RefPtr<API::ContentRuleList>, std::error_code)>);
     void lookupContentRuleList(const WTF::String& identifier, CompletionHandler<void(RefPtr<API::ContentRuleList>, std::error_code)>);
     void removeContentRuleList(const WTF::String& identifier, CompletionHandler<void(std::error_code)>);
     void getAvailableContentRuleListIdentifiers(CompletionHandler<void(WTF::Vector<WTF::String>)>);

Modified: trunk/Source/WebKit/UIProcess/API/C/WKUserContentExtensionStoreRef.cpp (286040 => 286041)


--- trunk/Source/WebKit/UIProcess/API/C/WKUserContentExtensionStoreRef.cpp	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebKit/UIProcess/API/C/WKUserContentExtensionStoreRef.cpp	2021-11-19 01:35:35 UTC (rev 286041)
@@ -76,7 +76,7 @@
 void WKUserContentExtensionStoreCompile(WKUserContentExtensionStoreRef store, WKStringRef identifier, WKStringRef jsonSource, void* context, WKUserContentExtensionStoreFunction callback)
 {
 #if ENABLE(CONTENT_EXTENSIONS)
-    toImpl(store)->compileContentRuleList(toWTFString(identifier), toWTFString(jsonSource), { HashSet<String>() }, [context, callback](RefPtr<API::ContentRuleList> contentRuleList, std::error_code error) {
+    toImpl(store)->compileContentRuleList(toWTFString(identifier), toWTFString(jsonSource), [context, callback](RefPtr<API::ContentRuleList> contentRuleList, std::error_code error) {
         callback(error ? nullptr : toAPI(contentRuleList.leakRef()), toResult(error), context);
     });
 #else

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStore.mm (286040 => 286041)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStore.mm	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStore.mm	2021-11-19 01:35:35 UTC (rev 286041)
@@ -87,7 +87,18 @@
 
 - (void)compileContentRuleListForIdentifier:(NSString *)identifier encodedContentRuleList:(NSString *)encodedContentRuleList completionHandler:(void (^)(WKContentRuleList *, NSError *))completionHandler
 {
-    [self _compileContentRuleListForIdentifier:identifier encodedContentRuleList:encodedContentRuleList allowedRedirectSchemes:nil completionHandler:completionHandler];
+#if ENABLE(CONTENT_EXTENSIONS)
+    _contentRuleListStore->compileContentRuleList(identifier, encodedContentRuleList, [completionHandler = makeBlockPtr(completionHandler)](RefPtr<API::ContentRuleList> contentRuleList, std::error_code error) {
+        if (error) {
+            auto userInfo = @{ NSHelpAnchorErrorKey: [NSString stringWithFormat:@"Rule list compilation failed: %s", error.message().c_str()] };
+
+            // error.value() could have a specific compiler error that is not equal to WKErrorContentRuleListStoreCompileFailed.
+            // We want to use error.message, but here we want to only pass on CompileFailed with userInfo from the std::error_code.
+            return completionHandler(nil, [NSError errorWithDomain:WKErrorDomain code:WKErrorContentRuleListStoreCompileFailed userInfo:userInfo]);
+        }
+        completionHandler(wrapper(*contentRuleList), nil);
+    });
+#endif
 }
 
 - (void)lookUpContentRuleListForIdentifier:(NSString *)identifier completionHandler:(void (^)(WKContentRuleList *, NSError *))completionHandler
@@ -171,25 +182,6 @@
 #endif
 }
 
-- (void)_compileContentRuleListForIdentifier:(NSString *)identifier encodedContentRuleList:(NSString *)encodedContentRuleList allowedRedirectSchemes:(NSSet<NSString *> *)schemes completionHandler:(void (^)(WKContentRuleList *, NSError *))completionHandler
-{
-#if ENABLE(CONTENT_EXTENSIONS)
-    HashSet<String> allowedRedirectSchemes;
-    for (NSString *scheme in schemes)
-        allowedRedirectSchemes.add(scheme);
-    _contentRuleListStore->compileContentRuleList(identifier, encodedContentRuleList, WTFMove(allowedRedirectSchemes), [completionHandler = makeBlockPtr(completionHandler)](RefPtr<API::ContentRuleList> contentRuleList, std::error_code error) {
-        if (error) {
-            auto userInfo = @{NSHelpAnchorErrorKey: [NSString stringWithFormat:@"Rule list compilation failed: %s", error.message().c_str()]};
-
-            // error.value() could have a specific compiler error that is not equal to WKErrorContentRuleListStoreCompileFailed.
-            // We want to use error.message, but here we want to only pass on CompileFailed with userInfo from the std::error_code.
-            return completionHandler(nil, [NSError errorWithDomain:WKErrorDomain code:WKErrorContentRuleListStoreCompileFailed userInfo:userInfo]);
-        }
-        completionHandler(wrapper(*contentRuleList), nil);
-    });
-#endif
-}
-
 + (instancetype)defaultStoreWithLegacyFilename
 {
 #if ENABLE(CONTENT_EXTENSIONS)

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStorePrivate.h (286040 => 286041)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStorePrivate.h	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKContentRuleListStorePrivate.h	2021-11-19 01:35:35 UTC (rev 286041)
@@ -32,8 +32,6 @@
 - (void)_invalidateContentRuleListVersionForIdentifier:(NSString *)identifier;
 - (void)_getContentRuleListSourceForIdentifier:(NSString *)identifier completionHandler:(void (^)(NSString *))completionHandler;
 
-- (void)_compileContentRuleListForIdentifier:(NSString *)identifier encodedContentRuleList:(NSString *)encodedContentRuleList allowedRedirectSchemes:(NSSet<NSString *> *)schemes completionHandler:(void (^)(WKContentRuleList *, NSError *))completionHandler;
-
 // FIXME: Remove _WKUserContentExtensionStore.
 + (instancetype)defaultStoreWithLegacyFilename;
 + (instancetype)storeWithURLAndLegacyFilename:(NSURL *)url;

Modified: trunk/Source/WebKit/UIProcess/API/glib/WebKitUserContentFilterStore.cpp (286040 => 286041)


--- trunk/Source/WebKit/UIProcess/API/glib/WebKitUserContentFilterStore.cpp	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Source/WebKit/UIProcess/API/glib/WebKitUserContentFilterStore.cpp	2021-11-19 01:35:35 UTC (rev 286041)
@@ -187,7 +187,7 @@
     }
 
     auto* store = WEBKIT_USER_CONTENT_FILTER_STORE(g_task_get_source_object(task.get()));
-    store->priv->store->compileContentRuleList(identifier, String::fromUTF8(sourceData, sourceSize), { HashSet<String>() }, [task = WTFMove(task)](RefPtr<API::ContentRuleList> contentRuleList, std::error_code error) {
+    store->priv->store->compileContentRuleList(identifier, String::fromUTF8(sourceData, sourceSize), [task = WTFMove(task)](RefPtr<API::ContentRuleList> contentRuleList, std::error_code error) {
         if (g_task_return_error_if_cancelled(task.get()))
             return;
 

Modified: trunk/Tools/ChangeLog (286040 => 286041)


--- trunk/Tools/ChangeLog	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Tools/ChangeLog	2021-11-19 01:35:35 UTC (rev 286041)
@@ -1,3 +1,17 @@
+2021-11-18  Alex Christensen  <achristen...@webkit.org>
+
+        Allow all redirect schemes when compiling a content rule list
+        https://bugs.webkit.org/show_bug.cgi?id=233338
+
+        Reviewed by Tim Hatcher.
+
+        * TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
+        (TestWebKitAPI::InMemoryCompiledContentExtension::create):
+        (TestWebKitAPI::checkCompilerError):
+        (TestWebKitAPI::TEST_F):
+        * TestWebKitAPI/Tests/WebKitCocoa/WKContentExtensionStore.mm:
+        (compileContentRuleList):
+
 2021-11-18  Robert Jenner  <jen...@apple.com>
 
         [ Monterey ] TestWebKitAPI.WebpagePreferences.WebsitePoliciesDuringRedirect is a constant timeout

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp (286040 => 286041)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp	2021-11-19 01:35:35 UTC (rev 286041)
@@ -131,7 +131,7 @@
     {
         CompiledContentExtensionData extensionData;
         InMemoryContentExtensionCompilationClient client(extensionData);
-        auto parsedRules = ContentExtensions::parseRuleList(filter, std::nullopt);
+        auto parsedRules = ContentExtensions::parseRuleList(filter);
         auto compilerError = ContentExtensions::compileRuleList(client, WTFMove(filter), WTFMove(parsedRules.value()));
 
         // Compiling should always succeed here. We have other tests for compile failures.
@@ -1362,11 +1362,11 @@
     EXPECT_EQ(1ul, createNFAs(combinedURLFilters).size());
 }
 
-void checkCompilerError(const char* json, std::error_code expectedError, const std::optional<HashSet<String>>& allowedRedirectURLSchemes = std::nullopt)
+void checkCompilerError(const char* json, std::error_code expectedError)
 {
     CompiledContentExtensionData extensionData;
     InMemoryContentExtensionCompilationClient client(extensionData);
-    auto parsedRules = ContentExtensions::parseRuleList(json, allowedRedirectURLSchemes);
+    auto parsedRules = ContentExtensions::parseRuleList(json);
     std::error_code compilerError;
     if (parsedRules.has_value())
         compilerError = ContentExtensions::compileRuleList(client, json, WTFMove(parsedRules.value()));
@@ -1532,7 +1532,6 @@
         ContentExtensionError::JSONInvalidActionType);
     checkCompilerError("[{\"action\":{\"type\":\"css-display-none\"},\"trigger\":{\"url-filter\":\"webkit.org\"}}]",
         ContentExtensionError::JSONInvalidCSSDisplayNoneActionType);
-    checkCompilerError("", ContentExtensionError::JSONRedirectURLSchemesShouldNotIncludeJavascript, { { "_javascript_" } });
 
     checkCompilerError("[{\"action\":{\"type\":\"ignore-previous-rules\"},\"trigger\":{\"url-filter\":\"webkit.org\"}},"
         "{\"action\":{\"type\":\"css-display-none\",\"selector\":\".hidden\"},\"trigger\":{\"url-filter\":\".*\"}}]", { });
@@ -1561,10 +1560,8 @@
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"query-transform\":{\"add-or-replace-parameters\":[{}]}}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONAddOrReplaceParametersKeyValueMissingKeyString);
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"query-transform\":{\"add-or-replace-parameters\":[{\"key\":5}]}}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONAddOrReplaceParametersKeyValueMissingKeyString);
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"query-transform\":{\"add-or-replace-parameters\":[{\"key\":\"k\",\"value\":5}]}}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONAddOrReplaceParametersKeyValueMissingValueString);
-    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"url\":\"about:blank\"}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectURLSchemeNotAllowed, { { "not-about" } });
-    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"url\":\"about:blank\"}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", { }, { { "about" } });
-    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"url\":\"https://127..1/\"}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectURLInvalid, { { "https" } });
-    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"url\":\"about:blank\"}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", { }, { { "about" } });
+    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"url\":\"https://127..1/\"}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectURLInvalid);
+    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"url\":\"_javascript_:dostuff\"}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectToJavaScriptURL);
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"port\":\"not a number\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectInvalidPort);
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"port\":\"65536\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectInvalidPort);
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"query\":\"no-question-mark\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectInvalidQuery);
@@ -1572,9 +1569,9 @@
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"port\":\"65535\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", { });
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"port\":\"\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", { });
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"extension-path\":\"/does/start/with/slash/\"}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", { });
-    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"scheme\":\"about\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectURLSchemeNotAllowed, { { "not-about" } });
     checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"scheme\":\"!@#$%\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectURLSchemeInvalid);
-    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"scheme\":\"About\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", { }, { { "about" } });
+    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"scheme\":\"_javascript_\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONRedirectToJavaScriptURL);
+    checkCompilerError("[{\"action\":{\"type\":\"redirect\",\"redirect\":{\"transform\":{\"scheme\":\"About\"}}},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", { });
     checkCompilerError("[{\"action\":{\"type\":\"modify-headers\",\"request-headers\":5},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONModifyHeadersNotArray);
     checkCompilerError("[{\"action\":{\"type\":\"modify-headers\",\"request-headers\":[5]},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONModifyHeadersInfoNotADictionary);
     checkCompilerError("[{\"action\":{\"type\":\"modify-headers\",\"request-headers\":[{}]},\"trigger\":{\"url-filter\":\"webkit.org\"}}]", ContentExtensionError::JSONModifyHeadersMissingOperation);

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKContentExtensionStore.mm (286040 => 286041)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKContentExtensionStore.mm	2021-11-19 00:51:46 UTC (rev 286040)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKContentExtensionStore.mm	2021-11-19 01:35:35 UTC (rev 286041)
@@ -484,7 +484,7 @@
     [[WKContentRuleListStore defaultStore] _removeAllContentRuleLists];
 
     __block RetainPtr<WKContentRuleList> list;
-    [[WKContentRuleListStore defaultStore] _compileContentRuleListForIdentifier:@"testidentifier" encodedContentRuleList:[NSString stringWithFormat:@"%s", json] allowedRedirectSchemes:[NSSet setWithObjects:@"testscheme", @"othertestscheme", nil] completionHandler:^(WKContentRuleList *filter, NSError *error) {
+    [[WKContentRuleListStore defaultStore] compileContentRuleListForIdentifier:@"testidentifier" encodedContentRuleList:[NSString stringWithFormat:@"%s", json] completionHandler:^(WKContentRuleList *filter, NSError *error) {
         EXPECT_NULL(error);
         list = filter;
     }];
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to