-
Notifications
You must be signed in to change notification settings - Fork 4
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
Every tested domain is always blocked #20
Comments
webrtc is unsupported. Most things that require a browser context are. Thanks for reporting though because this should not be matching. I'll look into it. Any other examples that don't use unsupported rules? |
Hi @TechnikEmpire - thanks for the quick reply!
Hope this helps! 😊 EDIT: as a temporary workaround, I came up with this code after loading the rules: IReadOnlyList<string> exclusions = new[]
{
"$webrtc",
"$media",
"$websocket",
"$csp",
"|http://$object",
"|https://$object",
".win^$other"
};
var validRules = (
from rule in urlFilters
where !exclusions.Any(start => rule.OriginalRule.StartsWith(start)) &&
rule.Parts.Count > 0
select rule).ToArray(); This seems to be working for now, but of course it's not unstable, as there might very well be other unsupported/glitched rules I haven't noticed for now, that could cause issues with other websites. |
Yeah so there's probably code in the base rule that returns |
I also know that part of my intention with this project was maximum speed, so preprocessing data to ensure that you are feeding valid data was sort of intended to be outside of the scope of the project. I wanted this to be stupid fast, and pausing execution look for errors in the input is considered generally out of scope. But I am concerned with the example of |
Yeah, the This workaround I'm using seems to work fine for now though, so feel free to look into it when you have some time, there's no rush 😊 |
Also found another issue: this URL |
Here is a quick fix for the rules that contain domain: The part of code that handles this domain is the following:
There can still be things that need to be fixed, I will try to discover them in my testing. If you wish, I may make a PR with the fixes. |
Ok guys you've got my attention, I'm going to look at this. I'm really surprised if this is a legit issue because this package has shipped in commercial software and I didn't hear any complaint. However, those people weren't the most capable either so, will let you know. Also any PR's are definitely welcome. |
Waaaaaaw. I found the issue. So a client requested a feature that we be able to match against the What would be really neat is to add a bunch of unit tests that check different kinds of rules to ensure conformance. If any of you get really bored, maybe a PR for unit tests would be really awesome! Thanks again for reporting. |
Fixed by 4e018c3 |
It looks like you deleted a comment. Domains are now handled in the main filter class so should be all good. If you find something new please do open a new issue. |
Hey @TechnikEmpire - thank you for the update, and I'm glad I actually helped you find an issue in the library! I've updated to the newest package and removed my workaround, everything seems to be working fine now! 😊 P.S. |
@Sergio0694 Someone else commented and it only showed up in my email. Anyway thanks again for your help. |
Hello, I'm trying to use this lib to filter HTTP requests in my application, but for some reason I can't get this work as expected, as virtually any
Uri
I try to pass results in at least a rule blocking it.Here's the repro code I'm using:
Running this script results in the domain being blocked by this rule:
Now, that rule shouldn't actually match with the given URL in this case.
EDIT: noticed that this rule in particular was always returning
true
, because theParts
property was empty. Tried filtering out all the rules with that property being empty, but the domain was still always blocked because of some other rules always matching (incorrectly).EDIT 2: also tried using the
easylist.txt
file embedded in this repo (thinking maybe the remote one I was using had been updated with some new, unsupported rules), but the issue is still there as well.Am I missing something here? I might as well be making a very obvious mistake, but I really can't figure out what am I doing wrong.
Thank you for your help, keep up the good work! 😄
Sergio
The text was updated successfully, but these errors were encountered: