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

Prevent cookie lists from being selected at the same time #3231

Open
9 tasks done
ghost opened this issue May 5, 2024 · 24 comments
Open
9 tasks done

Prevent cookie lists from being selected at the same time #3231

ghost opened this issue May 5, 2024 · 24 comments

Comments

@ghost
Copy link

ghost commented May 5, 2024

Prerequisites

  • I verified that this is not a filter list issue. Report any issues with filter lists or broken website functionality in the uAssets issue tracker.
  • This is NOT a YouTube, Facebook or Twitch report. These sites MUST be reported by clicking their respective links.
  • This is not a support issue or a question. For support, questions, or help, visit /r/uBlockOrigin.
  • I performed a cursory search of the issue tracker to avoid opening a duplicate issue.
  • The issue is not present after disabling uBO in the browser.
  • I checked the documentation to understand that the issue I am reporting is not normal behavior.

I tried to reproduce the issue when...

  • uBO is the only extension.
  • uBO uses default lists and settings.
  • using a new, unmodified browser profile.

Description

AdGuard/uBO – Cookie Notices and EasyList/uBO – Cookie Notices have filters that reload webpages after changing cookie values.

If they're programmed to set cookies to different values, this may cause an infinite refresh loop.

Consider adding a check to prevent them from being selected at the same time. This would prevent conflicts without having to remove either list from the extension.

If a user has selected EasyList/uBO – Cookie Notices, and then click on AdGuard/uBO – Cookie Notices , the extension will deselect EasyList/uBO – Cookie Notices, and vice-versa.

A specific URL where the issue occurs.

See https://github.com/uBlockOrigin/uAssets/issues/23562

Steps to Reproduce

Expected behavior

Actual behavior

uBO version

1.57.2

Browser name and version

Firefox 125.0.3

Operating System and version

Hannah Montana Linux

@ghost ghost mentioned this issue May 5, 2024
11 tasks
@jackrjli
Copy link

jackrjli commented May 5, 2024

This kind of breakage can occur with any list that uses set-cookie/set-cookie-reload. Making people choose either AdGuard or EasyList Cookie Notices wouldn't solve the issue beyond this particular case, and would get in the way of people with legitimate reasons to use both lists (they don't cover all the same sites).

@u-RraaLL
Copy link
Contributor

u-RraaLL commented May 5, 2024

I wonder if uBO should discard a second filter trying to set the same cookie just as if it were an exact duplicate.

@stephenhawk8054
Copy link
Member

stephenhawk8054 commented May 5, 2024

There were actually other issues prior to the above case with 2 cookie lists conflicted to each other. I agree with the solution of only choosing 1 list for cookie. Other lists can use set-cookie but these 2 setlists are the ones using it the most extensively and can highly likely cause conflictions, especially when this is the only scriptlet with a reload option which can cause more severe issue than other filters.

We don't encourage users to enable more lists to cover different sites any way, and all of the breakages will just be reported to uBO volunteers to solve. Breakage is always a more important issue than lack of sites' coverages. Reporting to respective list maintainers is the correct way to solve the latter.

@u-RraaLL
Copy link
Contributor

u-RraaLL commented May 5, 2024

Agreed. In this particular case, though, the user had a custom AG Annoyance added to his lists (instead of enabled AG Cookie Notices itself), so the issue would've still arisen.

@stephenhawk8054
Copy link
Member

Yes, I think we can just try to cover the most obvious/popular case here since it's impossible to cover all edge cases (even the same list can have same issue if there are some mistakes)

@gorhill
Copy link
Member

gorhill commented May 5, 2024

Consider adding a check to prevent them from being selected at the same time. This would prevent conflicts without having to remove either list from the extension.

This would make it difficult to diagnose filter issues since whichever one takes effect first is undetermined.

@garry-ut99

This comment was marked as abuse.

@krystian3w
Copy link

In the past set-constant was similar breakage: #156 (comment)

@ghost
Copy link
Author

ghost commented May 6, 2024

This would make it difficult to diagnose filter issues since whichever one takes effect first is undetermined.

My proposal is to make it possible to select only one of them in the GUI.

By "without having to remove either list from the extension", I meant "without having to remove them from the list of available lists" (in the other issue, someone hinted that you should remove the AdGuard list from uBlock Origin altogether).

@gorhill
Copy link
Member

gorhill commented Jul 5, 2024

Originally, a long time ago, the AdGuard lists were introduced as stock lists because EasyList lists didn't support extended filtering syntax, limiting itself to only ABP-compatible syntax. It has been a while now that EasyList supports extended syntax, and it might be that AdGuard's lists are no longer needed -- so maybe we should just remove them as stock lists? (Except where there is no alternative)

@dportvine
Copy link

As far as I can see, the problem will only be with the rules for mobile. Rules for them can be added to EasyList and regional lists. If this is a complex case, add it to uBlock filters.

@garry-ut99

This comment was marked as abuse.

@Marko-98
Copy link

Just stumbled upon this issue and was thinking about reporting it. Now I see I'm not the only one...

Recording.2024-07-30.093928.mp4

@krystian3w
Copy link

That is correct, GDPR at this page block load videos: AdguardTeam/AdguardFilters#137106

Therefore, you should check how to unblock the video in EasyList Cookies to eventually finally repair the player in uBo Cookie Notices.

@Marko-98
Copy link

Marko-98 commented Aug 2, 2024

That is correct, GDPR at this page block load videos: AdguardTeam/AdguardFilters#137106

Therefore, you should check how to unblock the video in EasyList Cookies to eventually finally repair the player in uBo Cookie Notices.

Huh, I didn't realize the videos aren't working (I'm not visiting this website often). But I will open an issue on uAssets regarding this.

Edited: Issue reported here.

@D4niloMR
Copy link

D4niloMR commented Dec 6, 2024

Is preventing set-cookie to reload the page more than once in uBO's side feasible? Those endless reload issues can happen even if there is no conflict between ELC/AGC: uBlockOrigin/uAssets#26299

@u-RraaLL
Copy link
Contributor

u-RraaLL commented Dec 6, 2024

It'd likely need to compare cookie values against the filters?

@D4niloMR
Copy link

D4niloMR commented Dec 6, 2024

Yes, this would be best "fixed" in the scriptlet itself, but it's probably not viable/worth the effort.

@gorhill
Copy link
Member

gorhill commented Dec 6, 2024

t'd likely need to compare cookie values against the filters?

It's already done: https://github.com/gorhill/uBlock/blob/1.61.3b4/src/js/resources/cookie.js#L109

If reload is required and the current cookie is the same as the target one, the scriptlet bails out.

@gorhill
Copy link
Member

gorhill commented Dec 6, 2024

Is preventing set-cookie to reload the page more than once in uBO's side feasible?

The scriptlet would need to store something to "remember" it was just executed, and what is stored from page world can be seen by the page, so not a solution.

Since I cannot reproduce the case, I won't be able to analyze what could be done. Signal here if this happens again elsewhere.

@D4niloMR
Copy link

The same issue happened again: https://www.reddit.com/r/uBlockOrigin/comments/1hbzrji/pixiv_breaking_with_uboagain/

AdGuard with pixiv.net##+js(set-cookie-reload, privacy_policy_agreement, 6) and EasyList with pixiv.net##+js(set-cookie, privacy_policy_agreement, 7, , reload, 1)

@stephenhawk8054
Copy link
Member

We actually have dontOverwrite vararg but the problem is the other cookie lists won't use it.

I forgot about this vararg in the substack issue and haven't checked if it could solve the issue without using other filters.

@D4niloMR
Copy link

D4niloMR commented Dec 11, 2024

Yes, then maybe a potential solution is to apply dontOverwrite logic by default when using set-cookie with reload parameters.

@u-RraaLL
Copy link
Contributor

Other lists won't use it or can't use it? What would happen if two list used this vararg?

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

No branches or pull requests

9 participants