Opened 8 months ago

Closed 7 months ago

Last modified 5 months ago

#6592 closed change (fixed)

Add filter option to rewrite URL

Reported by: hfiguiere Assignee: hfiguiere
Priority: P2 Milestone:
Module: Core Keywords: circumvention
Cc: arthur, mapx, sebastian, kzar, amrmak Blocked By:
Blocking: #6622, #6626, #6673 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29760704/

Description (last modified by hfiguiere)

Background

Instead of blocking content we could rewrite the URL to still get content.

Like:

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

There are multiple use cases for it.

  • Video ads: the ads are queried through the .m3u8 file used as source. See issue #6242
  • AMP: we could redirect people to the non AMP page as AMP is only meant to advertise and track not to actually make the web better.

What to change

  • Add a rewrite= option to blocking filters. The rewrite option value is the same as the replace argument of String.prototype.replace() as when it is called with a regexp.
  • Implement the rewrite function in the BlockingFilter class to be used by the webext.
  • Add tests

Note

  • The WebExt side of the feature is handled by issue #6622

Attachments (1)

testext.zip (824 bytes) - added by sebastian 8 months ago.

Download all attachments as: .zip

Change History (34)

comment:1 Changed 8 months ago by hfiguiere

  • Description modified (diff)

comment:2 Changed 8 months ago by arthur

  • Cc arthur added

comment:3 Changed 8 months ago by mapx

  • Cc mapx added

comment:4 Changed 8 months ago by hfiguiere

  • Cc sebastian kzar added

Changed 8 months ago by sebastian

comment:5 Changed 8 months ago by sebastian

So with this filter you would essentially strip the query string from the .m3u8 file requests. I created a simple test extension that does exactly that, in order to verify the approach. But then the videos in #6242 don't play at all.

comment:6 Changed 8 months ago by hfiguiere

I had already verified the approach do work before suggesting it.

comment:7 Changed 8 months ago by sebastian

Then tell me why the attached extension doesn't have the desired effect?

comment:8 Changed 8 months ago by hfiguiere

I limit the URL I rewrite. Because these .m3u8 point to other *.m3u8 with query parameters that are needed.

with this it works:

browser.webRequest.onBeforeRequest.addListener(details =>
{
  let match = details.url.match(/(^http.*\/.*\.m3u8)\?/);
  if (match)
  {
    let url = match[1];
    console.log(`${details.url} retwritten to ${url}`);
    return {redirectUrl: url};
  }
}, {urls: ["*://content.uplynk.com/ext/*"]}, ["blocking"]);
Last edited 8 months ago by hfiguiere (previous) (diff)

comment:9 Changed 8 months ago by sebastian

If I use your code, I still see the the pre-roll (on Chrome).

comment:10 Changed 8 months ago by hfiguiere

You need also a blocking filter. (both are needed, not sure why)

||content.uplynk.com/preplay/*

This will block queries to a json.

comment:11 Changed 8 months ago by sebastian

Ok, thanks. With the urls: ["*://content.uplynk.com/ext/*"] restriction in the test extension and the ||content.uplynk.com/preplay/* filter in Adblock Plus, the pre-roll indeed gets blocked while the video still plays. This demonstrates a case for a new filter options like this.

Last edited 8 months ago by sebastian (previous) (diff)

comment:12 Changed 8 months ago by sebastian

What is about security? Without further restrictions a malicious filter list could redirect arbitrary requests to arbitrary unrelated destinations. Perhaps it is enough to make sure that the destination of the redirect has the same origin as the original URL.

As for the naming, I might prefer to call this filter option $redirect (rather than $rewrite).

comment:13 Changed 8 months ago by mapx

I guess is much better $replace or $rewrite - $redirect was already proposed to redirect requests to some neutered locals (is used already by uBo)

comment:14 Changed 8 months ago by sebastian

FWIW, the $redirect filter option in uBlock is similar to what we might implement here. $redirect=1x1-transparent.gif causes an internal redirect to a data: URL that decodes to a transparent image (in uBlock). While the $redirect (or $rewrite) filter option here, causes an internal redirect to the given URL (after applying any regular expression substitution).

We have to make sure that its a valid (and safe, see above) URL anyway, hence we would just ignore filters that were meant for uBlock. And the same way around uBlock will just ignore filter option values that include a URL / regexp substitution where it expects an identifier for their assets.

Eventually, uBlock and/or Adblock Plus could support both flavors with the same filter option; if its a known identifier redirecting to the respective data: URL, if it results into a valid URL (after regular expression substitution) redirecting to that URL.

But I could also see arguments to have two seperate filter options for those. Though having $replace (or $rewrite) for redirects to a remote destination, while $redirect means a redirect to a known asset encoded as data: URL (which technically is less a redirect), seems inconsistent.

Last edited 8 months ago by sebastian (previous) (diff)

comment:15 Changed 8 months ago by uplynk

Much better call the filter option $uplynk

comment:16 Changed 8 months ago by amrmak

  • Cc amrmak added

comment:17 Changed 8 months ago by hfiguiere

$redirect isn't what we are doing here. $rewrite is IMHO appropriate since it is like URL rewriting in HTTP servers. Also this leave $redirect open to the proper implementation later.

As for security we can enforce a same origin by disallowing rewriting to a different domain.

comment:18 Changed 8 months ago by sebastian

Fair enough. Sounds good.

comment:19 Changed 8 months ago by hfiguiere

  • Blocking 6622 added

comment:20 Changed 8 months ago by hfiguiere

  • Description modified (diff)

comment:21 Changed 8 months ago by hfiguiere

  • Owner set to hfiguiere

comment:22 Changed 8 months ago by hfiguiere

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

comment:23 Changed 8 months ago by hfiguiere

  • Description modified (diff)

comment:24 Changed 8 months ago by hfiguiere

  • Blocking 6626 added

comment:25 Changed 7 months ago by hfiguiere

  • Description modified (diff)

comment:26 Changed 7 months ago by hfiguiere

  • Description modified (diff)

comment:27 Changed 7 months ago by hfiguiere

  • Blocking 6673 added

comment:28 Changed 7 months ago by abpbot

A commit referencing this issue has landed:
Issue 6592 - Implement $rewrite filter option

comment:29 Changed 7 months ago by hfiguiere

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:30 Changed 7 months ago by kzar

  • Milestone Adblock-Plus-for-Chrome-Opera-Firefox-next deleted
  • Priority changed from Unknown to P2
  • Ready set

(That milestone is just for Platform issues.)

comment:31 follow-up: Changed 7 months ago by SMed79

It seems that there is a different privilege between blocking and rewriting option if the host is first or third party. Test the different combinations you find _here_ (archive).

Is this known?

comment:32 in reply to: ↑ 31 Changed 7 months ago by hfiguiere

Replying to SMed79:

It seems that there is a different privilege between blocking and rewriting option if the host is first or third party. Test the different combinations you find _here_ (archive).

Is this known?

There is no different privilege. What I see is that you have two filters that match, one blocking, one rewriting and that is known to be an undefined behaviour, i.e. the order of evaluation is undefined.

comment:33 Changed 5 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

This is implemented and seems to be working as expected.

ABP 3.1.0.2069
Chrome 67 / 64 / 49 / Windows 7
Firefox 60 / 55 / 51 / Windows 7
Opera 52 / 45 / 38 / Windows 7

Note: See TracTickets for help on using tickets.