-
Notifications
You must be signed in to change notification settings - Fork 48
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
Add within support to allow iterating over IpNetworks (cidrs) #50
Closed
Closed
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
fe8dffc
within function that walks and debug prints in place
ross 4b5b461
Working iterator, but only info, no block
ross 6f091bb
WitninItem, ip_net & info
ross b629400
within example
ross baa607f
Flip type param order
ross 32ec66d
Cleaner way to hint to compiler about the iterator item type
ross 5087e6a
Use PhantomData to handle unused param T
ross 4d025e2
Make node_count a Within property
ross 9af2487
rustfmt within work
ross b4d0122
General error handling for within bits
ross e39f695
Error handling while iterating
ross 6edc84c
add back deny that got cut somehow
ross d42fedd
within example
ross 5c30778
Add example within doc/usage
ross caf577a
within example fixes
ross 300d4ba
bytes_and_prefix_to_net detect v4 in v6, within city test
ross 2858822
test_within_broken_database
ross 69fc352
remove within test prints
ross File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
use ipnetwork::IpNetwork; | ||
use maxminddb::{geoip2, Within}; | ||
|
||
fn main() -> Result<(), String> { | ||
let mut args = std::env::args().skip(1); | ||
let reader = maxminddb::Reader::open_readfile( | ||
args.next() | ||
.ok_or("First argument must be the path to the IP database")?, | ||
) | ||
.unwrap(); | ||
let cidr: String = args | ||
.next() | ||
.ok_or("Second argument must be the IP address and mask in CIDR notation, e.g. 0.0.0.0/0 or ::/0")? | ||
.parse() | ||
.unwrap(); | ||
let ip_net = if cidr.contains(":") { | ||
IpNetwork::V6(cidr.parse().unwrap()) | ||
} else { | ||
IpNetwork::V4(cidr.parse().unwrap()) | ||
}; | ||
|
||
let mut n = 0; | ||
let mut iter: Within<geoip2::City, _> = reader.within(ip_net).map_err(|e| e.to_string())?; | ||
while let Some(next) = iter.next() { | ||
let item = next.map_err(|e| e.to_string())?; | ||
let continent = item.info.continent.and_then(|c| c.code).unwrap_or(""); | ||
let country = item.info.country.and_then(|c| c.iso_code).unwrap_or(""); | ||
let city = match item.info.city.and_then(|c| c.names) { | ||
Some(names) => names.get("en").unwrap_or(&""), | ||
None => "", | ||
}; | ||
if !city.is_empty() { | ||
println!("{} {}-{}-{}", item.ip_net, continent, country, city); | ||
} else if !country.is_empty() { | ||
println!("{} {}-{}", item.ip_net, continent, country); | ||
} else if !continent.is_empty() { | ||
println!("{} {}", item.ip_net, continent); | ||
} | ||
n += 1; | ||
} | ||
eprintln!("processed {} items", n); | ||
|
||
Ok(()) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Fairly new to rust (in case it's not obvious) and I'm not finding a clean way to go about returning errors that happen during the walking of nodes. My searching for idiomatic patterns didn't turn anything up, but it's a tough thing to search for.
This results in some pretty ugly code below to map errors in the helper functions into
Some(Err(e))
. That seems bad enough, but currently have have things inmatch
Ok
/Err
conditionals that make it tough to read. Things like.map_err
won't let me wrap withSome
so I don't see a cleaner way. Seems like either a shortcoming of iterators or I'm just going about this all wrong 😁Input/thoughts/suggestions welcome.
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.
Looks like there may be a crate, https://docs.rs/fallible-iterator/0.2.0/fallible_iterator/ that calls out the situation.
Seems like using it might be the cleaner way to go, but will defer to more experienced opinions.
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.
I have a
FallibleIterator
based solution and it cleans up pretty much all of the ugliness in error handling here. It's not perfect since you can't usefor x in y
, and have to go withwhile let Some(x) = y.next()
, but that's a limitation of rust and seems like a reasonable tradeoff for cleaner error handling.Changes for that can be seen with https:/ross/maxminddb-rust/compare/within-support...ross:within-support-fallibleiterator?expand=1. Thoughts welcome.
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.
Although it does clean up the code somewhat, I think I'd prefer not having the dependency, especially given that the fallible iterator crate hasn't seen an update in two years and doesn't seem that widely used.
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.
I think it hasn't seen any updates b/c it does what it needs to do. Similar to how I wrote requests-futures 8 years ago and haven't really touched it meaningfully in most of that time.
As for not wanting the dep, I don't have a response to that. The code to implement the non-fallible iterator is ugly as hell, but using it isn't THAT bad. Kind of seems like a shortcoming of Rust itself that iterators can't fail.
I'll stick this back on my TODO list to pick back up and see if I can get it over the line.