You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/08/14 21:52:55 UTC

[GitHub] [trafficserver] bneradt opened a new pull request #8249: Fix log filteris for IP conditions.

bneradt opened a new pull request #8249:
URL: https://github.com/apache/trafficserver/pull/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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] bneradt commented on a change in pull request #8249: Fix log filters for IP conditions.

Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #8249:
URL: https://github.com/apache/trafficserver/pull/8249#discussion_r691505034



##########
File path: 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));

Review comment:
       Ah, yeah. Left over from the previous version of this that had to cast from the storage type.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] bneradt commented on a change in pull request #8249: Fix log filters for IP conditions.

Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #8249:
URL: https://github.com/apache/trafficserver/pull/8249#discussion_r691586409



##########
File path: 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));

Review comment:
       https://github.com/apache/trafficserver/pull/8259




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #8249: Fix log filters for IP conditions.

Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #8249:
URL: https://github.com/apache/trafficserver/pull/8249#discussion_r691392993



##########
File path: 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));

Review comment:
       Why are you casting an `IpAddr` to an `IpAddr`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] SolidWallOfCode commented on a change in pull request #8249: Fix log filters for IP conditions.

Posted by GitBox <gi...@apache.org>.
SolidWallOfCode commented on a change in pull request #8249:
URL: https://github.com/apache/trafficserver/pull/8249#discussion_r691392993



##########
File path: 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));

Review comment:
       Why are you casting an `IpAddr` to an `IpAddr`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] traeak merged pull request #8249: Fix log filters for IP conditions.

Posted by GitBox <gi...@apache.org>.
traeak merged pull request #8249:
URL: https://github.com/apache/trafficserver/pull/8249


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] traeak merged pull request #8249: Fix log filters for IP conditions.

Posted by GitBox <gi...@apache.org>.
traeak merged pull request #8249:
URL: https://github.com/apache/trafficserver/pull/8249


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org