Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support $redirect in DNR #1750

Merged
merged 11 commits into from
Aug 8, 2024
Merged

Conversation

seia-soto
Copy link
Member

@seia-soto seia-soto commented Jul 18, 2024

fixes #1714

This PR extracts resources from the ads engine binary on build time and registers into manifest.

TODO

  • Complete base64 encoded binary decoding
  • Convert uBO resource names to adguard format (we use tsurlfilter to convert networkfilters to dnr rules) // done on backend (not sure about the staging)
  • Custom filters remapping

@seia-soto seia-soto marked this pull request as draft July 18, 2024 08:49
@seia-soto seia-soto changed the title Support dnr redirect Support $redirect in DNR Jul 18, 2024
extension-manifest-v3/scripts/build.js Outdated Show resolved Hide resolved
extension-manifest-v3/scripts/build.js Outdated Show resolved Hide resolved
extension-manifest-v3/scripts/build.js Outdated Show resolved Hide resolved
},
{
"resources": [
"rule_resources/statics/*"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be merged with the list above as it is meant to be used on all_frames and with <all_urls>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrmod
Copy link
Member

chrmod commented Aug 1, 2024

@seia-soto please provide test cases for @GRadziejewski

@seia-soto seia-soto marked this pull request as ready for review August 1, 2024 08:35
Generating WAR list is simpler than introducing the glob pattern to copy assets.
@seia-soto
Copy link
Member Author

Hi @GRadziejewski ,

Here's a testing instruction for Ghostery Browser Extension.

Before we start, the purpose of $redirect option in filters is to avoid detection of ad-blocking and website breakage from the request failures. Also, we ship some mocked javascript files not to be detected. For example, we can inject Google Tag Manager shim instead of just redirecting to empty javascript file if the website is coded to require GTM API.

refs https://github.com/gorhill/uBlock/wiki/Static-filter-syntax#redirect

I'm going to use the following website to test if we can make sure if $redirect works:

https://www.iana.org/help/example-domains

using the following filter:

||iana.org/_img/2022/iana-logo-header.svg$redirect=32x32.png
  1. Open the website: https://www.iana.org/help/example-domains
  2. Apply the filter: ||iana.org/_img/2022/iana-logo-header.svg$redirect=32x32.png
  3. Open dev tools then look for iana-logo-header.svg and check if its type is set to / Redirect (depending on the browser, the message can be different)

Unlike the general request blocking, you always should get the response code of 30x redirect then 200 ok. If you see the request is blocked by (blocked:others) or blocked by [extension name] (in Firefox), we regard it as "redirect modifier failure".

@chrmod chrmod added the package CI: create extension packages label Aug 2, 2024
@seia-soto seia-soto marked this pull request as draft August 2, 2024 08:30
@seia-soto
Copy link
Member Author

Converting to draft: We have add a support for custom filters.

@seia-soto
Copy link
Member Author

Pending update from ghostery/urlfilter2dnr#16

Copy link
Collaborator

@smalluban smalluban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The redirect locations don't match DNR rules...

For example, the DoubleClick redirect shows me in the console:
chrome-extension://jccobfekniccmgppgbkonmdmdldhhgjc/rule_resources/redirects/googletagservices-gpt.js

But the real location is extension-manifest-v3/dist/rule_resources/redirects/googletagservices_gpt.js (by the transfer in the build step).

The - from DNR rule is _ in the file system.

@chrmod chrmod marked this pull request as ready for review August 8, 2024 07:56
@chrmod chrmod requested a review from GRadziejewski August 8, 2024 07:58
@chrmod
Copy link
Member

chrmod commented Aug 8, 2024

The redirect resource urls were fixed on the ruleset generation side.

@smalluban smalluban added package CI: create extension packages and removed package CI: create extension packages labels Aug 8, 2024
@chrmod chrmod added package CI: create extension packages and removed package CI: create extension packages labels Aug 8, 2024
@chrmod chrmod merged commit 20eab7f into ghostery:main Aug 8, 2024
5 checks passed
@smalluban smalluban mentioned this pull request Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package CI: create extension packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support $redirect in DNR
4 participants