Syntax mistake when using extraneous rule modifier marker is not highlighted by the code editor

partingscientist commented 1 year ago

Prerequisites

[X] 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.
[X] This is not a support issue or a question. For support, questions, or help, visit /r/uBlockOrigin.
[X] I performed a cursory search of the issue tracker to avoid opening a duplicate issue.
[X] The issue is not present after disabling uBO in the browser.
[X] I checked the documentation to understand that the issue I am reporting is not normal behavior.

I tried to reproduce the issue when...

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

Description

Per filter syntax documentation by Adguard, a valid filter is represented by the following form.

     rule = ["@@"] pattern [ "$" modifiers ]
modifiers = [modifier0, modifier1[, ...[, modifierN]]]

Filters that use extraneous rule modifier marker ($) are not highlighted as mistakes by uBlock Origin, even though they are invalid filters and discarded internally (at least this is what I presumed happening behind the scene) when parsed.

A specific URL where the issue occurs.

https://example.com https://www.youtube.com/watch?v=fJ9rUzIMcZQ

Steps to Reproduce

Add the following to My Filters list.

||example.com^$doc
||example.com^$doc,badfilter

Open https://example.com. Notice the page's behaviour.
Replace the second filter added in step 1 with the following.
```
||example.com^$doc,$badfilter
```
Notice the code editor's behaviour.
Repeat step 2.
Add the following to My Filters list.
```
fJ9rUzIMcZQ$doc,xhr,domain=youtube.com
```
Open https://www.youtube.com/watch?v=fJ9rUzIMcZQ. Notice the page's behaviour.
Replace the filter added in step 5 with the following.
```
fJ9rUzIMcZQ$doc,xhr,$domain=youtube.com
```
Notice the code editor's behaviour.
Repeat step 6.

Repeat step 7 and 8 with filters

fJ9rUzIMcZQ$doc,$xhr,domain=youtube.com

and

fJ9rUzIMcZQ$doc,$xhr,$domain=youtube.com

Expected behavior

The code editor highlights the syntax mistakes in step 3, 7, and 9, consistent with the fact that those filters are discarded as seen in step 4, 8, and 9.

Actual behavior

The code editor does not highlight the syntax mistakes in step 3, 7, and 9, even though those filters are discarded as seen in step 4, 8, and 9.

uBO version

1.48.5b7

Browser name and version

Firefox 111.0.1

Operating System and version

Windows 10

gorhill commented 1 year ago

fJ9rUzIMcZQ$doc,$xhr,$domain=youtube.com

uBO considers this a valid filter, it's possible to use $ in the pattern. I am pretty sure I have stumbled in the past in patterns using $. At least the syntax highlighter makes it clear how the filter is interpreted.

gorhill commented 1 year ago

If an extraneous $ was considered an error, the following filter:

||example.com/$/$all

Would not work to block:

https://example.com/$/tracker

uBlock-user commented 1 year ago

So ||example.com^$doc,$badfilter is a valid filter ?

gorhill commented 1 year ago

Yes valid, it will cause the filter ||example.com^$doc,$all to be discarded, which filter was meant to block URLs like https://example.com/$doc,/resource.

For demonstration purpose I used all, but your badfilter would discard ||example.com^$doc,$_. That's the main purpose of dummy option _, to signal that the first $ should be interpreted as part of the pattern.

uBlock-user commented 1 year ago

I noticed another difference --

||example.com^$script,$xhr shows 1 out of 1, so what would be the interpretation for this one ?

gorhill commented 1 year ago

what would be the interpretation for this one ?

pattern: ||example.com^$script, options: xhr

I think the syntax highlighter makes it clear.

uBlock-user commented 1 year ago

Seems wiki needs to be updated as per new syntax highlighter then. Also entire $script section seems to have been deleted and removed from Static filter syntax.

gwarser commented 1 year ago

Many appear to be errors, but a few looks deliberate:

113_Tofu Filter.txt:||bbthat.com^$script,$third-party
166_AdGuard Base Filter (AdGuard for Chromium).txt:@@||alkalimetricsink-pa.clients6.google.com/$rpc/google.internal.alkali.applications.metricsink.v1.MetricService/RecordMetrics$domain=matrix.itasoftware.com,stealth
166_AdGuard Base Filter (AdGuard for Chromium).txt:@@||waa-pa.clients6.google.com/$rpc/google.internal.waa.v1.Waa/Create$domain=matrix.itasoftware.com,stealth
176_Better.fyi  Trackers (List).txt:||yimg.com^*_east.swf$domain=yahoo.com$important
227_Acceptable Ads.txt:@@||tapad.com/tapestry/1?ta_partner_id=950&ta_redirect=https://t.myvisualiq.net/sync?prid%3D1001%26ao%3D0%26pruuid%3DTAPAD_${IDS:key}$image,xmlhttprequest
251_Filtros Nauscopicos.txt:||judgeporn.com/video_pop.php?$$image,~image,popup
263_Fanboy's IsraelList.txt:@@||cbsistatic.com/cnwk.1d/ads/$script,$domain=cnettv.cnet.com
374_Adversity - Extreme Measures.txt:||outbrain.com^$third-party$domain=~cnn.com|~damnyouautocorrect.com|~ninemsn.com.au
375_All-in-One Customized Adblock List.txt:||pegel.pw^$important$image
375_All-in-One Customized Adblock List.txt:||yimg.com^*_east.swf$domain=yahoo.com$important
563_List-KR (andromedarabbit).txt:!||tvcast.naver.com/resources/*/clickcr_*.js^$empty - until Adguard browser extensions suppport $empty modifier
565_EasyList - Lite.txt:@@||maxcdn.bootstrapcdn.com^$stylesheet,domain=$stylesheet,domain=shink.me|shon.xyz
1577_AdNauseam Filters.txt:||cdn.krxd.net$domain=www.nbcnews.com^$important
1577_AdNauseam Filters.txt:||tribdss.com^$third-party^$important
1577_AdNauseam Filters.txt:||atpixel.alephd.com/post_adtech$image^$important
2323_phishing-filter.txt:||api-01.moengage.com/v1/emailclick?em=joaquim.brites@sma-europe.eu&amp;&amp;user_id=@$xy*@!hys%c2%b7:%c3%a7%c3%a8z+%c3%98.ll%c2%b8%c2%9c%c3%8a%c3%9a2%c2%8e%c2%ae+%c2%bd%c3%95h%c2%8a%c2%a4a%c3%b3%00.5.&amp;&amp;d=@$xy*@!hn%c2%8e&lt;`f;$or.%c2%97+%c2%87cm&amp;&amp;cid=@$xy*@!h%c2%ba%c2%a7m%c2%9e%c2%9e.$.d%c2%90%c2%bf%c3%aezf.%c3%b9.%c3%b9%c3%b4b%c2%92l%c2%81.%c2%89rxvm%c2%92v(%c2%91%c3%91%00%c3%af.s%c2%a7%c2%86v%c3%a4?%c3%91%c2%9bot%c2%b3j%c2%be%c3%87%c2%acvs.%c3%be%c3%81%c3%91%c2%aaiqd%c3%b8%c3%b3,.+&gt;%c3%88%c3%88%c3%97o!.%c2%aa%c3%a1%.%c2%bf%00.&amp;&amp;ut=l&amp;&amp;moeclickid=61b35f5997223f7c61e6625a_f_t_em_ab_0_p_0_time_2021-12-10+14:09:02.859891_l_0ecli27&amp;&amp;rlink=https://geasia.in/////html/////ssl/rtpfp4b////prajer@fld.czu.cz$all
2323_phishing-filter.txt:||b12700.infura-ipfs.io/ipfs/qmzjfpn4ycq1bsba7sf8m2rxkst15uquja1gxthr49maaz?omet.@%07%5d%26%23128;%04+%ef%bf%bd%14a%ef%bf%bd$%ef%bf%bd%ef%bf%bd4%ef%bf%bd%ef%bf%bd+&_$all
2323_phishing-filter.txt:||ipfs.io/ipfs/qmahloxxjszqlsajppnmtnw7lg4ujpfxddhb8d9yvjm2qk?filename=jay.kyle$pdf.html$all
2323_phishing-filter.txt:||ipfs.io/ipfs/qmyl9od6695zcktwrdi4atc6pjs7u7knpzf4g8fvxwfbui?filename=&&!$jay0.office.link30pdf000.html$all
2362_AdBlockList Allowlist.txt:@@||netflix.com$important^$important
2371_XWHYC Rules.txt:||atm.youku.com/*&p=$f@s@(atm.youku.com/(?!vb).*)&p=\d{1,2}\b@$1&p=0@
2371_XWHYC Rules.txt:||btbit.net/$s@$s@var d=/(UCBrowser|QQBrowser)/i.test@@
2371_XWHYC Rules.txt:||haoweichi.com$s@$('#css').remove();*alert(@@
2371_XWHYC Rules.txt:||ip.cn$s@if($('img#tp').width() > 100@if($('img#tp').width() <10000@
2377_Dandelion Sprout's Nordic filters for tidier websites (for Adblock Plus with ABP inclusion compliance) + EasyList.txt:||maxetise.net^$redirect-rule=noop.js$script,domain=feber.se|tjock.se
2423_phishing-filter (AdGuard).txt:||api-01.moengage.com/v1/emailclick?em=joaquim.brites@sma-europe.eu&amp;&amp;user_id=@$xy*@!hys%c2%b7:%c3%a7%c3%a8z+%c3%98.ll%c2%b8%c2%9c%c3%8a%c3%9a2%c2%8e%c2%ae+%c2%bd%c3%95h%c2%8a%c2%a4a%c3%b3%00.5.&amp;&amp;d=@$xy*@!hn%c2%8e&lt;`f;$or.%c2%97+%c2%87cm&amp;&amp;cid=@$xy*@!h%c2%ba%c2%a7m%c2%9e%c2%9e.$.d%c2%90%c2%bf%c3%aezf.%c3%b9.%c3%b9%c3%b4b%c2%92l%c2%81.%c2%89rxvm%c2%92v(%c2%91%c3%91%00%c3%af.s%c2%a7%c2%86v%c3%a4?%c3%91%c2%9bot%c2%b3j%c2%be%c3%87%c2%acvs.%c3%be%c3%81%c3%91%c2%aaiqd%c3%b8%c3%b3,.+&gt;%c3%88%c3%88%c3%97o!.%c2%aa%c3%a1%.%c2%bf%00.&amp;&amp;ut=l&amp;&amp;moeclickid=61b35f5997223f7c61e6625a_f_t_em_ab_0_p_0_time_2021-12-10+14:09:02.859891_l_0ecli27&amp;&amp;rlink=https://geasia.in/////html/////ssl/rtpfp4b////prajer@fld.czu.cz$all
2423_phishing-filter (AdGuard).txt:||b12700.infura-ipfs.io/ipfs/qmzjfpn4ycq1bsba7sf8m2rxkst15uquja1gxthr49maaz?omet.@%07%5d%26%23128;%04+%ef%bf%bd%14a%ef%bf%bd$%ef%bf%bd%ef%bf%bd4%ef%bf%bd%ef%bf%bd+&_$all
2423_phishing-filter (AdGuard).txt:||ipfs.io/ipfs/qmahloxxjszqlsajppnmtnw7lg4ujpfxddhb8d9yvjm2qk?filename=jay.kyle$pdf.html$all
2423_phishing-filter (AdGuard).txt:||ipfs.io/ipfs/qmyl9od6695zcktwrdi4atc6pjs7u7knpzf4g8fvxwfbui?filename=&&!$jay0.office.link30pdf000.html$all
2424_phishing-filter (Vivaldi).txt:||api-01.moengage.com/v1/emailclick?em=joaquim.brites@sma-europe.eu&amp;&amp;user_id=@$xy*@!hys%c2%b7:%c3%a7%c3%a8z+%c3%98.ll%c2%b8%c2%9c%c3%8a%c3%9a2%c2%8e%c2%ae+%c2%bd%c3%95h%c2%8a%c2%a4a%c3%b3%00.5.&amp;&amp;d=@$xy*@!hn%c2%8e&lt;`f;$or.%c2%97+%c2%87cm&amp;&amp;cid=@$xy*@!h%c2%ba%c2%a7m%c2%9e%c2%9e.$.d%c2%90%c2%bf%c3%aezf.%c3%b9.%c3%b9%c3%b4b%c2%92l%c2%81.%c2%89rxvm%c2%92v(%c2%91%c3%91%00%c3%af.s%c2%a7%c2%86v%c3%a4?%c3%91%c2%9bot%c2%b3j%c2%be%c3%87%c2%acvs.%c3%be%c3%81%c3%91%c2%aaiqd%c3%b8%c3%b3,.+&gt;%c3%88%c3%88%c3%97o!.%c2%aa%c3%a1%.%c2%bf%00.&amp;&amp;ut=l&amp;&amp;moeclickid=61b35f5997223f7c61e6625a_f_t_em_ab_0_p_0_time_2021-12-10+14:09:02.859891_l_0ecli27&amp;&amp;rlink=https://geasia.in/////html/////ssl/rtpfp4b////prajer@fld.czu.cz$document
2424_phishing-filter (Vivaldi).txt:||b12700.infura-ipfs.io/ipfs/qmzjfpn4ycq1bsba7sf8m2rxkst15uquja1gxthr49maaz?omet.@%07%5d%26%23128;%04+%ef%bf%bd%14a%ef%bf%bd$%ef%bf%bd%ef%bf%bd4%ef%bf%bd%ef%bf%bd+&_$document
2424_phishing-filter (Vivaldi).txt:||ipfs.io/ipfs/qmahloxxjszqlsajppnmtnw7lg4ujpfxddhb8d9yvjm2qk?filename=jay.kyle$pdf.html$document
2424_phishing-filter (Vivaldi).txt:||ipfs.io/ipfs/qmyl9od6695zcktwrdi4atc6pjs7u7knpzf4g8fvxwfbui?filename=&&!$jay0.office.link30pdf000.html$document

uBlockOrigin / uBlock-issues