Use pytricia for ip matching on non-windows machines #547
Conversation
This reverts commit 7e31023.
… is windows use fallback
bitterpanda63
changed the title
This reverts commit d032fdd.
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
|
|
||
|
|
||
| def preparse(network: str) -> str: | ||
| # Remove the brackets around IPv6 addresses if they are there. |
There was a problem hiding this comment.
The comment on preparse merely restates the code. Either remove it or replace with why this normalization is needed (e.g., to canonicalize bracketed IPv6 and IPv4-mapped IPv6 inputs).
Details
✨ AI Reasoning
A newly added comment adjacent to the preparse function only restates what the code does (removing square brackets). It doesn't explain why this normalization is required (e.g., to handle bracketed IPv6 input or IPv4-mapped IPv6 addresses in callers), so it provides low informational value and increases maintenance burden.
🔧 How do I fix it?
Write comments that explain the purpose, reasoning, or business logic behind the code using words like 'because', 'so that', or 'in order to'.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
|
|
||
| def test_with_ranges(): | ||
| input_list = [ | ||
| "192.168.0.0/24", |
There was a problem hiding this comment.
Maybe good to test with multiple overlapping ranges too?
There was a problem hiding this comment.
these test cases are from node iirc? (also a bit out of scope, since this was existing code copied over)
| matcher = IPMatcher(input_list) | ||
| assert matcher.has("::ffff:0.0.0.0") == True | ||
| assert matcher.has("::ffff:127.0.0.1") == True | ||
| assert matcher.has("::ffff:123") == False |
There was a problem hiding this comment.
so this case is different for both?
There was a problem hiding this comment.
yes, PyTricia is more accurate in this regard, this is also not a huge issue, and I think no one is running with windows in production. Given IPC also doesnt work
Summary by Aikido
🚀 New Features
⚡ Enhancements
🔧 Refactors
More info