[trafficserver] branch master updated: Fix log filters for IP conditions. (#8249)

[bn...@apache.org]

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

bnolsen 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 945452d  Fix log filters for IP conditions. (#8249)
945452d is described below

commit 945452d82eac3ccbc78d2ec3bd652562391b3ca6
Author: Brian Neradt <br...@gmail.com>
AuthorDate: Wed Aug 18 07:52:46 2021 -0500

    Fix log filters for IP conditions. (#8249)
    
    Our log filter mechanism for IP addresses did not compare the filter's
    specified IP address with the log field's IP address correctly. This
    fixes that matching mechanism.
    
    Fixes #6405
---
 proxy/logging/LogFilter.cc                  | 16 +++++++++++-----
 tests/gold_tests/logging/log-filter.test.py | 28 ++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/proxy/logging/LogFilter.cc b/proxy/logging/LogFilter.cc
index c15e02f..0dcfcd3 100644
--- a/proxy/logging/LogFilter.cc
+++ b/proxy/logging/LogFilter.cc
@@ -833,11 +833,17 @@ LogFilterIP::is_match(LogAccess *lad)
   bool zret = false;
 
   if (m_field && lad) {
-    LogFieldIpStorage value;
-    m_field->marshal(lad, reinterpret_cast<char *>(&value));
-    // This is bad, we abuse the fact that the initial layout of LogFieldIpStorage and IpAddr
-    // are identical. We should look at converting the log stuff to use IpAddr directly.
-    zret = m_map.contains(reinterpret_cast<IpAddr &>(value));
+    LogFieldIpStorage field_ip_storage;
+    m_field->marshal(lad, reinterpret_cast<char *>(&field_ip_storage));
+
+    // Convert to the IpAddr type that the map holds.
+    IpAddr field_ip;
+    if (field_ip_storage._ip._family == AF_INET) {
+      field_ip = field_ip_storage._ip4._addr;
+    } else if (field_ip_storage._ip._family == AF_INET6) {
+      field_ip = field_ip_storage._ip6._addr;
+    }
+    zret = m_map.contains(reinterpret_cast<IpAddr &>(field_ip));
   }
 
   return zret;
diff --git a/tests/gold_tests/logging/log-filter.test.py b/tests/gold_tests/logging/log-filter.test.py
index ee1f6b3..1257680 100644
--- a/tests/gold_tests/logging/log-filter.test.py
+++ b/tests/gold_tests/logging/log-filter.test.py
@@ -28,6 +28,9 @@ server = Test.MakeVerifierServerProcess("server", replay_file)
 nameserver = Test.MakeDNServer("dns", default='127.0.0.1')
 
 ts.Disk.records_config.update({
+    'proxy.config.diags.debug.enabled': 1,
+    'proxy.config.diags.debug.tags': 'log',
+
     'proxy.config.net.connections_throttle': 100,
     'proxy.config.dns.nameservers': f"127.0.0.1:{nameserver.Variables.Port}",
     'proxy.config.dns.resolv_conf': 'NULL'
@@ -41,17 +44,34 @@ ts.Disk.logging_yaml.AddLines(
     '''
 logging:
   filters:
+    - name: only_localhost
+      action: accept
+      condition: chi MATCH 127.0.0.1
+
+    - name: not_localhost
+      action: accept
+      condition: chi MATCH 3.3.3.3
+
     - name: queryparamescaper_cquuc
       action: WIPE_FIELD_VALUE
       condition: cquuc CASE_INSENSITIVE_CONTAIN password,secret,access_token,session_redirect,cardNumber,code,query,search-query,prefix,keywords,email,handle
+
   formats:
     - name: custom
       format: '%<cquuc>'
+
   logs:
     - filename: filter-test
       format: custom
       filters:
       - queryparamescaper_cquuc
+      - only_localhost
+
+    - filename: should-not-be-written
+      format: custom
+      filters:
+      - queryparamescaper_cquuc
+      - not_localhost
 '''.split("\n")
 )
 
@@ -75,3 +95,11 @@ test_run.Processes.Default.Command = (
     os.path.join(ts.Variables.LOGDIR, 'filter-test.log')
 )
 test_run.Processes.Default.ReturnCode = 0
+
+# We already waited for the above, so we don't have to wait for this one.
+test_run = Test.AddTestRun()
+test_run.Processes.Default.Command = (
+    os.path.join(Test.Variables.AtsTestToolsDir, 'condwait') + ' 1 1 -f ' +
+    os.path.join(ts.Variables.LOGDIR, 'should-not-be-written.log')
+)
+test_run.Processes.Default.ReturnCode = 1