Skip to content

#62 - ASN is still empty when databases have values #68

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.

Sign up for GitHub

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

Open
nhi-vanye wants to merge 2 commits into
base: master
Choose a base branch
from
Open

#62 - ASN is still empty when databases have values #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de2dd65
Fixed accessing the extended attributes (i.e. ASN)
nhi-vanye Apr 26, 2023
0e555c6
Merge pull request #1 from nhi-vanye/fix-extended-fields
nhi-vanye Apr 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Package: rgeolocate
Type: Package
Title: IP Address Geolocation
Version: 1.4.2
Version: 1.4.3
Date: 2021-12-18
Author: Os Keyes [aut, cre], Drew Schmidt [aut], David Robinson [ctb],
Chris Davis [ctb], Bob Rudis [ctb], Maxmind, Inc. [cph], Pascal Gloor [cph], IP2Location.com [cph]
Expand Down
3 changes: 2 additions & 1 deletion R/maxmind.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#' \item{asn}{: Autonomous System Number. Requires an ISP database.}
#' \item{aso}{: Autonomous System Organization. Requires an ISP database.}
#' \item{connection}{: the type of internet connection. Requires a connection type/netspeed database.}
#' \item{user_type}{: the type of organization, i.e. Hosting, Corporate. Requires an ISP database.}
#'}
#'@details
#'\code{geolookup} uses the \href{https://dev.maxmind.com/geoip/geoip2/downloadable/}{MaxMind GeoIP2 databases}
Expand Down Expand Up @@ -56,4 +57,4 @@ maxmind <- function(ips, file, fields = c("continent_name", "country_name",
}

return(maxmind_(ips, normalizePath(path.expand(file)), fields))
}
}
4 changes: 2 additions & 2 deletions R/zzz.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ rgeo_env <- new.env(parent = emptyenv())
.onLoad <- function(...) {
maxmind_tags <- c("continent_name", "country_name", "country_code", "region_name",
"city_name", "timezone", "connection", "city_geoname_id", "latitude", "longitude",
"isp", "organization", "asn", "aso", "postcode","city_metro_code")
"isp", "organization", "asn", "aso", "postcode","city_metro_code", "user_type")
ipinfo_tags <- c("hostname", "city", "region", "country",
"loc", "org", "postal", "phone")

Expand All @@ -15,4 +15,4 @@ rgeo_env <- new.env(parent = emptyenv())
assign("maxmind_tags", maxmind_tags, envir = rgeo_env)
assign("ipinfo_tags", ipinfo_tags, envir = rgeo_env)
assign("ip2loc_tags", ip2loc_tags, envir = rgeo_env)
}
}
16 changes: 11 additions & 5 deletions src/maxmind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,23 +175,23 @@ CharacterVector maxmind_bindings::postcode(MMDB_s *data, CharacterVector ip_addr
}

CharacterVector maxmind_bindings::connection(MMDB_s *data, CharacterVector ip_addresses){
return mmdb_getstring(data, ip_addresses, "connection_type", NULL);
return mmdb_getstring(data, ip_addresses, "traits", "connection_type", NULL);
}

CharacterVector maxmind_bindings::isp(MMDB_s *data, CharacterVector ip_addresses){
return mmdb_getstring(data, ip_addresses, "isp", NULL);
return mmdb_getstring(data, ip_addresses, "traits", "isp", NULL);
}

CharacterVector maxmind_bindings::organization(MMDB_s *data, CharacterVector ip_addresses){
return mmdb_getstring(data, ip_addresses, "organization", NULL);
return mmdb_getstring(data, ip_addresses, "traits", "organization", NULL);
}

IntegerVector maxmind_bindings::asn(MMDB_s *data, CharacterVector ip_addresses){
return mmdb_getint32(data, ip_addresses, "autonomous_system_number", NULL);
return mmdb_getint32(data, ip_addresses, "traits", "autonomous_system_number", NULL);
Copy link
Copy Markdown

@oschwald oschwald Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these changes would work with the MaxMind GeoIP2 ISP, ASN, or Connection-Type databases. They do not use the nested traits map.

Copy link
Copy Markdown
Author

@nhi-vanye nhi-vanye Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original bug was for a "MaxMind Enterprise" database - which was also using the "traits" map.

How would you like issues between different layouts handled? I don't have access to other sources...

I've solved my issue, so I'm okay if its only a partial fix as I'm unlikely to use different database vendors...But making it clean for others would be good...

Copy link
Copy Markdown

@oschwald oschwald Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are test databases for the types MaxMind provides.

Yeah, I am not exactly sure what the best fix is given what this code looks like. I guess one option is to check both locations. Another would be to check the database type and do the right thing based on that.

The official MaxMind libraries generally return a structured objects with separate lookup methods for the different database types or as nested maps for the lower-level APIs.

}

CharacterVector maxmind_bindings::aso(MMDB_s *data, CharacterVector ip_addresses){
return mmdb_getstring(data, ip_addresses, "autonomous_system_organization", NULL);
return mmdb_getstring(data, ip_addresses, "traits", "autonomous_system_organization", NULL);
}

IntegerVector maxmind_bindings::city_geoname_id(MMDB_s *data, CharacterVector ip_addresses){
Expand All @@ -210,6 +210,10 @@ NumericVector maxmind_bindings::longitude(MMDB_s *data, CharacterVector ip_addre
return mmdb_getdouble(data, ip_addresses, "location", "longitude", NULL);
}

CharacterVector maxmind_bindings::user_type(MMDB_s *data, CharacterVector ip_addresses){
return mmdb_getstring(data, ip_addresses, "traits", "user_type", NULL);
}

List maxmind_bindings::lookup(CharacterVector ip_addresses, MMDB_s *mmdb_set,
std::vector < std::string > fields){

Expand Down Expand Up @@ -249,6 +253,8 @@ List maxmind_bindings::lookup(CharacterVector ip_addresses, MMDB_s *mmdb_set,
output.push_back(city_geoname_id(mmdb_set, ip_addresses));
} else if (fields[i] == "city_metro_code") {
output.push_back(city_metro_code(mmdb_set, ip_addresses));
} else if (fields[i] == "user_type") {
output.push_back(user_type(mmdb_set, ip_addresses));
}

}
Expand Down
2 changes: 2 additions & 0 deletions src/maxmind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class maxmind_bindings {

NumericVector longitude(MMDB_s *data, CharacterVector ip_addresses);

CharacterVector user_type(MMDB_s *data, CharacterVector ip_addresses);

List lookup(CharacterVector ip_addresses, MMDB_s *mmdb_set, std::vector < std::string > fields);

public:
Expand Down