Skip to content

Commit

Permalink
Unescaped escaped commas in regex-based removeparam values
Browse files Browse the repository at this point in the history 
Related discussion:
- uBlockOrigin/uAssets#14683 (comment)
  • Loading branch information
@gorhill
gorhill committed Sep 8, 2022
1 parent 2fc5cd0 commit f806438
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/js/static-net-filtering.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3427,14 +3427,18 @@ class FilterCompiler {
}
}

// https://github.com/uBlockOrigin/uAssets/discussions/14683#discussioncomment-3559284
// If the removeparam value is a regex, unescape escaped commas
extractTokenFromQuerypruneValue() {
const pattern = this.modifyValue;
if ( pattern === '*' || pattern.charCodeAt(0) === 0x7E /* '~' */ ) {
return;
}
const match = /^\/(.+)\/i?$/.exec(pattern);
if ( match !== null ) {
return this.extractTokenFromRegex(match[1]);
return this.extractTokenFromRegex(
match[1].replace(/(\{\d*)\\,/, '$1,')
);
}
if ( pattern.startsWith('|') ) {
return this.extractTokenFromRegex('\\b' + pattern.slice(1));
Expand Down

6 comments on commit f806438

@gwarser
Copy link
Contributor

@gwarser gwarser commented on f806438 Nov 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

"Related discussion" was removed:

Actually there is an issue with a specific filter in there which causes it to be suboptimal:

$removeparam=/^__s=[A-Za-z0-9]{6\,}/,domain=~univis.uni-erlangen.de|~univis.uni-bamberg.de|~univis.uni-luebeck.de|~univis.uni-kiel.de|~univis.th-koeln.de

It's supposed to be tokenizable (s being the token), but uBO detects the regex as invalid because of the \, in the quantifier part of the regex. This needs fixing.

@gorhill
Copy link
Owner Author

@gorhill gorhill commented on f806438 Nov 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Ok sorry, just quite confused here, I am not understanding what is being said here. I see s being used as token with or without the backslash.

@gwarser
Copy link
Contributor

@gwarser gwarser commented on f806438 Nov 9, 2022

Choose a reason for hiding this comment

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

This is quote from you. I forwarded the whole thread to ubo-security@... for you.

@gorhill
Copy link
Owner Author

@gorhill gorhill commented on f806438 Nov 9, 2022

Choose a reason for hiding this comment

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

I am not getting new email for ubo-security@.... Why was the discussion removed?

@gwarser
Copy link
Contributor

@gwarser gwarser commented on f806438 Nov 9, 2022

Choose a reason for hiding this comment

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

It was about performance of $removeparam and proposition to enable some param cleaning lists by default.

I am not getting new email for ubo-security@....

¯\_(ツ)_/¯

I did not get a bounce message.

Why was the discussion removed?

No idea.

@gorhill
Copy link
Owner Author

@gorhill gorhill commented on f806438 Nov 9, 2022

Choose a reason for hiding this comment

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

Ok, just fixed an issue with email's server config, got your email.

Please sign in to comment.