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
Cisco IOS: do not crash on malformed pool #6568
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6568 +/- ##
============================================
- Coverage 73.40% 73.39% -0.01%
- Complexity 35715 35719 +4
============================================
Files 2837 2837
Lines 144146 144177 +31
Branches 17427 17431 +4
============================================
+ Hits 105806 105824 +18
- Misses 29958 29969 +11
- Partials 8382 8384 +2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @progwriter)
projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoIosDynamicNat.java, line 245 at r1 (raw file):
if (pool == null) { return when(FALSE); }
This addition isn't necessary; isMalformed
already checked whether the specified pool is defined, so if it reaches this point then it must be.
projects/batfish/src/test/resources/org/batfish/grammar/cisco/testconfigs/ios-nat-malformed-pool, line 10 at r1 (raw file):
permit 1.1.1.1 0.0.0.255 ! Note the pool is not a valid range ip nat pool SNOOKER 10.161.151.222 10.161.151.30 netmask 255.255.255.0
could choose some less obscure IPs lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all files reviewed, all discussions resolved
projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoIosDynamicNat.java, line 245 at r1 (raw file):
Previously, corinaminer (Corina Miner) wrote…
This addition isn't necessary;
isMalformed
already checked whether the specified pool is defined, so if it reaches this point then it must be.
ah, cool, thanks.
projects/batfish/src/test/resources/org/batfish/grammar/cisco/testconfigs/ios-nat-malformed-pool, line 10 at r1 (raw file):
Previously, corinaminer (Corina Miner) wrote…
could choose some less obscure IPs lol
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved
In some cases (like anonymized configs) we were crashing the whole node conversion on malformed NAT pools. This change simply skips pool creation, and warns that the pool is malformed. The transformation referencing the pool will not match.