Skip to content

Web Extension Bug: allowAllRequests DNR Rule Broken in Safari Tech Preview #47081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 24, 2025
Merged

Web Extension Bug: allowAllRequests DNR Rule Broken in Safari Tech Preview #47081

merged 1 commit into from
Jun 24, 2025
+76 −40

Conversation

elijahsawyers
Copy link

@elijahsawyers elijahsawyers commented Jun 23, 2025 •
edited by webkit-early-warning-system
Loading

9ea53e5

Web Extension Bug: allowAllRequests DNR Rule Broken in Safari Tech Preview
https://bugs.webkit.org/show_bug.cgi?id=294099
rdar://152746422

Reviewed by Brian Weinstein and Timothy Hatcher.

This patch is working towards fixing allowAllRequests rules, which no longer
work in STP. The changes in this commit focus on fixing main_frame resource type
allowAllRequests rules because it's a straightforward fix with existing WebKit
content blocker functionality; however, fixing sub_frame allowAllRequests rules
will likely be more involved and require changes to WebKit content blockers.

To fix this for main_frame allowAllRequests rules, we can just change the
if-frame-url trigger to if-top-url, and drop the use of load context triggers.

Wrote new tests to validate the fix.

Left FIXME comments in the code for sub_frame allowAllRequests rules:
rdar://154124673

* Source/WebKit/UIProcess/Extensions/Cocoa/_WKWebExtensionDeclarativeNetRequestRule.mm:
(-[_WKWebExtensionDeclarativeNetRequestRule _webKitRuleWithWebKitActionType:chromeActionType:condition:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIDeclarativeNetRequest.mm:
(TestWebKitAPI::TEST(WKWebExtensionAPIDeclarativeNetRequest, MainFrameAllowAllRequests)):
(TestWebKitAPI::TEST(WKWebExtensionAPIDeclarativeNetRequest, RuleConversionWithMainFrameAllowAllRequests)):
(TestWebKitAPI::TEST(WKWebExtensionAPIDeclarativeNetRequest, RuleConversionWithAllowAllRequests)): Deleted.

Canonical link: https://commits.webkit.org/296565@main

dcdad66

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@elijahsawyers elijahsawyers self-assigned this Jun 23, 2025
@elijahsawyers elijahsawyers added the WebKit Extensions Bugs related to extension support. label Jun 23, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 23, 2025 •
edited
Loading

EWS run on current version of this PR (hash dcdad66)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@elijahsawyers elijahsawyers added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 24, 2025
@elijahsawyers
Web Extension Bug: allowAllRequests DNR Rule Broken in Safari Tech Pr…
9ea53e5 
…eview

https://bugs.webkit.org/show_bug.cgi?id=294099
rdar://152746422

Reviewed by Brian Weinstein and Timothy Hatcher.

This patch is working towards fixing allowAllRequests rules, which no longer
work in STP. The changes in this commit focus on fixing main_frame resource type
allowAllRequests rules because it's a straightforward fix with existing WebKit
content blocker functionality; however, fixing sub_frame allowAllRequests rules
will likely be more involved and require changes to WebKit content blockers.

To fix this for main_frame allowAllRequests rules, we can just change the
if-frame-url trigger to if-top-url, and drop the use of load context triggers.

Wrote new tests to validate the fix.

Left FIXME comments in the code for sub_frame allowAllRequests rules:
rdar://154124673

* Source/WebKit/UIProcess/Extensions/Cocoa/_WKWebExtensionDeclarativeNetRequestRule.mm:
(-[_WKWebExtensionDeclarativeNetRequestRule _webKitRuleWithWebKitActionType:chromeActionType:condition:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIDeclarativeNetRequest.mm:
(TestWebKitAPI::TEST(WKWebExtensionAPIDeclarativeNetRequest, MainFrameAllowAllRequests)):
(TestWebKitAPI::TEST(WKWebExtensionAPIDeclarativeNetRequest, RuleConversionWithMainFrameAllowAllRequests)):
(TestWebKitAPI::TEST(WKWebExtensionAPIDeclarativeNetRequest, RuleConversionWithAllowAllRequests)): Deleted.

Canonical link: https://commits.webkit.org/296565@main
@webkit-commit-queue
Copy link
Collaborator

Committed 296565@main (9ea53e5): https://commits.webkit.org/296565@main

Reviewed commits have been landed. Closing PR #47081 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 9ea53e5 into WebKit:main Jun 24, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xeenon xeenon xeenon approved these changes

@b-weinstein b-weinstein b-weinstein approved these changes

Assignees

@elijahsawyers elijahsawyers

Labels
WebKit Extensions Bugs related to extension support.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@elijahsawyers @webkit-early-warning-system @webkit-commit-queue @xeenon @b-weinstein