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

#brave-domain-block-1pes regressed query filter #32462

Closed
kjozwiak opened this issue Aug 23, 2023 · 4 comments · Fixed by brave/brave-core#19954
Closed

#brave-domain-block-1pes regressed query filter #32462

kjozwiak opened this issue Aug 23, 2023 · 4 comments · Fixed by brave/brave-core#19954

Comments

@kjozwiak
Copy link
Member

Description

Looks like the query filter feature has regressed when we enabled #brave-domain-block-1pes. Using the cases outlined via https://fmarier.github.io/brave-testing/query-filter.html under Redirected same-site tests & Same-site tests, the parameter is not being displayed in the URL nor via Dev Tools. @fmarier added some more context re: what he's seeing via https://bravesoftware.slack.com/archives/C6YNM6Y5S/p1692742963352529.

Steps to Reproduce

  1. install 1.59.39 Chromium: 116.0.5845.96 (or any recent version as this has regressed a while ago)
  2. visit https://fmarier.github.io/brave-testing/query-filter.html
  3. run through any of the tests under either Redirected same-site tests or Same-site tests
  4. you'll notice that the fbclid is not being displayed in those cases even though the test page mentions the following:
In all of these cases, the fbclid parameter should be present on the landing page, but the gclid parameter should be missing in the intermediate step (client-side redirection).

Actual result:

fbclid is not being displayed when running through the Redirected same-site tests or Same-site tests cases.

Expected result:

fbclid should be visible/appearing under the URL bar for the cases under Redirected same-site tests or Same-site tests.

Reproduces how often:

100% reproducible using the STR/Cases mentioned above (@fmarier also reproduced)

Brave version (brave://version info)

Brave | 1.59.39 Chromium: 116.0.5845.96 (Official Build) nightly (64-bit)
-- | --
Revision | 3fafa12c56ece8921f90593a446cde2c1867d02f
OS | Windows 11 Version 22H2 (Build 22621.2134)
Brave | 1.57.49 Chromium: 116.0.5845.96 (Official Build) (64-bit)
-- | --
Revision | 8d78ad7e96630cfcd3ab4165cf0fd421d3c4f068
OS | Windows 11 Version 22H2 (Build 22621.2134)

Version/Channel Information:

  • Can you reproduce this issue with the current release? Yes
  • Can you reproduce this issue with the beta channel? Yes
  • Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? N/A
  • Does the issue resolve itself when disabling Brave Rewards? N/A
  • Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

@fmarier
Copy link
Member

fmarier commented Aug 23, 2023

According to @goodov , the reason for this is that with 1pes enabled, all requests go through adblocker, which since brave/brave-core#15943 also can remove URL params.

Since the removeparam rule is different from our query string filtering policy, we need to ensure that these rules are ignored unless they come from a user-added filterlist.

@fmarier
Copy link
Member

fmarier commented Aug 23, 2023

We do have an issue open to move the query filter rules to a proper list (#10188) and ideally to reuse the implementation in our adblocker, but that is not currently ready due to the behavioral differences.

@kjozwiak
Copy link
Member Author

The above requires 1.58.121 or higher for 1.58.x verification 👍

@LaurenWags
Copy link
Member

LaurenWags commented Sep 12, 2023

Verified with

Brave | 1.58.121 Chromium: 117.0.5938.48 (Official Build) (x86_64)
-- | --
Revision | d80a8110b88df6ae5073d8420fada4168542786f
OS | macOS Version 13.5.2 (Build 22G91)
  1. Test plan from Don't do $removeparam in default blocking mode brave-core#19954 (comment) - PASSED
  • Add ||example.com^$removeparam=test to the custom filters box in brave://settings/shields/filters
  • Visit https://example.com?test=1
  • The URL bar should show https://example.com?test=1 in Default blocking mode and https://example.com in Aggressive mode
brave://settings/shields/filters Default Aggressive
1 2 3
  1. Additional tests from Don't do $removeparam in default blocking mode brave-core#19954 (comment) - PASSED
  1. Direct navigations - PASSED
  • loaded https://brave.com/?fbclid=1 and ensured that the parameters have been removed as per the following:
1
  1. Cross-site tests - PASSED

In all of these cases, the fbclid parameter should be missing from the landing page as per https://fmarier.github.io/brave-testing/query-filter.html.

Cross-origin navigation 301 302 303 307 308
1 2 3 4 5 6
  1. Redirected same-site tests - PASSED

In all of these cases, the fbclid parameter should be present on the landing page, but the gclid parameter should be missing in the intermediate step (client-side redirection) as per https://fmarier.github.io/brave-testing/query-filter.html.

Cross-origin navigation to same-origin checks

301 302 303 307 308
1 2 3 4 5

Cross-origin navigation, then same-origin navigation to same-origin checks

301 302 303 307 308
6 7 8 9 10
  1. Same-site tests - PASSED

In all of these cases, the fbclid parameter should be present on the landing page as per https://fmarier.github.io/brave-testing/query-filter.html.

Same-origin Same-site
1 2
  1. POST tests - PASSED

After clicking on this button, the fbclid parameter should be present on the landing page as per https://fmarier.github.io/brave-testing/query-filter.html.

3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants