Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#5283 closed change (fixed)

[abp2blocklist] Map $websocket and $webrtc to raw

Reported by: mjethani Assignee: mjethani
Priority: P3 Milestone:
Module: Platform Keywords: abp2blocklist
Cc: sebastian, kzar, arthur, mario Blocked By:
Blocking: #5464 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29452289/
https://codereview.adblockplus.org/29492555/

Description (last modified by mjethani)

Background

Content blocker rules in Safari also apply to WebSocket requests. In the future if and when Safari supports RTCPeerConnection, they will very likely apply to those requests as well. Right now abp2blocklist ignores these types of filters, but it should just map them to the raw type instead, just the way it's doing with XMLHttpRequest.

What to change

Edit filterClasses.js to add WEBSOCKET (128) and WEBRTC (256) to RegExpFilter.typeMap.

In lib/abp2blocklist.js, add WEBSOCKET and WEBRTC to whitelistableRequestTypes. In the getRequestTypes function, map WEBSOCKET and WEBRTC to raw.

In the convertFilterAddRules function, generate rules based on the following criteria:

  1. If the filter contains $websocket, generate a rule with the raw type and the wss?:// URL scheme
  2. If the filter contains $webrtc, generate two rules with the raw type and the stuns?: and turns?: URL schemes
  3. If the filter contains $xmlhttprequest, generate a rule with the raw type and the https?:// URL scheme
  4. However, if the filter contains all three of $websocket, $webrtc, and at least one other option that would map to the raw type (e.g. $xmlhttprequest), ignore 1-3 and instead generate a single rule with the raw type and the [^:]+:(//)? URL scheme
  5. If the filter contains any other types, add the types to the rule if it covers the https?:// URL scheme already, otherwise generate an additional rule with those types

Handle special cases like filters beginning with http://.

Add unit tests.

Change History (13)

comment:1 Changed 3 years ago by sebastian

  • Priority changed from Unknown to P3
  • Ready set

comment:2 Changed 3 years ago by kzar

  • Cc arthur mario added

comment:3 Changed 3 years ago by mjethani

  • Cc arthur mario removed
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:4 Changed 3 years ago by mjethani

  • Cc arthur mario added

comment:5 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5283 - Add support for $websocket and $webrtc

comment:6 Changed 2 years ago by mjethani

  • Description modified (diff)

comment:7 Changed 2 years ago by mjethani

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:8 Changed 2 years ago by mjethani

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm reopening this because we forgot to deal with other raw types like $ping

comment:9 Changed 2 years ago by abpbot

comment:10 Changed 2 years ago by mjethani

  • Review URL(s) modified (diff)

comment:11 Changed 2 years ago by mjethani

  • Description modified (diff)

comment:12 Changed 2 years ago by mjethani

  • Resolution set to fixed
  • Status changed from reopened to closed

comment:13 Changed 2 years ago by mjethani

  • Blocking 5464 added
Note: See TracTickets for help on using tickets.