Opened 15 months ago

Last modified 13 months ago

#6622 closed change

Implement rewrite filter option — at Version 11

Reported by: hfiguiere Assignee: hfiguiere
Priority: P2 Milestone: Adblock-Plus-3.2-for-Chrome-Opera-Firefox
Module: Platform Keywords: circumvention
Cc: sebastian, kzar, Ross, mapx, arthur Blocked By: #6592
Blocking: #6242 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29760707/

Description (last modified by kzar)

Background

Sometimes it's desirable to redirect a request, instead of blocking it entirely. Perhaps the website detects when requests fail, or perhaps a request is necessary but its URL has tracking information appended which would be desirable to strip. For this we're adding the $rewrite filter option. In #6592 we introduced the option, and here we're going to make use of it.

Take an example $rewrite filter:

/(server\.com\/content\/.*\.m3u8)\?.*$/$rewrite=$1

A request to this URL:

https://server.com/content/foo.m3u8?userId=12345

Would be redirected to would be redirected to

https://server.com/content/foo.m3u8
  • The syntax for the rewrite option is the same as for Regexp.replace.
  • A request will only be redirected if the rewritten URL is of the same origin, yet different to the original URL.

What to change

  • Update dependencies to have the patch for issue #6592 from core.
    • Issue #6592 - Implement "rewrite" filter option
    • Issue #6649 - Allow u flag in :-abp-contains()
    • Noissue - Update JSDoc @type for Matcher.filterByKeyword
    • Noissue - Use plural for element hiding domains
    • Issue #6562 - Remove filter keys from the element hiding code
    • Noissue - Compare individual characters instead of substring
    • Issue #6610 - Prefer inline style for :-abp-properties()
    • Issue #6437 - Filter out patterns that do not match DOM mutations
    • Issue #6562 - Remove the unused getSelectors function
    • Issue #6562 - Remove the unused provideFilterKeys parameter
  • Implement the rewrite in requestBlocker.js by calling the function for core that will perform it. This is handled as an exception of blocking in the same handler.
  • Update UI to display the "rewritten to" information in the devtools.
  • This will require a dependency update on adblockplusui.
    • TBD

Hint for testers

  • Test that regular request blocking still works.
  • Test filters with rewrite option.
    • Basic rewrite: add filter /(testpages\.adblockplus\.org\/css\/testpages\.css)\?14$/$rewrite=$1?42 and visit https://testpages.adblockplus.org/en/testcases/css/03 . You should notice the rewrite in the Adblock Plus developer tool panel.
    • Rewrite to relative URL. Should succeed. Add filter /testpages\.adblockplus\.org(\/css\/testpages\.css)\?14$/$rewrite=$1?42 (and make sure the previous filter is removed). You should notice the rewrite in the Adblock Plus developer tool panel.
    • Rewrite to different origin: Should fail and the original request let through. Add filter /testpages\.adblockplus\.org(\/css\/testpages\.css)\?14$/$rewrite=bogus.adblockplus.org$1?42 (and make sure the previous filter is removed). You should notice the rewrite in the Adblock Plus developer tool panel, showing that the rewritten URL is the same as the original one.
  • Ensure that the rewrites are logged accordingly into the developer tools.

Change History (11)

comment:1 Changed 15 months ago by hfiguiere

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 15 months ago by hfiguiere

  • Keywords circumvention added

comment:3 Changed 15 months ago by hfiguiere

  • Blocking 6242 added

comment:4 Changed 15 months ago by hfiguiere

  • Description modified (diff)

comment:5 Changed 15 months ago by hfiguiere

  • Description modified (diff)
  • Review URL(s) modified (diff)

comment:6 Changed 15 months ago by greiner

For reference: I've created gitlab#76 to come up with a simple design based on the one provided via the review.

While that's being worked on, we can continue the review with the existing code.

comment:7 Changed 15 months ago by hfiguiere

  • Description modified (diff)

comment:8 Changed 14 months ago by hfiguiere

  • Description modified (diff)

comment:9 Changed 14 months ago by kzar

Please can you expand on the introduction to explain what the idea of the $rewrite option is and to include an example filter with an explanation of what it does?

Please can you improve the "Hints for testers" section? For example you say "Test filters with rewrite option" without giving any examples or saying how they are expected to work.

comment:10 Changed 14 months ago by hfiguiere

  • Description modified (diff)

comment:11 Changed 14 months ago by kzar

  • Cc sebastian kzar Ross added
  • Description modified (diff)
  • Priority changed from Unknown to P2

Thanks for updating the test instructions. I've fleshed out the description a bit more for you and once the dependency update details are in I'll mark this as ready.

Note: See TracTickets for help on using tickets.