IPMatcher: Switch node logic out in favour of pytricia #547
Conversation
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| def _add(self, network): | ||
| try: | ||
| self.trie[preparse(network)] = True | ||
| except ValueError: |
There was a problem hiding this comment.
The _add method swallows ValueError and SystemError exceptions (bypassing error handling) which can hide failures when adding networks.
Details
🔧 How do I fix it?
Remove debugging statements like console.log, debugger, dd(), or logic bypasses like || true. Keep legitimate logging for monitoring and error handling.
More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
| ip = ipaddress.IPv6Address(network) | ||
| if ip.ipv4_mapped: | ||
| return str(ip.ipv4_mapped) | ||
| except ValueError: |
There was a problem hiding this comment.
Empty except ValueError in preparse silently swallows parsing errors for IPv6 addresses
Details
✨ AI Reasoning
1) preparse strips brackets and tries to parse IPv6; the except ValueError is used to ignore parse failures and continue.
2) Silently swallowing ValueError without comment hides unexpected input errors and makes debugging harder.
3) This was introduced in the PR (lines added here) and therefore worsens error handling by adding an empty catch.
🔧 How do I fix it?
Add proper error handling in catch blocks. Log the error, show user feedback, or rethrow if needed.
More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
|
|
||
| def preparse(network: str) -> str: | ||
| # Remove the brackets around IPv6 addresses if they are there. | ||
| network = network.strip("[]") |
There was a problem hiding this comment.
Function parameter 'network' is reassigned with network = network.strip("[]") in preparse; reassigning input parameters reduces clarity.
Details
✨ AI Reasoning
1) The new function preparse(network) strips surrounding brackets and may convert IPv6 mapped addresses to IPv4; it reassigns the parameter 'network' via network = network.strip("[]").
2) Reassigning the function parameter can obscure the original input value and make debugging or future changes harder to follow.
3) This change was introduced in this diff (preparse was added) and thus constitutes a new instance of argument reassignment, so it violates the rule.
🔧 How do I fix it?
Create new local variables instead of reassigning parameters. Use different variable names to clearly distinguish between input and modified values.
More info - Comment @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
This reverts commit 7e31023.
No description provided.