From cc8fa328580e4e27a86734a4fd2346dd2da0cb6a Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Fri, 6 Mar 2026 15:29:26 -0800 Subject: [PATCH 1/8] Fix header_rewrite MaxMind geo lookups for standard mmdb databases The MaxMind geo lookup code used incorrect field paths that don't match the structure of standard GeoLite2 and DBIP mmdb databases. Country lookups used a flat "country_code" field that doesn't exist; the correct path is "country" -> "names" -> "en" for country name and "country" -> "iso_code" for the ISO code. Changes: - Fix GEO_QUAL_COUNTRY to use nested path "country/names/en" - Add missing GEO_QUAL_COUNTRY_ISO support using "country/iso_code" - Add has_data and type checks before accessing entry data - Fix memory leak: set gMaxMindDB to nullptr after delete on open failure - Ensure entry_data_list is freed on all code paths Verified with DBIP Country Lite and ASN Lite databases using the same MMDB_get_value paths. Matches the field paths used by maxmind_acl plugin. Fixes: #11812 --- .../header_rewrite/conditions_geo_maxmind.cc | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/plugins/header_rewrite/conditions_geo_maxmind.cc b/plugins/header_rewrite/conditions_geo_maxmind.cc index 5fd905deca1..fe72d012d1f 100644 --- a/plugins/header_rewrite/conditions_geo_maxmind.cc +++ b/plugins/header_rewrite/conditions_geo_maxmind.cc @@ -51,6 +51,7 @@ MMConditionGeo::initLibrary(const std::string &path) if (MMDB_SUCCESS != status) { Dbg(pi_dbg_ctl, "Cannot open %s - %s", path.c_str(), MMDB_strerror(status)); delete gMaxMindDB; + gMaxMindDB = nullptr; return; } Dbg(pi_dbg_ctl, "Loaded %s", path.c_str()); @@ -91,33 +92,36 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const return ret; } - const char *field_name; + MMDB_entry_data_s entry_data; + switch (_geo_qual) { case GEO_QUAL_COUNTRY: - field_name = "country_code"; + status = MMDB_get_value(&result.entry, &entry_data, "country", "names", "en", NULL); + break; + case GEO_QUAL_COUNTRY_ISO: + status = MMDB_get_value(&result.entry, &entry_data, "country", "iso_code", NULL); break; case GEO_QUAL_ASN_NAME: - field_name = "autonomous_system_organization"; + status = MMDB_get_value(&result.entry, &entry_data, "autonomous_system_organization", NULL); break; default: Dbg(pi_dbg_ctl, "Unsupported field %d", _geo_qual); + MMDB_free_entry_data_list(entry_data_list); return ret; - break; } - MMDB_entry_data_s entry_data; - - status = MMDB_get_value(&result.entry, &entry_data, field_name, NULL); if (MMDB_SUCCESS != status) { - Dbg(pi_dbg_ctl, "ERROR on get value asn value: %s", MMDB_strerror(status)); + Dbg(pi_dbg_ctl, "Error looking up geo string field: %s", MMDB_strerror(status)); + MMDB_free_entry_data_list(entry_data_list); return ret; } - ret = std::string(entry_data.utf8_string, entry_data.data_size); - if (nullptr != entry_data_list) { - MMDB_free_entry_data_list(entry_data_list); + if (entry_data.has_data && entry_data.type == MMDB_DATA_TYPE_UTF8_STRING) { + ret = std::string(entry_data.utf8_string, entry_data.data_size); } + MMDB_free_entry_data_list(entry_data_list); + return ret; } @@ -156,29 +160,29 @@ MMConditionGeo::get_geo_int(const sockaddr *addr) const return ret; } - const char *field_name; + MMDB_entry_data_s entry_data; + switch (_geo_qual) { case GEO_QUAL_ASN: - field_name = "autonomous_system_number"; + status = MMDB_get_value(&result.entry, &entry_data, "autonomous_system_number", NULL); break; default: Dbg(pi_dbg_ctl, "Unsupported field %d", _geo_qual); + MMDB_free_entry_data_list(entry_data_list); return ret; - break; } - MMDB_entry_data_s entry_data; - - status = MMDB_get_value(&result.entry, &entry_data, field_name, NULL); if (MMDB_SUCCESS != status) { - Dbg(pi_dbg_ctl, "ERROR on get value asn value: %s", MMDB_strerror(status)); + Dbg(pi_dbg_ctl, "Error looking up geo int field: %s", MMDB_strerror(status)); + MMDB_free_entry_data_list(entry_data_list); return ret; } - ret = entry_data.uint32; - if (nullptr != entry_data_list) { - MMDB_free_entry_data_list(entry_data_list); + if (entry_data.has_data && entry_data.type == MMDB_DATA_TYPE_UINT32) { + ret = entry_data.uint32; } + MMDB_free_entry_data_list(entry_data_list); + return ret; } From 58588ed9026d8c0fe714b432208369f521a07475 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Fri, 6 Mar 2026 15:42:10 -0800 Subject: [PATCH 2/8] Address Copilot review feedback - GEO_QUAL_COUNTRY now returns iso_code ("KR") instead of English name ("South Korea"), matching the GeoIP backend behavior - Remove dead GEO_QUAL_COUNTRY_ISO case from get_geo_string() since the dispatcher routes it to get_geo_int() - Remove unnecessary MMDB_get_entry_data_list() allocation -- the MMDB_get_value() API works directly from result.entry --- .../header_rewrite/conditions_geo_maxmind.cc | 37 +------------------ 1 file changed, 2 insertions(+), 35 deletions(-) diff --git a/plugins/header_rewrite/conditions_geo_maxmind.cc b/plugins/header_rewrite/conditions_geo_maxmind.cc index fe72d012d1f..fb4e99d161c 100644 --- a/plugins/header_rewrite/conditions_geo_maxmind.cc +++ b/plugins/header_rewrite/conditions_geo_maxmind.cc @@ -75,30 +75,16 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const return ret; } - MMDB_entry_data_list_s *entry_data_list = nullptr; if (!result.found_entry) { Dbg(pi_dbg_ctl, "No entry for this IP was found"); return ret; } - int status = MMDB_get_entry_data_list(&result.entry, &entry_data_list); - if (MMDB_SUCCESS != status) { - Dbg(pi_dbg_ctl, "Error looking up entry data: %s", MMDB_strerror(status)); - return ret; - } - - if (entry_data_list == nullptr) { - Dbg(pi_dbg_ctl, "No data found"); - return ret; - } - MMDB_entry_data_s entry_data; + int status; switch (_geo_qual) { case GEO_QUAL_COUNTRY: - status = MMDB_get_value(&result.entry, &entry_data, "country", "names", "en", NULL); - break; - case GEO_QUAL_COUNTRY_ISO: status = MMDB_get_value(&result.entry, &entry_data, "country", "iso_code", NULL); break; case GEO_QUAL_ASN_NAME: @@ -106,13 +92,11 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const break; default: Dbg(pi_dbg_ctl, "Unsupported field %d", _geo_qual); - MMDB_free_entry_data_list(entry_data_list); return ret; } if (MMDB_SUCCESS != status) { Dbg(pi_dbg_ctl, "Error looking up geo string field: %s", MMDB_strerror(status)); - MMDB_free_entry_data_list(entry_data_list); return ret; } @@ -120,8 +104,6 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const ret = std::string(entry_data.utf8_string, entry_data.data_size); } - MMDB_free_entry_data_list(entry_data_list); - return ret; } @@ -143,24 +125,13 @@ MMConditionGeo::get_geo_int(const sockaddr *addr) const return ret; } - MMDB_entry_data_list_s *entry_data_list = nullptr; if (!result.found_entry) { Dbg(pi_dbg_ctl, "No entry for this IP was found"); return ret; } - int status = MMDB_get_entry_data_list(&result.entry, &entry_data_list); - if (MMDB_SUCCESS != status) { - Dbg(pi_dbg_ctl, "Error looking up entry data: %s", MMDB_strerror(status)); - return ret; - } - - if (entry_data_list == nullptr) { - Dbg(pi_dbg_ctl, "No data found"); - return ret; - } - MMDB_entry_data_s entry_data; + int status; switch (_geo_qual) { case GEO_QUAL_ASN: @@ -168,13 +139,11 @@ MMConditionGeo::get_geo_int(const sockaddr *addr) const break; default: Dbg(pi_dbg_ctl, "Unsupported field %d", _geo_qual); - MMDB_free_entry_data_list(entry_data_list); return ret; } if (MMDB_SUCCESS != status) { Dbg(pi_dbg_ctl, "Error looking up geo int field: %s", MMDB_strerror(status)); - MMDB_free_entry_data_list(entry_data_list); return ret; } @@ -182,7 +151,5 @@ MMConditionGeo::get_geo_int(const sockaddr *addr) const ret = entry_data.uint32; } - MMDB_free_entry_data_list(entry_data_list); - return ret; } From 318b1bee665a2d3f7e77fabb312476d3e4ffffe4 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Fri, 6 Mar 2026 15:49:30 -0800 Subject: [PATCH 3/8] Add MaxMind MMDB path validation test to header_rewrite Adds a test that verifies the MMDB lookup paths used by the geo conditions (country/iso_code, country/names/en) work correctly against a real MaxMind or DBIP database. The test skips gracefully if no MMDB file is available, and can be pointed at a specific file via MMDB_COUNTRY_PATH environment variable. --- plugins/header_rewrite/CMakeLists.txt | 1 + plugins/header_rewrite/header_rewrite_test.cc | 114 ++++++++++++++++++ 2 files changed, 115 insertions(+) diff --git a/plugins/header_rewrite/CMakeLists.txt b/plugins/header_rewrite/CMakeLists.txt index 4c658b78ba9..d333d877a2a 100644 --- a/plugins/header_rewrite/CMakeLists.txt +++ b/plugins/header_rewrite/CMakeLists.txt @@ -60,6 +60,7 @@ if(BUILD_TESTING) target_link_libraries(test_header_rewrite PRIVATE header_rewrite_parser ts::inkevent ts::tscore) if(maxminddb_FOUND) + target_compile_definitions(test_header_rewrite PRIVATE TS_USE_HRW_MAXMINDDB=1) target_link_libraries(test_header_rewrite PRIVATE maxminddb::maxminddb) endif() diff --git a/plugins/header_rewrite/header_rewrite_test.cc b/plugins/header_rewrite/header_rewrite_test.cc index 8e92f7ae5f1..49d8d3adb7a 100644 --- a/plugins/header_rewrite/header_rewrite_test.cc +++ b/plugins/header_rewrite/header_rewrite_test.cc @@ -24,9 +24,15 @@ #include #include #include +#include #include "parser.h" +#if TS_USE_HRW_MAXMINDDB +#include +#include +#endif + namespace header_rewrite_ns { const char PLUGIN_NAME[] = "TEST_header_rewrite"; @@ -538,6 +544,108 @@ test_tokenizer() return errors; } +#if TS_USE_HRW_MAXMINDDB +static bool +file_exists(const char *path) +{ + struct stat st; + return stat(path, &st) == 0; +} + +static const char * +find_country_mmdb() +{ + static const char *paths[] = { + "/usr/share/GeoIP/GeoLite2-Country.mmdb", "/usr/local/share/GeoIP/GeoLite2-Country.mmdb", + "/var/lib/GeoIP/GeoLite2-Country.mmdb", "/opt/geoip/GeoLite2-Country.mmdb", + "/usr/share/GeoIP/GeoIP2-Country.mmdb", "/usr/share/GeoIP/dbip-country-lite.mmdb", + }; + for (auto *p : paths) { + if (file_exists(p)) { + return p; + } + } + if (const char *env = getenv("MMDB_COUNTRY_PATH")) { + if (file_exists(env)) { + return env; + } + } + return nullptr; +} + +int +test_maxmind_geo() +{ + const char *db_path = find_country_mmdb(); + if (db_path == nullptr) { + std::cout << "SKIP: No MaxMind country mmdb found (set MMDB_COUNTRY_PATH to override)" << std::endl; + return 0; + } + + std::cout << "Testing MaxMind geo lookups with: " << db_path << std::endl; + + int errors = 0; + MMDB_s mmdb; + int status = MMDB_open(db_path, MMDB_MODE_MMAP, &mmdb); + if (MMDB_SUCCESS != status) { + std::cerr << "Cannot open " << db_path << ": " << MMDB_strerror(status) << std::endl; + return 1; + } + + int gai_error, mmdb_error; + MMDB_lookup_result_s result = MMDB_lookup_string(&mmdb, "8.8.8.8", &gai_error, &mmdb_error); + + if (MMDB_SUCCESS != mmdb_error || !result.found_entry) { + std::cerr << "Cannot look up 8.8.8.8 in " << db_path << std::endl; + MMDB_close(&mmdb); + return 1; + } + + MMDB_entry_data_s entry_data; + + // Verify "country" -> "iso_code" path (used by GEO_QUAL_COUNTRY) + status = MMDB_get_value(&result.entry, &entry_data, "country", "iso_code", NULL); + if (MMDB_SUCCESS != status || !entry_data.has_data || entry_data.type != MMDB_DATA_TYPE_UTF8_STRING) { + std::cerr << "FAIL: country/iso_code lookup failed for 8.8.8.8" << std::endl; + ++errors; + } else { + std::string iso(entry_data.utf8_string, entry_data.data_size); + if (iso != "US") { + std::cerr << "FAIL: expected country iso_code 'US' for 8.8.8.8, got '" << iso << "'" << std::endl; + ++errors; + } else { + std::cout << " PASS: country/iso_code = " << iso << std::endl; + } + } + + // Verify "country" -> "names" -> "en" path exists (not used by header_rewrite but validates structure) + status = MMDB_get_value(&result.entry, &entry_data, "country", "names", "en", NULL); + if (MMDB_SUCCESS != status || !entry_data.has_data || entry_data.type != MMDB_DATA_TYPE_UTF8_STRING) { + std::cerr << "FAIL: country/names/en lookup failed for 8.8.8.8" << std::endl; + ++errors; + } else { + std::string name(entry_data.utf8_string, entry_data.data_size); + std::cout << " PASS: country/names/en = " << name << std::endl; + } + + // Verify loopback returns no entry + result = MMDB_lookup_string(&mmdb, "127.0.0.1", &gai_error, &mmdb_error); + if (MMDB_SUCCESS == mmdb_error && result.found_entry) { + std::cerr << "FAIL: expected no entry for 127.0.0.1" << std::endl; + ++errors; + } else { + std::cout << " PASS: 127.0.0.1 correctly returns no entry" << std::endl; + } + + MMDB_close(&mmdb); + + if (errors == 0) { + std::cout << "MaxMind geo tests passed" << std::endl; + } + return errors; +} +#endif + int main() { @@ -545,5 +653,11 @@ main() return 1; } +#if TS_USE_HRW_MAXMINDDB + if (test_maxmind_geo()) { + return 1; + } +#endif + return 0; } From dc1ab5dc73fb4860fab9390e7f21bd523c8a9440 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Mon, 9 Mar 2026 16:51:09 -0700 Subject: [PATCH 4/8] Address review feedback: check gai_error and add comments Check gai_error from MMDB_lookup_string() in the unit test instead of silently ignoring it (per zwoop review comment). Add comments explaining the nested MMDB field paths, nullptr-after-delete fix, data type validation guards, and the dual error code API. --- .../header_rewrite/conditions_geo_maxmind.cc | 9 ++++++++- plugins/header_rewrite/header_rewrite_test.cc | 17 +++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/plugins/header_rewrite/conditions_geo_maxmind.cc b/plugins/header_rewrite/conditions_geo_maxmind.cc index fb4e99d161c..1cb348aae35 100644 --- a/plugins/header_rewrite/conditions_geo_maxmind.cc +++ b/plugins/header_rewrite/conditions_geo_maxmind.cc @@ -51,7 +51,7 @@ MMConditionGeo::initLibrary(const std::string &path) if (MMDB_SUCCESS != status) { Dbg(pi_dbg_ctl, "Cannot open %s - %s", path.c_str(), MMDB_strerror(status)); delete gMaxMindDB; - gMaxMindDB = nullptr; + gMaxMindDB = nullptr; // allow retry on next call instead of dangling pointer return; } Dbg(pi_dbg_ctl, "Loaded %s", path.c_str()); @@ -83,8 +83,12 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const MMDB_entry_data_s entry_data; int status; + // GeoLite2/GeoIP2/DBIP databases use nested field paths, not flat names. + // Use MMDB_get_value() directly on the entry -- no need for the more + // expensive MMDB_get_entry_data_list() allocation. switch (_geo_qual) { case GEO_QUAL_COUNTRY: + // "country" -> "iso_code" returns e.g. "US", "KR" (matches old GeoIP backend behavior) status = MMDB_get_value(&result.entry, &entry_data, "country", "iso_code", NULL); break; case GEO_QUAL_ASN_NAME: @@ -100,6 +104,8 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const return ret; } + // Validate before access -- entry_data may be uninitialized if the field + // exists but has an unexpected type in a third-party database. if (entry_data.has_data && entry_data.type == MMDB_DATA_TYPE_UTF8_STRING) { ret = std::string(entry_data.utf8_string, entry_data.data_size); } @@ -135,6 +141,7 @@ MMConditionGeo::get_geo_int(const sockaddr *addr) const switch (_geo_qual) { case GEO_QUAL_ASN: + // GeoLite2-ASN / DBIP-ASN store this as a top-level uint32 field status = MMDB_get_value(&result.entry, &entry_data, "autonomous_system_number", NULL); break; default: diff --git a/plugins/header_rewrite/header_rewrite_test.cc b/plugins/header_rewrite/header_rewrite_test.cc index 49d8d3adb7a..fd55cccb8f4 100644 --- a/plugins/header_rewrite/header_rewrite_test.cc +++ b/plugins/header_rewrite/header_rewrite_test.cc @@ -31,6 +31,7 @@ #if TS_USE_HRW_MAXMINDDB #include #include +#include #endif namespace header_rewrite_ns @@ -592,11 +593,20 @@ test_maxmind_geo() return 1; } + // MMDB_lookup_string() returns two independent error codes: + // gai_error - getaddrinfo() failure (string-to-IP conversion) + // mmdb_error - MMDB lookup failure (database query) + // Check both to avoid masking failures. int gai_error, mmdb_error; MMDB_lookup_result_s result = MMDB_lookup_string(&mmdb, "8.8.8.8", &gai_error, &mmdb_error); + if (gai_error != 0) { + std::cerr << "getaddrinfo failed for 8.8.8.8: " << gai_strerror(gai_error) << std::endl; + MMDB_close(&mmdb); + return 1; + } if (MMDB_SUCCESS != mmdb_error || !result.found_entry) { - std::cerr << "Cannot look up 8.8.8.8 in " << db_path << std::endl; + std::cerr << "Cannot look up 8.8.8.8 in " << db_path << ": " << MMDB_strerror(mmdb_error) << std::endl; MMDB_close(&mmdb); return 1; } @@ -630,7 +640,10 @@ test_maxmind_geo() // Verify loopback returns no entry result = MMDB_lookup_string(&mmdb, "127.0.0.1", &gai_error, &mmdb_error); - if (MMDB_SUCCESS == mmdb_error && result.found_entry) { + if (gai_error != 0) { + std::cerr << "FAIL: getaddrinfo failed for 127.0.0.1: " << gai_strerror(gai_error) << std::endl; + ++errors; + } else if (MMDB_SUCCESS == mmdb_error && result.found_entry) { std::cerr << "FAIL: expected no entry for 127.0.0.1" << std::endl; ++errors; } else { From fedfd374a11fc909efb4af8d242b2168cc81f6b3 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Wed, 11 Mar 2026 12:20:16 -0700 Subject: [PATCH 5/8] Fix misleading comment: initLibrary runs under std::call_once, no retry --- plugins/header_rewrite/conditions_geo_maxmind.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/header_rewrite/conditions_geo_maxmind.cc b/plugins/header_rewrite/conditions_geo_maxmind.cc index 1cb348aae35..29308e632e6 100644 --- a/plugins/header_rewrite/conditions_geo_maxmind.cc +++ b/plugins/header_rewrite/conditions_geo_maxmind.cc @@ -51,7 +51,7 @@ MMConditionGeo::initLibrary(const std::string &path) if (MMDB_SUCCESS != status) { Dbg(pi_dbg_ctl, "Cannot open %s - %s", path.c_str(), MMDB_strerror(status)); delete gMaxMindDB; - gMaxMindDB = nullptr; // allow retry on next call instead of dangling pointer + gMaxMindDB = nullptr; // avoid leaving a dangling global pointer after delete return; } Dbg(pi_dbg_ctl, "Loaded %s", path.c_str()); From c24e44673fcc8bb4355607e5d46cf2fbc4a88f4d Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Thu, 12 Mar 2026 11:43:48 -0700 Subject: [PATCH 6/8] Auto-detect MMDB schema: support both flat and nested field layouts Some vendor MMDB databases use flat top-level fields (country_code), while standard GeoLite2/GeoIP2/DBIP databases use nested paths (country/iso_code). Detect the schema on the first successful lookup by probing for the nested path first, then falling back to flat. Cache the result in an atomic for the process lifetime. Add a Python script to generate test MMDB files with both schemas, and rewrite the unit test to verify lookups work against both. --- plugins/header_rewrite/CMakeLists.txt | 26 +++ .../header_rewrite/conditions_geo_maxmind.cc | 47 +++- plugins/header_rewrite/generate_test_mmdb.py | 103 +++++++++ plugins/header_rewrite/header_rewrite_test.cc | 204 ++++++++++++------ 4 files changed, 306 insertions(+), 74 deletions(-) create mode 100644 plugins/header_rewrite/generate_test_mmdb.py diff --git a/plugins/header_rewrite/CMakeLists.txt b/plugins/header_rewrite/CMakeLists.txt index d333d877a2a..5ee1fdd1991 100644 --- a/plugins/header_rewrite/CMakeLists.txt +++ b/plugins/header_rewrite/CMakeLists.txt @@ -62,6 +62,32 @@ if(BUILD_TESTING) if(maxminddb_FOUND) target_compile_definitions(test_header_rewrite PRIVATE TS_USE_HRW_MAXMINDDB=1) target_link_libraries(test_header_rewrite PRIVATE maxminddb::maxminddb) + + find_package( + Python3 + COMPONENTS Interpreter + QUIET + ) + if(Python3_FOUND) + set(_mmdb_test_dir "${CMAKE_CURRENT_BINARY_DIR}/test_mmdb") + add_custom_command( + OUTPUT "${_mmdb_test_dir}/test_flat_geo.mmdb" "${_mmdb_test_dir}/test_nested_geo.mmdb" + COMMAND ${CMAKE_COMMAND} -E make_directory "${_mmdb_test_dir}" + COMMAND ${Python3_EXECUTABLE} "${CMAKE_CURRENT_SOURCE_DIR}/generate_test_mmdb.py" "${_mmdb_test_dir}" + DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/generate_test_mmdb.py" + COMMENT "Generating test MMDB files for header_rewrite" + ) + add_custom_target( + generate_test_mmdb DEPENDS "${_mmdb_test_dir}/test_flat_geo.mmdb" "${_mmdb_test_dir}/test_nested_geo.mmdb" + ) + add_dependencies(test_header_rewrite generate_test_mmdb) + set_tests_properties( + test_header_rewrite + PROPERTIES + ENVIRONMENT + "MMDB_TEST_FLAT=${_mmdb_test_dir}/test_flat_geo.mmdb;MMDB_TEST_NESTED=${_mmdb_test_dir}/test_nested_geo.mmdb" + ) + endif() endif() # This test has linker issue when cripts is enabled, so its commented for now diff --git a/plugins/header_rewrite/conditions_geo_maxmind.cc b/plugins/header_rewrite/conditions_geo_maxmind.cc index 29308e632e6..c7fc131a6f6 100644 --- a/plugins/header_rewrite/conditions_geo_maxmind.cc +++ b/plugins/header_rewrite/conditions_geo_maxmind.cc @@ -23,6 +23,7 @@ #include #include +#include #include "ts/ts.h" @@ -32,6 +33,9 @@ MMDB_s *gMaxMindDB = nullptr; +enum class MmdbSchema : int { UNKNOWN = 0, NESTED = 1, FLAT = 2 }; +std::atomic gMmdbSchema{MmdbSchema::UNKNOWN}; + void MMConditionGeo::initLibrary(const std::string &path) { @@ -57,6 +61,30 @@ MMConditionGeo::initLibrary(const std::string &path) Dbg(pi_dbg_ctl, "Loaded %s", path.c_str()); } +// Detect whether the MMDB uses nested (GeoLite2) or flat (vendor) field layout +// by probing for the nested country path on a real lookup result. Called once +// on the first successful lookup; result is cached in gMmdbSchema. +static MmdbSchema +detect_schema(MMDB_entry_s *entry) +{ + MMDB_entry_data_s probe; + int status = MMDB_get_value(entry, &probe, "country", "iso_code", NULL); + + if (MMDB_SUCCESS == status && probe.has_data && probe.type == MMDB_DATA_TYPE_UTF8_STRING) { + Dbg(pi_dbg_ctl, "Detected nested MMDB schema (country/iso_code)"); + return MmdbSchema::NESTED; + } + + status = MMDB_get_value(entry, &probe, "country_code", NULL); + if (MMDB_SUCCESS == status && probe.has_data && probe.type == MMDB_DATA_TYPE_UTF8_STRING) { + Dbg(pi_dbg_ctl, "Detected flat MMDB schema (country_code)"); + return MmdbSchema::FLAT; + } + + Dbg(pi_dbg_ctl, "Could not detect MMDB schema; defaulting to nested"); + return MmdbSchema::NESTED; +} + std::string MMConditionGeo::get_geo_string(const sockaddr *addr) const { @@ -80,16 +108,23 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const return ret; } + // Lazy schema detection on first successful lookup + MmdbSchema schema = gMmdbSchema.load(std::memory_order_relaxed); + if (schema == MmdbSchema::UNKNOWN) { + schema = detect_schema(&result.entry); + gMmdbSchema.store(schema, std::memory_order_relaxed); + } + MMDB_entry_data_s entry_data; int status; - // GeoLite2/GeoIP2/DBIP databases use nested field paths, not flat names. - // Use MMDB_get_value() directly on the entry -- no need for the more - // expensive MMDB_get_entry_data_list() allocation. switch (_geo_qual) { case GEO_QUAL_COUNTRY: - // "country" -> "iso_code" returns e.g. "US", "KR" (matches old GeoIP backend behavior) - status = MMDB_get_value(&result.entry, &entry_data, "country", "iso_code", NULL); + if (schema == MmdbSchema::FLAT) { + status = MMDB_get_value(&result.entry, &entry_data, "country_code", NULL); + } else { + status = MMDB_get_value(&result.entry, &entry_data, "country", "iso_code", NULL); + } break; case GEO_QUAL_ASN_NAME: status = MMDB_get_value(&result.entry, &entry_data, "autonomous_system_organization", NULL); @@ -104,8 +139,6 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const return ret; } - // Validate before access -- entry_data may be uninitialized if the field - // exists but has an unexpected type in a third-party database. if (entry_data.has_data && entry_data.type == MMDB_DATA_TYPE_UTF8_STRING) { ret = std::string(entry_data.utf8_string, entry_data.data_size); } diff --git a/plugins/header_rewrite/generate_test_mmdb.py b/plugins/header_rewrite/generate_test_mmdb.py new file mode 100644 index 00000000000..efdba213267 --- /dev/null +++ b/plugins/header_rewrite/generate_test_mmdb.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Generate test MMDB files for header_rewrite geo lookup unit tests. + +Two schemas exist in the wild: + + Nested (GeoLite2/GeoIP2/DBIP): country -> iso_code + Flat (vendor-specific): country_code (top-level) + +This script generates one MMDB file for each schema so the C++ test +can verify that auto-detection works for both. + +Requires: pip install mmdb-writer +""" + +import os +import sys + +try: + from mmdb_writer import MMDBWriter, MmdbU32 + import netaddr +except ImportError: + print("SKIP: mmdb-writer or netaddr not installed (pip install mmdb-writer)", file=sys.stderr) + sys.exit(0) + + +def net(cidr): + return netaddr.IPSet([netaddr.IPNetwork(cidr)]) + + +def generate_flat(path): + """Flat schema: country_code at top level (vendor databases).""" + w = MMDBWriter(ip_version=4, database_type="Test-Flat-GeoIP") + w.insert_network( + net("8.8.8.0/24"), { + "country_code": "US", + "autonomous_system_number": MmdbU32(15169), + "autonomous_system_organization": "GOOGLE", + }) + w.insert_network( + net("1.2.3.0/24"), { + "country_code": "KR", + "autonomous_system_number": MmdbU32(9286), + "autonomous_system_organization": "KINX", + }) + w.to_db_file(path) + + +def generate_nested(path): + """Nested schema: country/iso_code (GeoLite2, GeoIP2, DBIP).""" + w = MMDBWriter(ip_version=4, database_type="Test-Nested-GeoIP2") + w.insert_network( + net("8.8.8.0/24"), { + "country": { + "iso_code": "US", + "names": { + "en": "United States" + } + }, + "autonomous_system_number": MmdbU32(15169), + "autonomous_system_organization": "GOOGLE", + }) + w.insert_network( + net("1.2.3.0/24"), { + "country": { + "iso_code": "KR", + "names": { + "en": "South Korea" + } + }, + "autonomous_system_number": MmdbU32(9286), + "autonomous_system_organization": "KINX", + }) + w.to_db_file(path) + + +if __name__ == "__main__": + outdir = sys.argv[1] if len(sys.argv) > 1 else "." + + flat_path = os.path.join(outdir, "test_flat_geo.mmdb") + nested_path = os.path.join(outdir, "test_nested_geo.mmdb") + + generate_flat(flat_path) + generate_nested(nested_path) + + print(f"Generated {flat_path} ({os.path.getsize(flat_path)} bytes)") + print(f"Generated {nested_path} ({os.path.getsize(nested_path)} bytes)") diff --git a/plugins/header_rewrite/header_rewrite_test.cc b/plugins/header_rewrite/header_rewrite_test.cc index fd55cccb8f4..c8cc2741993 100644 --- a/plugins/header_rewrite/header_rewrite_test.cc +++ b/plugins/header_rewrite/header_rewrite_test.cc @@ -546,6 +546,7 @@ test_tokenizer() } #if TS_USE_HRW_MAXMINDDB + static bool file_exists(const char *path) { @@ -553,108 +554,177 @@ file_exists(const char *path) return stat(path, &st) == 0; } -static const char * -find_country_mmdb() +static int +open_test_mmdb(MMDB_s &mmdb, const char *path) { - static const char *paths[] = { - "/usr/share/GeoIP/GeoLite2-Country.mmdb", "/usr/local/share/GeoIP/GeoLite2-Country.mmdb", - "/var/lib/GeoIP/GeoLite2-Country.mmdb", "/opt/geoip/GeoLite2-Country.mmdb", - "/usr/share/GeoIP/GeoIP2-Country.mmdb", "/usr/share/GeoIP/dbip-country-lite.mmdb", - }; - for (auto *p : paths) { - if (file_exists(p)) { - return p; - } + int status = MMDB_open(path, MMDB_MODE_MMAP, &mmdb); + if (MMDB_SUCCESS != status) { + std::cerr << "Cannot open " << path << ": " << MMDB_strerror(status) << std::endl; + return 1; } - if (const char *env = getenv("MMDB_COUNTRY_PATH")) { - if (file_exists(env)) { - return env; - } + return 0; +} + +static std::string +lookup_country(MMDB_s &mmdb, const char *ip) +{ + int gai_error, mmdb_error; + MMDB_lookup_result_s result = MMDB_lookup_string(&mmdb, ip, &gai_error, &mmdb_error); + + if (gai_error != 0 || MMDB_SUCCESS != mmdb_error || !result.found_entry) { + return "(lookup_failed)"; + } + + MMDB_entry_data_s entry_data; + + // Try nested path first (GeoLite2 style) + int status = MMDB_get_value(&result.entry, &entry_data, "country", "iso_code", NULL); + if (MMDB_SUCCESS == status && entry_data.has_data && entry_data.type == MMDB_DATA_TYPE_UTF8_STRING) { + return std::string(entry_data.utf8_string, entry_data.data_size); + } + + // Try flat path (vendor style) + status = MMDB_get_value(&result.entry, &entry_data, "country_code", NULL); + if (MMDB_SUCCESS == status && entry_data.has_data && entry_data.type == MMDB_DATA_TYPE_UTF8_STRING) { + return std::string(entry_data.utf8_string, entry_data.data_size); + } + + return "(not_found)"; +} + +static uint32_t +lookup_asn(MMDB_s &mmdb, const char *ip) +{ + int gai_error, mmdb_error; + MMDB_lookup_result_s result = MMDB_lookup_string(&mmdb, ip, &gai_error, &mmdb_error); + + if (gai_error != 0 || MMDB_SUCCESS != mmdb_error || !result.found_entry) { + return 0; + } + + MMDB_entry_data_s entry_data; + int status = MMDB_get_value(&result.entry, &entry_data, "autonomous_system_number", NULL); + if (MMDB_SUCCESS == status && entry_data.has_data && entry_data.type == MMDB_DATA_TYPE_UINT32) { + return entry_data.uint32; } - return nullptr; + return 0; } +// Test that we can read country codes from a flat-schema MMDB (vendor databases +// where country_code is a top-level string field). int -test_maxmind_geo() +test_flat_schema() { - const char *db_path = find_country_mmdb(); - if (db_path == nullptr) { - std::cout << "SKIP: No MaxMind country mmdb found (set MMDB_COUNTRY_PATH to override)" << std::endl; + const char *path = getenv("MMDB_TEST_FLAT"); + if (path == nullptr || !file_exists(path)) { + std::cout << "SKIP: flat-schema test mmdb not found (set MMDB_TEST_FLAT)" << std::endl; return 0; } - std::cout << "Testing MaxMind geo lookups with: " << db_path << std::endl; - + std::cout << "Testing flat-schema MMDB: " << path << std::endl; int errors = 0; MMDB_s mmdb; - int status = MMDB_open(db_path, MMDB_MODE_MMAP, &mmdb); - if (MMDB_SUCCESS != status) { - std::cerr << "Cannot open " << db_path << ": " << MMDB_strerror(status) << std::endl; + if (open_test_mmdb(mmdb, path) != 0) { return 1; } - // MMDB_lookup_string() returns two independent error codes: - // gai_error - getaddrinfo() failure (string-to-IP conversion) - // mmdb_error - MMDB lookup failure (database query) - // Check both to avoid masking failures. - int gai_error, mmdb_error; - MMDB_lookup_result_s result = MMDB_lookup_string(&mmdb, "8.8.8.8", &gai_error, &mmdb_error); + std::string country = lookup_country(mmdb, "8.8.8.8"); + if (country != "US") { + std::cerr << "FAIL: flat schema 8.8.8.8 expected 'US', got '" << country << "'" << std::endl; + ++errors; + } else { + std::cout << " PASS: flat 8.8.8.8 country = " << country << std::endl; + } - if (gai_error != 0) { - std::cerr << "getaddrinfo failed for 8.8.8.8: " << gai_strerror(gai_error) << std::endl; - MMDB_close(&mmdb); - return 1; + country = lookup_country(mmdb, "1.2.3.4"); + if (country != "KR") { + std::cerr << "FAIL: flat schema 1.2.3.4 expected 'KR', got '" << country << "'" << std::endl; + ++errors; + } else { + std::cout << " PASS: flat 1.2.3.4 country = " << country << std::endl; } - if (MMDB_SUCCESS != mmdb_error || !result.found_entry) { - std::cerr << "Cannot look up 8.8.8.8 in " << db_path << ": " << MMDB_strerror(mmdb_error) << std::endl; - MMDB_close(&mmdb); - return 1; + + uint32_t asn = lookup_asn(mmdb, "8.8.8.8"); + if (asn != 15169) { + std::cerr << "FAIL: flat schema 8.8.8.8 expected ASN 15169, got " << asn << std::endl; + ++errors; + } else { + std::cout << " PASS: flat 8.8.8.8 ASN = " << asn << std::endl; } - MMDB_entry_data_s entry_data; + // Loopback should not be found + country = lookup_country(mmdb, "127.0.0.1"); + if (country != "(lookup_failed)") { + std::cerr << "FAIL: flat schema 127.0.0.1 expected no entry, got '" << country << "'" << std::endl; + ++errors; + } else { + std::cout << " PASS: flat 127.0.0.1 correctly not found" << std::endl; + } - // Verify "country" -> "iso_code" path (used by GEO_QUAL_COUNTRY) - status = MMDB_get_value(&result.entry, &entry_data, "country", "iso_code", NULL); - if (MMDB_SUCCESS != status || !entry_data.has_data || entry_data.type != MMDB_DATA_TYPE_UTF8_STRING) { - std::cerr << "FAIL: country/iso_code lookup failed for 8.8.8.8" << std::endl; + MMDB_close(&mmdb); + return errors; +} + +// Test that we can read country codes from a nested-schema MMDB (GeoLite2/GeoIP2/DBIP +// where country is country -> iso_code). +int +test_nested_schema() +{ + const char *path = getenv("MMDB_TEST_NESTED"); + if (path == nullptr || !file_exists(path)) { + std::cout << "SKIP: nested-schema test mmdb not found (set MMDB_TEST_NESTED)" << std::endl; + return 0; + } + + std::cout << "Testing nested-schema MMDB: " << path << std::endl; + int errors = 0; + MMDB_s mmdb; + if (open_test_mmdb(mmdb, path) != 0) { + return 1; + } + + std::string country = lookup_country(mmdb, "8.8.8.8"); + if (country != "US") { + std::cerr << "FAIL: nested schema 8.8.8.8 expected 'US', got '" << country << "'" << std::endl; ++errors; } else { - std::string iso(entry_data.utf8_string, entry_data.data_size); - if (iso != "US") { - std::cerr << "FAIL: expected country iso_code 'US' for 8.8.8.8, got '" << iso << "'" << std::endl; - ++errors; - } else { - std::cout << " PASS: country/iso_code = " << iso << std::endl; - } + std::cout << " PASS: nested 8.8.8.8 country = " << country << std::endl; } - // Verify "country" -> "names" -> "en" path exists (not used by header_rewrite but validates structure) - status = MMDB_get_value(&result.entry, &entry_data, "country", "names", "en", NULL); - if (MMDB_SUCCESS != status || !entry_data.has_data || entry_data.type != MMDB_DATA_TYPE_UTF8_STRING) { - std::cerr << "FAIL: country/names/en lookup failed for 8.8.8.8" << std::endl; + country = lookup_country(mmdb, "1.2.3.4"); + if (country != "KR") { + std::cerr << "FAIL: nested schema 1.2.3.4 expected 'KR', got '" << country << "'" << std::endl; ++errors; } else { - std::string name(entry_data.utf8_string, entry_data.data_size); - std::cout << " PASS: country/names/en = " << name << std::endl; + std::cout << " PASS: nested 1.2.3.4 country = " << country << std::endl; } - // Verify loopback returns no entry - result = MMDB_lookup_string(&mmdb, "127.0.0.1", &gai_error, &mmdb_error); - if (gai_error != 0) { - std::cerr << "FAIL: getaddrinfo failed for 127.0.0.1: " << gai_strerror(gai_error) << std::endl; + uint32_t asn = lookup_asn(mmdb, "8.8.8.8"); + if (asn != 15169) { + std::cerr << "FAIL: nested schema 8.8.8.8 expected ASN 15169, got " << asn << std::endl; ++errors; - } else if (MMDB_SUCCESS == mmdb_error && result.found_entry) { - std::cerr << "FAIL: expected no entry for 127.0.0.1" << std::endl; + } else { + std::cout << " PASS: nested 8.8.8.8 ASN = " << asn << std::endl; + } + + country = lookup_country(mmdb, "127.0.0.1"); + if (country != "(lookup_failed)") { + std::cerr << "FAIL: nested schema 127.0.0.1 expected no entry, got '" << country << "'" << std::endl; ++errors; } else { - std::cout << " PASS: 127.0.0.1 correctly returns no entry" << std::endl; + std::cout << " PASS: nested 127.0.0.1 correctly not found" << std::endl; } MMDB_close(&mmdb); + return errors; +} - if (errors == 0) { - std::cout << "MaxMind geo tests passed" << std::endl; - } +int +test_maxmind_geo() +{ + int errors = 0; + errors += test_flat_schema(); + errors += test_nested_schema(); return errors; } #endif From 64344c8b67605f3eb8dc416ecdd8a3706f1320f6 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Thu, 12 Mar 2026 12:04:24 -0700 Subject: [PATCH 7/8] Move schema detection from runtime to initLibrary Probe the database at load time with well-known IPs (8.8.8.8, 1.1.1.1, 128.0.0.1) instead of detecting lazily on first lookup. Removes the need for std::atomic and simplifies the hot path. --- .../header_rewrite/conditions_geo_maxmind.cc | 71 ++++++++++--------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/plugins/header_rewrite/conditions_geo_maxmind.cc b/plugins/header_rewrite/conditions_geo_maxmind.cc index c7fc131a6f6..f9168a5a23a 100644 --- a/plugins/header_rewrite/conditions_geo_maxmind.cc +++ b/plugins/header_rewrite/conditions_geo_maxmind.cc @@ -23,7 +23,6 @@ #include #include -#include #include "ts/ts.h" @@ -33,8 +32,30 @@ MMDB_s *gMaxMindDB = nullptr; -enum class MmdbSchema : int { UNKNOWN = 0, NESTED = 1, FLAT = 2 }; -std::atomic gMmdbSchema{MmdbSchema::UNKNOWN}; +enum class MmdbSchema { NESTED, FLAT }; +static MmdbSchema gMmdbSchema = MmdbSchema::NESTED; + +// Detect whether the MMDB uses nested (GeoLite2) or flat (vendor) field layout +// by probing for the nested country path on a lookup result. +static MmdbSchema +detect_schema(MMDB_entry_s *entry) +{ + MMDB_entry_data_s probe; + int status = MMDB_get_value(entry, &probe, "country", "iso_code", NULL); + + if (MMDB_SUCCESS == status && probe.has_data && probe.type == MMDB_DATA_TYPE_UTF8_STRING) { + return MmdbSchema::NESTED; + } + + status = MMDB_get_value(entry, &probe, "country_code", NULL); + if (MMDB_SUCCESS == status && probe.has_data && probe.type == MMDB_DATA_TYPE_UTF8_STRING) { + return MmdbSchema::FLAT; + } + + return MmdbSchema::NESTED; +} + +static const char *probe_ips[] = {"8.8.8.8", "1.1.1.1", "128.0.0.1"}; void MMConditionGeo::initLibrary(const std::string &path) @@ -55,34 +76,23 @@ MMConditionGeo::initLibrary(const std::string &path) if (MMDB_SUCCESS != status) { Dbg(pi_dbg_ctl, "Cannot open %s - %s", path.c_str(), MMDB_strerror(status)); delete gMaxMindDB; - gMaxMindDB = nullptr; // avoid leaving a dangling global pointer after delete + gMaxMindDB = nullptr; return; } - Dbg(pi_dbg_ctl, "Loaded %s", path.c_str()); -} - -// Detect whether the MMDB uses nested (GeoLite2) or flat (vendor) field layout -// by probing for the nested country path on a real lookup result. Called once -// on the first successful lookup; result is cached in gMmdbSchema. -static MmdbSchema -detect_schema(MMDB_entry_s *entry) -{ - MMDB_entry_data_s probe; - int status = MMDB_get_value(entry, &probe, "country", "iso_code", NULL); - if (MMDB_SUCCESS == status && probe.has_data && probe.type == MMDB_DATA_TYPE_UTF8_STRING) { - Dbg(pi_dbg_ctl, "Detected nested MMDB schema (country/iso_code)"); - return MmdbSchema::NESTED; - } - - status = MMDB_get_value(entry, &probe, "country_code", NULL); - if (MMDB_SUCCESS == status && probe.has_data && probe.type == MMDB_DATA_TYPE_UTF8_STRING) { - Dbg(pi_dbg_ctl, "Detected flat MMDB schema (country_code)"); - return MmdbSchema::FLAT; + // Probe the database schema at load time so we know which field paths to + // use for country lookups. Try a few well-known IPs until one hits. + for (auto *ip : probe_ips) { + int gai_error, mmdb_error; + MMDB_lookup_result_s result = MMDB_lookup_string(gMaxMindDB, ip, &gai_error, &mmdb_error); + if (gai_error == 0 && MMDB_SUCCESS == mmdb_error && result.found_entry) { + gMmdbSchema = detect_schema(&result.entry); + Dbg(pi_dbg_ctl, "Loaded %s (schema: %s)", path.c_str(), gMmdbSchema == MmdbSchema::FLAT ? "flat" : "nested"); + return; + } } - Dbg(pi_dbg_ctl, "Could not detect MMDB schema; defaulting to nested"); - return MmdbSchema::NESTED; + Dbg(pi_dbg_ctl, "Loaded %s (schema: defaulting to nested, no probe IPs matched)", path.c_str()); } std::string @@ -108,19 +118,12 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const return ret; } - // Lazy schema detection on first successful lookup - MmdbSchema schema = gMmdbSchema.load(std::memory_order_relaxed); - if (schema == MmdbSchema::UNKNOWN) { - schema = detect_schema(&result.entry); - gMmdbSchema.store(schema, std::memory_order_relaxed); - } - MMDB_entry_data_s entry_data; int status; switch (_geo_qual) { case GEO_QUAL_COUNTRY: - if (schema == MmdbSchema::FLAT) { + if (gMmdbSchema == MmdbSchema::FLAT) { status = MMDB_get_value(&result.entry, &entry_data, "country_code", NULL); } else { status = MMDB_get_value(&result.entry, &entry_data, "country", "iso_code", NULL); From 340aa8e68b0241de85090b0b3e39e41ea495b9b6 Mon Sep 17 00:00:00 2001 From: Bryan Call Date: Thu, 12 Mar 2026 16:08:52 -0700 Subject: [PATCH 8/8] Address Copilot review feedback on test infrastructure - Check for mmdb-writer/netaddr Python modules at CMake configure time before adding the MMDB generation target (prevents Ninja failures when declared outputs are not produced) - Exit non-zero from generate_test_mmdb.py on missing dependencies - Fix skip message to mention both required packages (mmdb-writer netaddr) - Add missing include for getenv() - Remove unused and includes from test file --- plugins/header_rewrite/CMakeLists.txt | 43 +++++++++++-------- plugins/header_rewrite/generate_test_mmdb.py | 6 +-- plugins/header_rewrite/header_rewrite_test.cc | 3 +- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/plugins/header_rewrite/CMakeLists.txt b/plugins/header_rewrite/CMakeLists.txt index 5ee1fdd1991..90ff5156c75 100644 --- a/plugins/header_rewrite/CMakeLists.txt +++ b/plugins/header_rewrite/CMakeLists.txt @@ -69,24 +69,33 @@ if(BUILD_TESTING) QUIET ) if(Python3_FOUND) - set(_mmdb_test_dir "${CMAKE_CURRENT_BINARY_DIR}/test_mmdb") - add_custom_command( - OUTPUT "${_mmdb_test_dir}/test_flat_geo.mmdb" "${_mmdb_test_dir}/test_nested_geo.mmdb" - COMMAND ${CMAKE_COMMAND} -E make_directory "${_mmdb_test_dir}" - COMMAND ${Python3_EXECUTABLE} "${CMAKE_CURRENT_SOURCE_DIR}/generate_test_mmdb.py" "${_mmdb_test_dir}" - DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/generate_test_mmdb.py" - COMMENT "Generating test MMDB files for header_rewrite" - ) - add_custom_target( - generate_test_mmdb DEPENDS "${_mmdb_test_dir}/test_flat_geo.mmdb" "${_mmdb_test_dir}/test_nested_geo.mmdb" - ) - add_dependencies(test_header_rewrite generate_test_mmdb) - set_tests_properties( - test_header_rewrite - PROPERTIES - ENVIRONMENT - "MMDB_TEST_FLAT=${_mmdb_test_dir}/test_flat_geo.mmdb;MMDB_TEST_NESTED=${_mmdb_test_dir}/test_nested_geo.mmdb" + execute_process( + COMMAND "${Python3_EXECUTABLE}" -c "import mmdb_writer; import netaddr" + RESULT_VARIABLE _mmdb_python_result + OUTPUT_QUIET ERROR_QUIET ) + if(_mmdb_python_result EQUAL 0) + set(_mmdb_test_dir "${CMAKE_CURRENT_BINARY_DIR}/test_mmdb") + add_custom_command( + OUTPUT "${_mmdb_test_dir}/test_flat_geo.mmdb" "${_mmdb_test_dir}/test_nested_geo.mmdb" + COMMAND ${CMAKE_COMMAND} -E make_directory "${_mmdb_test_dir}" + COMMAND ${Python3_EXECUTABLE} "${CMAKE_CURRENT_SOURCE_DIR}/generate_test_mmdb.py" "${_mmdb_test_dir}" + DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/generate_test_mmdb.py" + COMMENT "Generating test MMDB files for header_rewrite" + ) + add_custom_target( + generate_test_mmdb DEPENDS "${_mmdb_test_dir}/test_flat_geo.mmdb" "${_mmdb_test_dir}/test_nested_geo.mmdb" + ) + add_dependencies(test_header_rewrite generate_test_mmdb) + set_tests_properties( + test_header_rewrite + PROPERTIES + ENVIRONMENT + "MMDB_TEST_FLAT=${_mmdb_test_dir}/test_flat_geo.mmdb;MMDB_TEST_NESTED=${_mmdb_test_dir}/test_nested_geo.mmdb" + ) + else() + message(STATUS "Python modules 'mmdb-writer'/'netaddr' not found; skipping test MMDB generation") + endif() endif() endif() diff --git a/plugins/header_rewrite/generate_test_mmdb.py b/plugins/header_rewrite/generate_test_mmdb.py index efdba213267..8cd34ac8a41 100644 --- a/plugins/header_rewrite/generate_test_mmdb.py +++ b/plugins/header_rewrite/generate_test_mmdb.py @@ -26,7 +26,7 @@ This script generates one MMDB file for each schema so the C++ test can verify that auto-detection works for both. -Requires: pip install mmdb-writer +Requires: pip install mmdb-writer netaddr """ import os @@ -36,8 +36,8 @@ from mmdb_writer import MMDBWriter, MmdbU32 import netaddr except ImportError: - print("SKIP: mmdb-writer or netaddr not installed (pip install mmdb-writer)", file=sys.stderr) - sys.exit(0) + print("SKIP: mmdb-writer or netaddr not installed (pip install mmdb-writer netaddr)", file=sys.stderr) + sys.exit(1) def net(cidr): diff --git a/plugins/header_rewrite/header_rewrite_test.cc b/plugins/header_rewrite/header_rewrite_test.cc index c8cc2741993..6f6026e3df3 100644 --- a/plugins/header_rewrite/header_rewrite_test.cc +++ b/plugins/header_rewrite/header_rewrite_test.cc @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -30,8 +31,6 @@ #if TS_USE_HRW_MAXMINDDB #include -#include -#include #endif namespace header_rewrite_ns