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

Increase the numerical limit for allowed values in set-cookie #3016

Open
9 tasks done
peace2000 opened this issue Dec 10, 2023 · 11 comments
Open
9 tasks done

Increase the numerical limit for allowed values in set-cookie #3016

peace2000 opened this issue Dec 10, 2023 · 11 comments
Labels
enhancement New feature or request

Comments

@peace2000
Copy link
Member

peace2000 commented Dec 10, 2023

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 an issue with YouTube, Facebook or Twitch.
  • 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

I propose increasing the numerical limit of set-cookie scriptlet.

I suggest the new limit to be 111.

Reasoning:

This page: https://launer.com/ uses values from 100 - 111 to define what cookie settings are accepted.
(It also uses value 000 to say that no cookies are accepted but that value is not reasonable for us anyway...)

A specific URL where the issue occurs.

https://launer.com/

Steps to Reproduce

  1. Add launer.com##+js(set-cookie, cookie_consent, 100) or launer.com##+js(trusted-set-cookie, cookie_consent, 111)

Expected behavior

Those values would be accepted by the scriptlet.

Actual behavior

They are not accepted.

uBO version

1.54.1b8

Browser name and version

FF 120.0.1

Operating System and version

Win 10

@gwarser
Copy link
Member

gwarser commented Dec 10, 2023

Looks like bitfield, any time more digits can be added.

@peace2000
Copy link
Member Author

peace2000 commented Dec 10, 2023

set-local-storage-item accepts values up to 32767 https://github.com/gorhill/uBlock/wiki/Resources-Library#set-local-storage-itemjs- ... is there a reason to keep the value as low as 15 for set-cookie?

Looks like bitfield, any time more digits can be added.

True, one way to handle this could be to allow a limitless sequence of 0 / 1 values but so far gorhill has wanted actual cases as a justification to add more values.

@gorhill
Copy link
Member

gorhill commented Dec 10, 2023

I implemented set-cookie according to AdGuard's documentation.

Now I am facing an open ended stream of requests to constantly add to it. I think at this point this should involve AdGuard too, there might be good reasons these limits were put in place. cc @ameshkov

@garry-ut99
Copy link

garry-ut99 commented Dec 10, 2023

@peace2000
Copy link
Member Author

peace2000 commented Dec 10, 2023

Here is the reason :

* [Why can't I set the cookie value to a number greater than 15? AdguardTeam/Scriptlets#121](https://github.com/AdguardTeam/Scriptlets/issues/121)

it's dangerous to provide an option to set an arbitrary cookie, so we made set-cookie scriptlet similar to set-constant with possible values which where the most used in filter lists AdguardTeam/Scriptlets#79

No reason was stated why values above 15 are too arbitrary for set-cookie but not arbitrary for set-local-storage-item. The only reason for 15 seems to be cover "mostly used" values. Though that 32767 is still unclear as AG doesn't seem to use that high values in their set-local-storage-item filters... Unless I missed them. :)

@ameshkov
Copy link

ameshkov commented Dec 11, 2023

@gorhill
I suggest keeping scriptlets consistent and having the allowed constants similar in all three: set-constant / set-cookie / set-local-storage-item. Linked issue in our repo: AdguardTeam/Scriptlets#388

@ryanbr
Copy link

ryanbr commented Dec 11, 2023

Since its binary values, maybe just cover 3-4 digit binarys? Not ideal but an option

@ryanbr
Copy link

ryanbr commented Dec 11, 2023

Another option to improve set-local/session/cookie, is for these values to be shared. Since we have a few set-cookie options, why not just cover them all.

@uBlock-user uBlock-user added the enhancement New feature or request label Jan 26, 2024
@slavaleleka
Copy link

we have decided to increase the limit up to 32767 for AdGuard's cookie setting scriptlets

gorhill added a commit to gorhill/uBlock that referenced this issue Mar 15, 2024
@krystian3w
Copy link

So simple set-constant sill will support small range 0-15?

@slavaleleka
Copy link

@krystian3w what are you talking about? set-constant has always supported positive decimal integer <= 32767

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

No branches or pull requests

9 participants