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

Crash when adding scriptlet injection filters with too many arguments #32916

Closed
antonok-edm opened this issue Sep 11, 2023 · 4 comments · Fixed by brave/brave-core#20113
Closed
Assignees
Labels
crash dependencies Pull requests that update a dependency file OS/Android Fixes related to Android browser functionality OS/Desktop QA Pass - Android ARM QA Pass-Win64 QA/Yes release-notes/include

Comments

@antonok-edm
Copy link
Collaborator

See brave/adblock-rust#307 for details. This can be fixed by updating adblock-rust to v0.8.1.

@antonok-edm antonok-edm added crash dependencies Pull requests that update a dependency file OS/Android Fixes related to Android browser functionality OS/Desktop labels Sep 11, 2023
@antonok-edm antonok-edm self-assigned this Sep 11, 2023
@antonok-edm
Copy link
Collaborator Author

we'll want to uplift this one to 1.59.x.

@kjozwiak
Copy link
Member

The above requires 1.59.87 or higher for 1.59.x verification 👍

@MadhaviSeelam
Copy link

MadhaviSeelam commented Sep 15, 2023

Verification PASSED using

Brave | 1.59.87 Chromium: 117.0.5938.62 (Official Build) beta (64-bit)
-- | --
Revision | c26501cb2de033e3e84888e61109f6e90f1f2f75
OS | Windows 11 Version 22H2 (Build 22621.2283)

Reproduced the issue in 1.58.124 Chromium: 117.0.5938.62 using STR/Cases from brave/brave-core#20113 (comment)

example example
image image
  1. Installed 1.59.87
  2. launched Brave
  3. opened brave://settings/shields/filters in a new tab page
  4. Added brave.com##+js(acs, this, probably, is, going, to, break, brave, and, crash, it, instead, of, ignoring, it) to the custom filters
  5. visited https://brave.com/ in a new tab page

Confirmed browser not crashed

Note: Additionally verified the issue with VPN enabled and no crash happened.

example example
image image

@hffvld
Copy link
Contributor

hffvld commented Oct 4, 2023

Verified on Pixel 7 using version(s):

Device/OS: Pixel 7 [panther_beta-user 14 U1B1.230908.003 release-keys]
Brave build: 1.59.109
Chromium: 118.0.5993.21 (Official Build) beta (64-bit) 
Revision: aacb6a6f8bd916bd49f064b2137b007fa6c47b51

STEPS:

  1. Launch Brave
  2. Go to brave://adblock
  3. Add brave.com##+js(acs, this, probably, is, going, to, break, brave, and, crash, it, instead, of, ignoring, it) to Custom Filters
  4. Go to https://brave.com > Verify

ACTUAL RESULTS:

  • Verified that Brave is not crashing when brave.com##+js(acs, this, probably, is, going, to, break, brave, and, crash, it, instead, of, ignoring, it) added to the Custom Filters

Crash on 1.58.135 No crash on 1.59.109
1 2
1 2
2023-10-03_19-40-02.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash dependencies Pull requests that update a dependency file OS/Android Fixes related to Android browser functionality OS/Desktop QA Pass - Android ARM QA Pass-Win64 QA/Yes release-notes/include
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants