HttpClient: check the adblocker inside isUrlBlocked`

src/network/HttpClient.zig

@@ -35,7 +35,6 @@ const Network = @import("Network.zig");
 const Cache = @import("cache/Cache.zig");
 const RobotsGate = @import("RobotsGate.zig");
 const UrlBlocklist = @import("UrlBlocklist.zig");
-const AdBlocker = @import("adblock/AdBlocker.zig");
 
 pub const BlockPattern = UrlBlocklist.Pattern;
 
@@ -187,10 +186,6 @@ obey_robots: bool,
 
 robots: RobotsGate,
 url_blocklist: ?UrlBlocklist,
-/// Hostname dictionaries built from `--adblock-lists`. Owned by Network,
-/// parsed once at startup and read-only from here on, so every client
-/// shares the one copy.
-adblocker: ?*const AdBlocker,
 
 pub fn init(self: *Client, allocator: Allocator, network: *Network, cdp: ?*CDP) !void {
     var handles = try http.Handles.init(network.config);
@@ -230,7 +225,6 @@ pub fn init(self: *Client, allocator: Allocator, network: *Network, cdp: ?*CDP)
         .obey_robots = network.config.obeyRobots(),
         .robots = .{ .allocator = allocator, .network = network },
         .url_blocklist = url_blocklist,
-        .adblocker = if (network.adblocker) |*blocker| blocker else null,
         .arena_pool = &network.app.arena_pool,
     };
 }
@@ -376,15 +370,19 @@ fn clearUrlBlocklist(self: *Client) void {
     }
 }
 
-fn isUrlBlocked(self: *const Client, url: []const u8, internal: bool) bool {
+/// Every reason a request is refused before it reaches the network:
+/// `--block-urls` patterns and the `--adblock-lists` filters both land here
+/// so that no call site can apply one without the other.
+fn isUrlBlocked(self: *const Client, url: [:0]const u8, internal: bool) bool {
     if (internal) return false;
-    const blocklist = self.url_blocklist orelse return false;
-    return blocklist.isBlocked(url);
+    if (self.url_blocklist) |*blocklist| {
+        if (blocklist.isBlocked(url)) return true;
+    }
+    return self.isHostAdblocked(url);
 }
 
-fn isHostAdblocked(self: *const Client, url: [:0]const u8, internal: bool) bool {
-    if (internal) return false;
-    const blocker = self.adblocker orelse return false;
+fn isHostAdblocked(self: *const Client, url: [:0]const u8) bool {
+    const blocker = if (self.network.adblocker) |*b| b else return false;
     const host = URL.getHostname(url);
     if (host.len == 0 or host.len > 253) return false;
     // The trie expects normalized (lowercase) hostnames; URLs aren't
@@ -862,13 +860,9 @@ fn pipeline(self: *Client, transfer: *Transfer, from: SubmitFrom) !void {
         },
         .after_intercept => {
             if (self.isUrlBlocked(transfer.req.url, transfer.req.internal)) {
-                log.warn(.http, "blocked url", .{ .url = transfer.req.url });
+                log.info(.http, "blocked url", .{ .url = transfer.req.url });
                 return transfer.failAsync(error.UrlBlocked);
             }
-            if (self.isHostAdblocked(transfer.req.url, transfer.req.internal)) {
-                log.info(.http, "adblock blocked", .{ .url = transfer.req.url });
-                return transfer.failAsync(error.AdblockBlocked);
-            }
             if (try self.cacheLookup(transfer)) {
                 // response came from the cache, we're done
                 return;
@@ -1508,7 +1502,7 @@ fn processOneMessage(self: *Client, msg: http.Handles.MultiMessage, transfer: *T
                     }
 
                     if (self.isUrlBlocked(transfer.req.url, transfer.req.internal)) {
-                        log.warn(.http, "blocked url", .{ .url = transfer.req.url });
+                        log.info(.http, "blocked url", .{ .url = transfer.req.url });
                         self.removeConn(msg.conn);
                         transfer._conn = null;
                         transfer.failAsync(error.UrlBlocked);
@@ -3241,6 +3235,11 @@ const Synthetic = struct {
 };
 
 const testing = @import("../testing.zig");
+const AdBlocker = @import("adblock/AdBlocker.zig");
+
+// The Network every test client points at: only the fields a test actually
+// exercises are ever set, by initTestClient or by the test itself.
+var test_network: Network = undefined;
 
 test "HttpClient: isFetchInterceptionMethod matches the four Fetch methods" {
     try testing.expect(isFetchInterceptionMethod("Fetch.continueRequest"));
@@ -3411,7 +3410,10 @@ fn initTestClient(client: *Client, pool: *ArenaPool) void {
     client.obey_robots = false;
     client.robots = .{ .allocator = testing.allocator, .network = undefined };
     client.url_blocklist = null;
-    client.adblocker = null;
+    // isUrlBlocked reaches through here for the adblocker; tests that want
+    // one assign it to `client.network` after this returns.
+    test_network.adblocker = null;
+    client.network = &test_network;
 }
 
 test "HttpClient: setBlockedUrls owns, replaces, and clears patterns" {
@@ -3449,15 +3451,16 @@ test "HttpClient: adblock verdicts apply per request hostname" {
         \\@@||good.ads.example.com^
     );
     try blocker.parse(&list);
-    client.adblocker = &blocker;
+    client.network.adblocker = blocker;
+    defer client.network.adblocker = null;
 
-    try testing.expect(client.isHostAdblocked("https://ads.example.com/pixel.gif", false));
+    try testing.expect(client.isUrlBlocked("https://ads.example.com/pixel.gif", false));
     // Hostnames are matched case-insensitively and without the port.
-    try testing.expect(client.isHostAdblocked("https://SUB.ADS.EXAMPLE.COM:8443/x", false));
-    try testing.expect(!client.isHostAdblocked("https://good.ads.example.com/app.js", false));
-    try testing.expect(!client.isHostAdblocked("https://example.com/", false));
+    try testing.expect(client.isUrlBlocked("https://SUB.ADS.EXAMPLE.COM:8443/x", false));
+    try testing.expect(!client.isUrlBlocked("https://good.ads.example.com/app.js", false));
+    try testing.expect(!client.isUrlBlocked("https://example.com/", false));
     // Internal transfers (robots.txt, ...) are never adblocked.
-    try testing.expect(!client.isHostAdblocked("https://ads.example.com/", true));
+    try testing.expect(!client.isUrlBlocked("https://ads.example.com/", true));
 }
 
 test "HttpClient: URL blocking exempts internal transfers" {
@@ -3696,6 +3699,7 @@ test "HttpClient: fulfillIntercepted follows a 3xx redirect" {
     // network.cache and the (empty) connection pool are read on this path.
     var net: Network = undefined;
     net.cache = null;
+    net.adblocker = null;
     // An empty pool makes processTransfer queue the re-issued request
     // instead of putting it on the wire — the queue IS the capture.
     net.available = .{};

src/network/http.zig

@@ -828,7 +828,6 @@ pub const ErrorReason = enum {
     too_large,
     aborted,
     robots_blocked,
-    adblock_blocked,
     other,
 };
 
@@ -856,7 +855,6 @@ pub fn errorReason(err: anyerror) ErrorReason {
         error.SyncWaitInterrupted,
         => .aborted,
         error.RobotsBlocked => .robots_blocked,
-        error.AdblockBlocked => .adblock_blocked,
         else => .other,
     };
 }