[trafficserver] branch master updated: header_rewrite: Various fixes for MaxMind support (#7746)

[rr...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rrm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 1b5139d  header_rewrite: Various fixes for MaxMind support (#7746)
1b5139d is described below

commit 1b5139d19f9707edf40cd6865f948b1ff235b84c
Author: Randall Meyer <rr...@apache.org>
AuthorDate: Tue Apr 27 18:31:30 2021 -0700

    header_rewrite: Various fixes for MaxMind support (#7746)
    
    Including:
    * Demote scary errors and other log changes
    * ASN fieldname correction
---
 plugins/header_rewrite/conditions_geo_maxmind.cc | 13 ++++++++++--
 plugins/header_rewrite/header_rewrite.cc         | 27 +++++++++++++++++-------
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/plugins/header_rewrite/conditions_geo_maxmind.cc b/plugins/header_rewrite/conditions_geo_maxmind.cc
index 8438e1c..5e0e76c 100644
--- a/plugins/header_rewrite/conditions_geo_maxmind.cc
+++ b/plugins/header_rewrite/conditions_geo_maxmind.cc
@@ -36,14 +36,21 @@ void
 MMConditionGeo::initLibrary(const std::string &path)
 {
   if (path.empty()) {
-    TSError("[%s] Empty db path specified. Not initializing!", PLUGIN_NAME);
+    TSDebug(PLUGIN_NAME, "Empty MaxMind db path specified. Not initializing!");
     return;
   }
+
+  if (gMaxMindDB != nullptr) {
+    TSDebug(PLUGIN_NAME, "Maxmind library already initialized");
+    return;
+  }
+
   gMaxMindDB = new MMDB_s;
 
   int status = MMDB_open(path.c_str(), MMDB_MODE_MMAP, gMaxMindDB);
   if (MMDB_SUCCESS != status) {
     TSDebug(PLUGIN_NAME, "Cannot open %s - %s", path.c_str(), MMDB_strerror(status));
+    delete gMaxMindDB;
     return;
   }
   TSDebug(PLUGIN_NAME, "Loaded %s", path.c_str());
@@ -56,6 +63,7 @@ MMConditionGeo::get_geo_string(const sockaddr *addr) const
   int mmdb_error;
 
   if (gMaxMindDB == nullptr) {
+    TSDebug(PLUGIN_NAME, "MaxMind not initialized; using default value");
     return ret;
   }
 
@@ -120,6 +128,7 @@ MMConditionGeo::get_geo_int(const sockaddr *addr) const
   int mmdb_error;
 
   if (gMaxMindDB == nullptr) {
+    TSDebug(PLUGIN_NAME, "MaxMind not initialized; using default value");
     return ret;
   }
 
@@ -150,7 +159,7 @@ MMConditionGeo::get_geo_int(const sockaddr *addr) const
   const char *field_name;
   switch (_geo_qual) {
   case GEO_QUAL_ASN:
-    field_name = "autonomous_system";
+    field_name = "autonomous_system_number";
     break;
   default:
     TSDebug(PLUGIN_NAME, "Unsupported field %d", _geo_qual);
diff --git a/plugins/header_rewrite/header_rewrite.cc b/plugins/header_rewrite/header_rewrite.cc
index 78e9f42..09593ea 100644
--- a/plugins/header_rewrite/header_rewrite.cc
+++ b/plugins/header_rewrite/header_rewrite.cc
@@ -40,6 +40,7 @@ static std::once_flag initGeoLibs;
 static void
 initGeoLib(const std::string &dbPath)
 {
+  TSDebug(PLUGIN_NAME, "Loading geo db %s", dbPath.c_str());
 #if TS_USE_HRW_GEOIP
   GeoIPConditionGeo::initLibrary(dbPath);
 #elif TS_USE_HRW_MAXMINDDB
@@ -334,13 +335,19 @@ TSPluginInit(int argc, const char *argv[])
     }
   }
 
+  if (!geoDBpath.empty() && geoDBpath.find("/") != 0) {
+    geoDBpath = std::string(TSConfigDirGet()) + '/' + geoDBpath;
+  }
+
+  TSDebug(PLUGIN_NAME, "Global geo db %s", geoDBpath.c_str());
+
+  std::call_once(initGeoLibs, [&geoDBpath]() { initGeoLib(geoDBpath); });
+
   // Parse the global config file(s). All rules are just appended
   // to the "global" Rules configuration.
   RulesConfig *conf = new RulesConfig;
   bool got_config   = false;
 
-  std::call_once(initGeoLibs, [&geoDBpath]() { initGeoLib(geoDBpath); });
-
   for (int i = optind; i < argc; ++i) {
     // Parse the config file(s). Note that multiple config files are
     // just appended to the configurations.
@@ -412,13 +419,13 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE
   --argc;
   ++argv;
 
-  std::string geoDBPath;
+  std::string geoDBpath;
   while (true) {
     int opt = getopt_long(argc, (char *const *)argv, "m:", longopt, NULL);
 
     switch (opt) {
     case 'm': {
-      geoDBPath = optarg;
+      geoDBpath = optarg;
     } break;
     }
     if (opt == -1) {
@@ -426,11 +433,15 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE
     }
   }
 
-  if (geoDBPath.find("/") != 0) {
-    geoDBPath = std::string(TSConfigDirGet()) + '/' + geoDBPath;
-  }
+  if (!geoDBpath.empty()) {
+    if (geoDBpath.find("/") != 0) {
+      geoDBpath = std::string(TSConfigDirGet()) + '/' + geoDBpath;
+    }
+
+    TSDebug(PLUGIN_NAME, "Remap geo db %s", geoDBpath.c_str());
 
-  std::call_once(initGeoLibs, [&geoDBPath]() { initGeoLib(geoDBPath); });
+    std::call_once(initGeoLibs, [&geoDBpath]() { initGeoLib(geoDBpath); });
+  }
 
   RulesConfig *conf = new RulesConfig;