brave / ab2cb Goto Github PK

• The separator (^) regex is not reliable as it uses escaped ansi characters and a pipe (|) for logical-or case, which is not supported by iOS regex.
• URL Schemes are hard coded to “https?” which may lead to leak of non https/http url.
• Currently the data structure is not good for a merge of similar rules
• Unless there is a conflict in unless-domain and if-domain, all domains should be prefixed with ‘*’ to allow/block their subdomains also.
• if-domain and unless-domain are mutually exclusive keys. This case is not handled and leads to errors.
• A rule has a scheme it is not handled and is treated as part of domain.
• A regex for domain is not present in any rule. This leaks many urls.
• The new keys in Adblock Plus; generic hide and generic block are not handled.
• The option ‘~third-party’ is not handled to add first-party in load -type
• None of the domains are puny-coded

We ran into an issue today where the following rule:
||api.twitter.com^$third-party,domain=~tweetdeck.com|~twitter.com|~twitter.jp

was added with brave/adblock-lists@eb7a15d

This specific rule is supposed to get converted to:

  {
    "trigger": {
      "url-filter": "^[^:]+:(//)?([^/]+.)?api\\.twitter\\.com",
      "load-type": [
        "third-party"
      ],
      "unless-domain": [
        "*.tweetdeck.com", <-- the * here are the relevant parts
        "*.twitter.com",
        "*.twitter.jp"
      ]
    },
    "action": {
      "type": "block"
    }
  },

However, it wasn't. It's getting converted to this (missing the wildcard):

{
    "trigger": {
      "url-filter": "^[^:]+:(//)?([^/]+.)?api\\.twitter\\.com",
      "load-type": [
        "third-party"
      ],
      "unless-domain": [
        "tweetdeck.com",
        "twitter.com",
        "twitter.jp"
      ]
    },
    "action": {
      "type": "block"
    }
  },

We need to fix this behavior - would be great to have a unit test capturing this. @kylehickinson mentioned there is an additional complexity in that we can't have both if-domain and unless-domain in the same rule. We should scan for occurrences of that happening and handle appropriately