Skip to content

Commit da79410

Browse files
committed
Return an Option rather than AddressNotFoundError
when an address is not in the database. This seems more idiomatic given that it is an expected situation. This will hopefully make the error handling simpler. Closes #83.
1 parent ae20d19 commit da79410

File tree

4 files changed

+296
-134
lines changed

4 files changed

+296
-134
lines changed

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@
22

33
## 0.26.0
44

5+
* **BREAKING CHANGE:** The `lookup` and `lookup_prefix` methods now return
6+
`Ok(None)` or `Ok((None, prefix_len))` respectively when an IP address is
7+
valid but not found in the database (or has no associated data record),
8+
instead of returning an `Err(MaxMindDBError::AddressNotFoundError)`.
9+
Code previously matching on `AddressNotFoundError` must be updated to
10+
handle the `Ok(None)` / `Ok((None, prefix_len))` variants.
11+
* `lookup_prefix` now returns the prefix length of the entry even when the
12+
value is not found.
513
* Fixed an internal bounds checking error when resolving data pointers.
614
The previous logic could cause a panic on a corrupt database.
715

examples/lookup.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ fn main() -> Result<(), String> {
1414
.ok_or("Second argument must be the IP address, like 128.101.101.101")?
1515
.parse()
1616
.unwrap();
17-
let city: geoip2::City = reader.lookup(ip).unwrap();
17+
let city: Option<geoip2::City> = reader.lookup(ip).unwrap();
1818
println!("{city:#?}");
1919
Ok(())
2020
}

src/maxminddb/lib.rs

Lines changed: 127 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ compile_error!("features `simdutf8` and `unsafe-str-decode` are mutually exclusi
2323

2424
#[derive(Debug, PartialEq, Eq)]
2525
pub enum MaxMindDBError {
26-
AddressNotFoundError(String),
2726
InvalidDatabaseError(String),
2827
IoError(String),
2928
MapError(String),
@@ -41,9 +40,6 @@ impl From<io::Error> for MaxMindDBError {
4140
impl Display for MaxMindDBError {
4241
fn fmt(&self, fmt: &mut Formatter<'_>) -> Result<(), fmt::Error> {
4342
match self {
44-
MaxMindDBError::AddressNotFoundError(msg) => {
45-
write!(fmt, "AddressNotFoundError: {msg}")?
46-
}
4743
MaxMindDBError::InvalidDatabaseError(msg) => {
4844
write!(fmt, "InvalidDatabaseError: {msg}")?
4945
}
@@ -231,7 +227,10 @@ impl<'de> Reader<Mmap> {
231227
/// # Example
232228
///
233229
/// ```
230+
/// # #[cfg(feature = "mmap")]
231+
/// # {
234232
/// let reader = maxminddb::Reader::open_mmap("test-data/test-data/GeoIP2-City-Test.mmdb").unwrap();
233+
/// # }
235234
/// ```
236235
pub fn open_mmap<P: AsRef<Path>>(database: P) -> Result<Reader<Mmap>, MaxMindDBError> {
237236
let file_read = File::open(database)?;
@@ -286,59 +285,93 @@ impl<'de, S: AsRef<[u8]>> Reader<S> {
286285
Ok(reader)
287286
}
288287

289-
/// Lookup the socket address in the opened MaxMind DB
288+
/// Lookup the socket address in the opened MaxMind DB.
289+
/// Returns `Ok(None)` if the address is not found in the database.
290290
///
291291
/// Example:
292292
///
293293
/// ```
294-
/// use maxminddb::geoip2;
295-
/// use std::net::IpAddr;
296-
/// use std::str::FromStr;
297-
///
298-
/// let reader = maxminddb::Reader::open_readfile("test-data/test-data/GeoIP2-City-Test.mmdb").unwrap();
294+
/// # use maxminddb::geoip2;
295+
/// # use std::net::IpAddr;
296+
/// # use std::str::FromStr;
297+
/// # fn main() -> Result<(), maxminddb::MaxMindDBError> {
298+
/// let reader = maxminddb::Reader::open_readfile("test-data/test-data/GeoIP2-City-Test.mmdb")?;
299299
///
300300
/// let ip: IpAddr = FromStr::from_str("89.160.20.128").unwrap();
301-
/// let city: geoip2::City = reader.lookup(ip).unwrap();
302-
/// print!("{:?}", city);
301+
/// if let Some(city) = reader.lookup::<geoip2::City>(ip)? {
302+
/// println!("{:?}", city);
303+
/// } else {
304+
/// println!("Address not found");
305+
/// }
306+
/// # Ok(())
307+
/// # }
303308
/// ```
304-
pub fn lookup<T>(&'de self, address: IpAddr) -> Result<T, MaxMindDBError>
309+
pub fn lookup<T>(&'de self, address: IpAddr) -> Result<Option<T>, MaxMindDBError>
305310
where
306311
T: Deserialize<'de>,
307312
{
308-
self.lookup_prefix(address).map(|(v, _)| v)
313+
self.lookup_prefix(address)
314+
.map(|(option_value, _prefix_len)| option_value)
309315
}
310316

311-
/// Lookup the socket address in the opened MaxMind DB
317+
/// Lookup the socket address in the opened MaxMind DB, returning the found value (if any)
318+
/// and the prefix length of the network associated with the lookup.
319+
///
320+
/// Returns `Ok((None, prefix_len))` if the address is found in the tree but has no data record.
321+
/// Returns `Err(...)` for database errors (IO, corruption, decoding).
312322
///
313323
/// Example:
314324
///
315325
/// ```
316-
/// use maxminddb::geoip2;
317-
/// use std::net::IpAddr;
318-
/// use std::str::FromStr;
326+
/// # use maxminddb::geoip2;
327+
/// # use std::net::IpAddr;
328+
/// # use std::str::FromStr;
329+
/// # fn main() -> Result<(), maxminddb::MaxMindDBError> {
330+
/// let reader = maxminddb::Reader::open_readfile("test-data/test-data/GeoIP2-City-Test.mmdb")?;
319331
///
320-
/// let reader = maxminddb::Reader::open_readfile("test-data/test-data/GeoIP2-City-Test.mmdb").unwrap();
332+
/// let ip: IpAddr = "89.160.20.128".parse().unwrap(); // Known IP
333+
/// let ip_unknown: IpAddr = "10.0.0.1".parse().unwrap(); // Unknown IP
334+
///
335+
/// let (city_option, prefix_len) = reader.lookup_prefix::<geoip2::City>(ip)?;
336+
/// if let Some(city) = city_option {
337+
/// println!("Found {:?} at prefix length {}", city.city.unwrap().names.unwrap().get("en").unwrap(), prefix_len);
338+
/// } else {
339+
/// // This case is less likely with lookup_prefix if the IP resolves in the tree
340+
/// println!("IP found in tree but no data (prefix_len: {})", prefix_len);
341+
/// }
321342
///
322-
/// let ip: IpAddr = "89.160.20.128".parse().unwrap();
323-
/// let (city, prefix_len) = reader.lookup_prefix::<geoip2::City>(ip).unwrap();
324-
/// print!("{:?}, prefix length: {}", city, prefix_len);
343+
/// let (city_option_unknown, prefix_len_unknown) = reader.lookup_prefix::<geoip2::City>(ip_unknown)?;
344+
/// assert!(city_option_unknown.is_none());
345+
/// println!("Unknown IP resolved to prefix_len: {}", prefix_len_unknown);
346+
/// # Ok(())
347+
/// # }
325348
/// ```
326-
pub fn lookup_prefix<T>(&'de self, address: IpAddr) -> Result<(T, usize), MaxMindDBError>
349+
pub fn lookup_prefix<T>(
350+
&'de self,
351+
address: IpAddr,
352+
) -> Result<(Option<T>, usize), MaxMindDBError>
327353
where
328354
T: Deserialize<'de>,
329355
{
330356
let ip_int = IpInt::new(address);
357+
// find_address_in_tree returns Result<(usize, usize), MaxMindDBError> -> (pointer, prefix_len)
331358
let (pointer, prefix_len) = self.find_address_in_tree(&ip_int)?;
359+
332360
if pointer == 0 {
333-
return Err(MaxMindDBError::AddressNotFoundError(
334-
"Address not found in database".to_owned(),
335-
));
361+
// If pointer is 0, it signifies no data record was associated during tree traversal.
362+
// Return None for the data, but include the calculated prefix_len.
363+
return Ok((None, prefix_len));
336364
}
337365

366+
// If pointer > 0, attempt to resolve and decode data
338367
let rec = self.resolve_data_pointer(pointer)?;
339368
let mut decoder = decoder::Decoder::new(&self.buf.as_ref()[self.pointer_base..], rec);
340369

341-
T::deserialize(&mut decoder).map(|v| (v, prefix_len))
370+
// Attempt to deserialize. If successful, wrap in Some. If error, propagate.
371+
match T::deserialize(&mut decoder) {
372+
Ok(value) => Ok((Some(value), prefix_len)),
373+
Err(e) => Err(e),
374+
}
342375
}
343376

344377
/// Iterate over blocks of IP networks in the opened MaxMind DB
@@ -423,6 +456,8 @@ impl<'de, S: AsRef<[u8]>> Reader<S> {
423456
node = self.read_node(node, bit as usize)?;
424457
}
425458
match node_count {
459+
// If node == node_count, it means we hit the placeholder "empty" node
460+
// return 0 as the pointer value to signify "not found".
426461
n if n == node => Ok((0, prefix_len)),
427462
n if node > n => Ok((node, prefix_len)),
428463
_ => Err(MaxMindDBError::InvalidDatabaseError(
@@ -494,9 +529,8 @@ impl<'de, S: AsRef<[u8]>> Reader<S> {
494529

495530
// Check bounds using pointer_base which marks the start of the data section
496531
if resolved >= (self.buf.as_ref().len() - self.pointer_base) {
497-
return Err(MaxMindDBError::InvalidDatabaseError(
498-
"the MaxMind DB file's data pointer resolves to an invalid location"
499-
.to_owned(),
532+
return Err(MaxMindDBError::InvalidDatabaseError(
533+
"the MaxMind DB file's data pointer resolves to an invalid location".to_owned(),
500534
));
501535
}
502536

@@ -548,13 +582,6 @@ mod tests {
548582

549583
#[test]
550584
fn test_error_display() {
551-
assert_eq!(
552-
format!(
553-
"{}",
554-
MaxMindDBError::AddressNotFoundError("something went wrong".to_owned())
555-
),
556-
"AddressNotFoundError: something went wrong".to_owned(),
557-
);
558585
assert_eq!(
559586
format!(
560587
"{}",
@@ -583,5 +610,68 @@ mod tests {
583610
),
584611
"DecodingError: something went wrong".to_owned(),
585612
);
613+
assert_eq!(
614+
format!(
615+
"{}",
616+
MaxMindDBError::InvalidNetworkError("something went wrong".to_owned())
617+
),
618+
"InvalidNetworkError: something went wrong".to_owned(),
619+
);
620+
}
621+
622+
#[test]
623+
fn test_lookup_returns_none_for_unknown_address() {
624+
use super::Reader;
625+
use crate::geoip2;
626+
use std::net::IpAddr;
627+
use std::str::FromStr;
628+
629+
let reader = Reader::open_readfile("test-data/test-data/GeoIP2-City-Test.mmdb").unwrap();
630+
let ip: IpAddr = FromStr::from_str("10.0.0.1").unwrap();
631+
632+
let result_lookup = reader.lookup::<geoip2::City>(ip);
633+
assert!(
634+
matches!(result_lookup, Ok(None)),
635+
"lookup should return Ok(None) for unknown IP"
636+
);
637+
638+
let result_lookup_prefix = reader.lookup_prefix::<geoip2::City>(ip);
639+
assert!(
640+
matches!(result_lookup_prefix, Ok((None, 8))),
641+
"lookup_prefix should return Ok(None) for unknown IP"
642+
);
643+
}
644+
645+
#[test]
646+
fn test_lookup_returns_some_for_known_address() {
647+
use super::Reader;
648+
use crate::geoip2;
649+
use std::net::IpAddr;
650+
use std::str::FromStr;
651+
652+
let reader = Reader::open_readfile("test-data/test-data/GeoIP2-City-Test.mmdb").unwrap();
653+
let ip: IpAddr = FromStr::from_str("89.160.20.128").unwrap();
654+
655+
let result_lookup = reader.lookup::<geoip2::City>(ip);
656+
assert!(
657+
matches!(result_lookup, Ok(Some(_))),
658+
"lookup should return Ok(Some(_)) for known IP"
659+
);
660+
assert!(
661+
result_lookup.unwrap().unwrap().city.is_some(),
662+
"Expected city data"
663+
);
664+
665+
let result_lookup_prefix = reader.lookup_prefix::<geoip2::City>(ip);
666+
assert!(
667+
matches!(result_lookup_prefix, Ok((Some(_), _))),
668+
"lookup_prefix should return Ok(Some(_)) for known IP"
669+
);
670+
let (city_data, prefix_len) = result_lookup_prefix.unwrap();
671+
assert!(
672+
city_data.unwrap().city.is_some(),
673+
"Expected city data from prefix lookup"
674+
);
675+
assert_eq!(prefix_len, 25, "Expected valid prefix length");
586676
}
587677
}

0 commit comments

Comments
 (0)