You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by su...@apache.org on 2019/07/31 23:37:06 UTC

[trafficserver] branch master updated (9772eb6 -> e068b76)

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

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


    from 9772eb6  Add soft limit for HTTP Request URI and Header field length. Add a default body_factory template when rejecting a request that's too long
     new 1df0470  Preserve the raw log fields when wiping using case insensitive contains
     new e068b76  Add support for updating Container fields as well

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 proxy/logging/LogAccess.cc | 49 ++++++++++++++++++++++++++++++++++++++++++++++
 proxy/logging/LogAccess.h  |  1 +
 proxy/logging/LogField.cc  | 37 ++++++++++++++++++++++++++++++++--
 proxy/logging/LogField.h   |  2 ++
 proxy/logging/LogFilter.cc | 26 ++++++++++++++++--------
 proxy/logging/LogFilter.h  | 42 +++++++++++++++++++++------------------
 6 files changed, 128 insertions(+), 29 deletions(-)


[trafficserver] 01/02: Preserve the raw log fields when wiping using case insensitive contains

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 1df04706d5f28ec11fdebdfc1d03b6df7e46673b
Author: Sudheer Vinukonda <su...@apache.org>
AuthorDate: Thu Jul 25 16:58:25 2019 -0700

    Preserve the raw log fields when wiping using case insensitive contains
    
    Fix clang error
---
 proxy/logging/LogFilter.cc | 26 ++++++++++++++++++--------
 proxy/logging/LogFilter.h  | 40 ++++++++++++++++++++++------------------
 2 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/proxy/logging/LogFilter.cc b/proxy/logging/LogFilter.cc
index b80887d..bebab28 100644
--- a/proxy/logging/LogFilter.cc
+++ b/proxy/logging/LogFilter.cc
@@ -313,9 +313,12 @@ LogFilterString::wipe_this_entry(LogAccess *lad)
 
   static const unsigned BUFSIZE = 1024;
   char small_buf[BUFSIZE];
-  char *big_buf    = nullptr;
-  char *buf        = small_buf;
-  size_t marsh_len = m_field->marshal_len(lad); // includes null termination
+  char small_buf_upper[BUFSIZE];
+  char *big_buf       = nullptr;
+  char *big_buf_upper = nullptr;
+  char *buf           = small_buf;
+  char *buf_upper     = small_buf_upper;
+  size_t marsh_len    = m_field->marshal_len(lad); // includes null termination
 
   if (marsh_len > BUFSIZE) {
     big_buf = (char *)ats_malloc(marsh_len);
@@ -339,19 +342,25 @@ LogFilterString::wipe_this_entry(LogAccess *lad)
     // actual length, so we just use the fact that a MATCH is not possible
     // when marsh_len <= (length of the filter string)
     //
-    cond_satisfied = _checkConditionAndWipe(&strcmp, &buf, marsh_len, m_value, DATA_LENGTH_LARGER);
+    cond_satisfied = _checkConditionAndWipe(&strcmp, &buf, marsh_len, m_value, nullptr, DATA_LENGTH_LARGER);
     break;
   case CASE_INSENSITIVE_MATCH:
-    cond_satisfied = _checkConditionAndWipe(&strcasecmp, &buf, marsh_len, m_value, DATA_LENGTH_LARGER);
+    cond_satisfied = _checkConditionAndWipe(&strcasecmp, &buf, marsh_len, m_value, nullptr, DATA_LENGTH_LARGER);
     break;
   case CONTAIN:
-    cond_satisfied = _checkConditionAndWipe(&_isSubstring, &buf, marsh_len, m_value, DATA_LENGTH_LARGER);
+    cond_satisfied = _checkConditionAndWipe(&_isSubstring, &buf, marsh_len, m_value, nullptr, DATA_LENGTH_LARGER);
     break;
   case CASE_INSENSITIVE_CONTAIN:
+    if (big_buf) {
+      big_buf_upper = (char *)ats_malloc((unsigned int)marsh_len);
+      buf_upper     = big_buf_upper;
+    } else {
+      buf = small_buf; // make clang happy
+    }
     for (size_t i = 0; i < marsh_len; i++) {
-      buf[i] = ParseRules::ink_toupper(buf[i]);
+      buf_upper[i] = ParseRules::ink_toupper(buf[i]);
     }
-    cond_satisfied = _checkConditionAndWipe(&_isSubstring, &buf, marsh_len, m_value_uppercase, DATA_LENGTH_LARGER);
+    cond_satisfied = _checkConditionAndWipe(&_isSubstring, &buf_upper, marsh_len, m_value_uppercase, &buf, DATA_LENGTH_LARGER);
     break;
   default:
     ink_assert(!"INVALID FILTER OPERATOR");
@@ -360,6 +369,7 @@ LogFilterString::wipe_this_entry(LogAccess *lad)
   m_field->updateField(lad, buf, strlen(buf));
 
   ats_free(big_buf);
+  ats_free(big_buf_upper);
   return cond_satisfied;
 }
 
diff --git a/proxy/logging/LogFilter.h b/proxy/logging/LogFilter.h
index 4fe7c8e..45a2440 100644
--- a/proxy/logging/LogFilter.h
+++ b/proxy/logging/LogFilter.h
@@ -171,7 +171,7 @@ private:
                               LengthCondition lc);
 
   inline bool _checkConditionAndWipe(OperatorFunction f, char **field_value, size_t field_value_length, char **val,
-                                     LengthCondition lc);
+                                     char **orig_field_value, LengthCondition lc);
 
   // -- member functions that are not allowed --
   LogFilterString();
@@ -388,24 +388,28 @@ LogFilterString::_checkCondition(OperatorFunction f, const char *field_value, si
 
 --------------------------------------------------------------------------*/
 static void
-wipeField(char **dest, char *field)
+wipeField(char **dest, char *field, char **orig_dest)
 {
-  char *buf_dest = *dest;
+  char *buf_dest      = *dest;
+  char *buf_orig_dest = orig_dest ? *orig_dest : *dest;
 
   if (buf_dest) {
-    char *query_param = strstr(buf_dest, "?");
+    char *query_param      = strstr(buf_dest, "?");
+    char *orig_query_param = strstr(buf_orig_dest, "?");
 
-    if (!query_param) {
+    if (!query_param || !orig_query_param) {
       return;
     }
 
-    char *p1 = strstr(query_param, field);
+    char *p1      = strstr(query_param, field);
+    int field_pos = p1 - query_param;
+    p1            = orig_query_param + field_pos;
 
     if (p1) {
-      char tmp_text[strlen(buf_dest) + 10];
+      char tmp_text[strlen(buf_orig_dest) + 10];
       char *temp_text = tmp_text;
-      memcpy(temp_text, buf_dest, (p1 - buf_dest));
-      temp_text += (p1 - buf_dest);
+      memcpy(temp_text, buf_orig_dest, (p1 - buf_orig_dest));
+      temp_text += (p1 - buf_orig_dest);
       char *p2 = strstr(p1, "=");
       if (p2) {
         p2++;
@@ -417,9 +421,9 @@ wipeField(char **dest, char *field)
             temp_text[i] = 'X';
           }
           temp_text += (p3 - p2);
-          memcpy(temp_text, p3, ((buf_dest + strlen(buf_dest)) - p3));
+          memcpy(temp_text, p3, ((buf_orig_dest + strlen(buf_orig_dest)) - p3));
         } else {
-          for (int i = 0; i < ((buf_dest + strlen(buf_dest)) - p2); i++) {
+          for (int i = 0; i < ((buf_orig_dest + strlen(buf_orig_dest)) - p2); i++) {
             temp_text[i] = 'X';
           }
         }
@@ -427,8 +431,8 @@ wipeField(char **dest, char *field)
         return;
       }
 
-      tmp_text[strlen(buf_dest)] = '\0';
-      strcpy(*dest, tmp_text);
+      tmp_text[strlen(buf_orig_dest)] = '\0';
+      orig_dest ? strcpy(*orig_dest, tmp_text) : strcpy(*dest, tmp_text);
     }
   }
 }
@@ -454,7 +458,7 @@ wipeField(char **dest, char *field)
 
 inline bool
 LogFilterString::_checkConditionAndWipe(OperatorFunction f, char **field_value, size_t field_value_length, char **val,
-                                        LengthCondition lc)
+                                        char **orig_field_value, LengthCondition lc)
 {
   bool retVal = false;
 
@@ -469,13 +473,13 @@ LogFilterString::_checkConditionAndWipe(OperatorFunction f, char **field_value,
     case DATA_LENGTH_EQUAL:
       retVal = (field_value_length == *m_length ? ((*f)(*field_value, *val) == 0 ? true : false) : false);
       if (retVal) {
-        wipeField(field_value, *val);
+        wipeField(field_value, *val, orig_field_value);
       }
       break;
     case DATA_LENGTH_LARGER:
       retVal = (field_value_length > *m_length ? ((*f)(*field_value, *val) == 0 ? true : false) : false);
       if (retVal) {
-        wipeField(field_value, *val);
+        wipeField(field_value, *val, orig_field_value);
       }
       break;
     default:
@@ -490,7 +494,7 @@ LogFilterString::_checkConditionAndWipe(OperatorFunction f, char **field_value,
         // condition is satisfied if f returns zero
         if (field_value_length == m_length[i] && (*f)(*field_value, val[i]) == 0) {
           retVal = true;
-          wipeField(field_value, val[i]);
+          wipeField(field_value, val[i], orig_field_value);
         }
       }
       break;
@@ -499,7 +503,7 @@ LogFilterString::_checkConditionAndWipe(OperatorFunction f, char **field_value,
         // condition is satisfied if f returns zero
         if (field_value_length > m_length[i] && (*f)(*field_value, val[i]) == 0) {
           retVal = true;
-          wipeField(field_value, val[i]);
+          wipeField(field_value, val[i], orig_field_value);
         }
       }
       break;


[trafficserver] 02/02: Add support for updating Container fields as well

Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit e068b7685f01594b8faa48792459f875b5e5c3ad
Author: Sudheer Vinukonda <su...@apache.org>
AuthorDate: Mon Jul 29 14:41:11 2019 -0700

    Add support for updating Container fields as well
    
    Minor optimization
    
    Refactor argument names and signature per review comments
    
    Fix compile error after running clang-format
    
    Fix argument names to better clarify what they represent
---
 proxy/logging/LogAccess.cc | 49 +++++++++++++++++++++++++++++++++++++++
 proxy/logging/LogAccess.h  |  1 +
 proxy/logging/LogField.cc  | 37 +++++++++++++++++++++++++++--
 proxy/logging/LogField.h   |  2 ++
 proxy/logging/LogFilter.cc |  2 +-
 proxy/logging/LogFilter.h  | 58 +++++++++++++++++++++++-----------------------
 6 files changed, 117 insertions(+), 32 deletions(-)

diff --git a/proxy/logging/LogAccess.cc b/proxy/logging/LogAccess.cc
index 91f6c7f..ec8d781 100644
--- a/proxy/logging/LogAccess.cc
+++ b/proxy/logging/LogAccess.cc
@@ -2796,3 +2796,52 @@ LogAccess::marshal_milestone_diff(TSMilestonesType ms1, TSMilestonesType ms2, ch
   }
   return INK_MIN_ALIGN;
 }
+
+void
+LogAccess::set_http_header_field(LogField::Container container, char *field, char *buf, int len)
+{
+  HTTPHdr *header;
+
+  switch (container) {
+  case LogField::CQH:
+  case LogField::ECQH:
+    header = m_client_request;
+    break;
+
+  case LogField::PSH:
+  case LogField::EPSH:
+    header = m_proxy_response;
+    break;
+
+  case LogField::PQH:
+  case LogField::EPQH:
+    header = m_proxy_request;
+    break;
+
+  case LogField::SSH:
+  case LogField::ESSH:
+    header = m_server_response;
+    break;
+
+  case LogField::CSSH:
+  case LogField::ECSSH:
+    header = m_cache_response;
+    break;
+
+  default:
+    header = nullptr;
+    break;
+  }
+
+  if (header && buf) {
+    MIMEField *fld = header->field_find(field, (int)::strlen(field));
+    if (fld) {
+      // Loop over dups, update each of them
+      //
+      while (fld) {
+        header->value_set((const char *)field, (int)::strlen(field), buf, len);
+        fld = fld->m_next_dup;
+      }
+    }
+  }
+}
diff --git a/proxy/logging/LogAccess.h b/proxy/logging/LogAccess.h
index 264ccea..1a8a2ef 100644
--- a/proxy/logging/LogAccess.h
+++ b/proxy/logging/LogAccess.h
@@ -280,6 +280,7 @@ public:
   inkcoreapi int marshal_milestone_fmt_time(TSMilestonesType ms, char *buf);
   inkcoreapi int marshal_milestone_fmt_ms(TSMilestonesType ms, char *buf);
   inkcoreapi int marshal_milestone_diff(TSMilestonesType ms1, TSMilestonesType ms2, char *buf);
+  inkcoreapi void set_http_header_field(LogField::Container container, char *field, char *buf, int len);
   //
   // unmarshalling routines
   //
diff --git a/proxy/logging/LogField.cc b/proxy/logging/LogField.cc
index 563638d..fab35da 100644
--- a/proxy/logging/LogField.cc
+++ b/proxy/logging/LogField.cc
@@ -29,6 +29,7 @@
  ***************************************************************************/
 #include "tscore/ink_platform.h"
 
+#include "MIME.h"
 #include "LogUtils.h"
 #include "LogField.h"
 #include "LogBuffer.h"
@@ -326,7 +327,7 @@ LogField::LogField(const char *field, Container container, SetFunc _setfunc)
     m_milestone2(TS_MILESTONE_LAST_ENTRY),
     m_time_field(false),
     m_alias_map(nullptr),
-    m_set_func(_setfunc)
+    m_set_func(nullptr)
 {
   ink_assert(m_name != nullptr);
   ink_assert(m_symbol != nullptr);
@@ -462,13 +463,39 @@ LogField::marshal_len(LogAccess *lad)
   }
 }
 
+bool
+LogField::isContainerUpdateFieldSupported(Container container)
+{
+  switch (container) {
+  case CQH:
+  case PSH:
+  case PQH:
+  case SSH:
+  case CSSH:
+  case ECQH:
+  case EPSH:
+  case EPQH:
+  case ESSH:
+  case ECSSH:
+  case SCFG:
+    return true;
+  default:
+    return false;
+  }
+}
+
 void
 LogField::updateField(LogAccess *lad, char *buf, int len)
 {
   if (m_container == NO_CONTAINER) {
     return (lad->*m_set_func)(buf, len);
+  } else {
+    if (isContainerUpdateFieldSupported(m_container)) {
+      return set_http_header_field(lad, m_container, this->m_name, buf, len);
+    } else {
+      // no set function defined for the container
+    }
   }
-  // else...// future enhancement
 }
 
 /*-------------------------------------------------------------------------
@@ -694,6 +721,12 @@ LogField::fieldlist_contains_aggregates(const char *fieldlist)
   return false;
 }
 
+void
+LogField::set_http_header_field(LogAccess *lad, LogField::Container container, char *field, char *buf, int len)
+{
+  return lad->set_http_header_field(container, field, buf, len);
+}
+
 /*-------------------------------------------------------------------------
   LogFieldList
 
diff --git a/proxy/logging/LogField.h b/proxy/logging/LogField.h
index e02dba3..5b19aac 100644
--- a/proxy/logging/LogField.h
+++ b/proxy/logging/LogField.h
@@ -174,6 +174,7 @@ public:
     return m_time_field;
   }
 
+  inkcoreapi void set_http_header_field(LogAccess *lad, LogField::Container container, char *field, char *buf, int len);
   void set_aggregate_op(Aggregate agg_op);
   void update_aggregate(int64_t val);
 
@@ -181,6 +182,7 @@ public:
   static Container valid_container_name(char *name);
   static Aggregate valid_aggregate_name(char *name);
   static bool fieldlist_contains_aggregates(const char *fieldlist);
+  static bool isContainerUpdateFieldSupported(Container container);
 
 private:
   char *m_name;
diff --git a/proxy/logging/LogFilter.cc b/proxy/logging/LogFilter.cc
index bebab28..685ccdb 100644
--- a/proxy/logging/LogFilter.cc
+++ b/proxy/logging/LogFilter.cc
@@ -360,7 +360,7 @@ LogFilterString::wipe_this_entry(LogAccess *lad)
     for (size_t i = 0; i < marsh_len; i++) {
       buf_upper[i] = ParseRules::ink_toupper(buf[i]);
     }
-    cond_satisfied = _checkConditionAndWipe(&_isSubstring, &buf_upper, marsh_len, m_value_uppercase, &buf, DATA_LENGTH_LARGER);
+    cond_satisfied = _checkConditionAndWipe(&_isSubstring, &buf, marsh_len, m_value_uppercase, buf_upper, DATA_LENGTH_LARGER);
     break;
   default:
     ink_assert(!"INVALID FILTER OPERATOR");
diff --git a/proxy/logging/LogFilter.h b/proxy/logging/LogFilter.h
index 45a2440..1cb55fe 100644
--- a/proxy/logging/LogFilter.h
+++ b/proxy/logging/LogFilter.h
@@ -171,7 +171,7 @@ private:
                               LengthCondition lc);
 
   inline bool _checkConditionAndWipe(OperatorFunction f, char **field_value, size_t field_value_length, char **val,
-                                     char **orig_field_value, LengthCondition lc);
+                                     const char *uppercase_field_value, LengthCondition lc);
 
   // -- member functions that are not allowed --
   LogFilterString();
@@ -388,42 +388,41 @@ LogFilterString::_checkCondition(OperatorFunction f, const char *field_value, si
 
 --------------------------------------------------------------------------*/
 static void
-wipeField(char **dest, char *field, char **orig_dest)
+wipeField(char **field, char *pattern, const char *uppercase_field)
 {
-  char *buf_dest      = *dest;
-  char *buf_orig_dest = orig_dest ? *orig_dest : *dest;
+  char *buf_dest          = *field;
+  const char *lookup_dest = uppercase_field ? uppercase_field : *field;
 
   if (buf_dest) {
-    char *query_param      = strstr(buf_dest, "?");
-    char *orig_query_param = strstr(buf_orig_dest, "?");
-
-    if (!query_param || !orig_query_param) {
+    char *query_param              = strstr(buf_dest, "?");
+    const char *lookup_query_param = strstr(lookup_dest, "?");
+    if (!query_param || !lookup_query_param) {
       return;
     }
 
-    char *p1      = strstr(query_param, field);
-    int field_pos = p1 - query_param;
-    p1            = orig_query_param + field_pos;
+    const char *p1 = strstr(lookup_query_param, pattern);
+    int field_pos  = p1 - lookup_query_param;
+    p1             = query_param + field_pos;
 
     if (p1) {
-      char tmp_text[strlen(buf_orig_dest) + 10];
+      char tmp_text[strlen(buf_dest) + 10];
       char *temp_text = tmp_text;
-      memcpy(temp_text, buf_orig_dest, (p1 - buf_orig_dest));
-      temp_text += (p1 - buf_orig_dest);
-      char *p2 = strstr(p1, "=");
+      memcpy(temp_text, buf_dest, (p1 - buf_dest));
+      temp_text += (p1 - buf_dest);
+      const char *p2 = strstr(p1, "=");
       if (p2) {
         p2++;
         memcpy(temp_text, p1, (p2 - p1));
         temp_text += (p2 - p1);
-        char *p3 = strstr(p2, "&");
+        const char *p3 = strstr(p2, "&");
         if (p3) {
           for (int i = 0; i < (p3 - p2); i++) {
             temp_text[i] = 'X';
           }
           temp_text += (p3 - p2);
-          memcpy(temp_text, p3, ((buf_orig_dest + strlen(buf_orig_dest)) - p3));
+          memcpy(temp_text, p3, ((buf_dest + strlen(buf_dest)) - p3));
         } else {
-          for (int i = 0; i < ((buf_orig_dest + strlen(buf_orig_dest)) - p2); i++) {
+          for (int i = 0; i < ((buf_dest + strlen(buf_dest)) - p2); i++) {
             temp_text[i] = 'X';
           }
         }
@@ -431,8 +430,8 @@ wipeField(char **dest, char *field, char **orig_dest)
         return;
       }
 
-      tmp_text[strlen(buf_orig_dest)] = '\0';
-      orig_dest ? strcpy(*orig_dest, tmp_text) : strcpy(*dest, tmp_text);
+      tmp_text[strlen(buf_dest)] = '\0';
+      strcpy(*field, tmp_text);
     }
   }
 }
@@ -458,7 +457,7 @@ wipeField(char **dest, char *field, char **orig_dest)
 
 inline bool
 LogFilterString::_checkConditionAndWipe(OperatorFunction f, char **field_value, size_t field_value_length, char **val,
-                                        char **orig_field_value, LengthCondition lc)
+                                        const char *uppercase_field_value, LengthCondition lc)
 {
   bool retVal = false;
 
@@ -468,18 +467,19 @@ LogFilterString::_checkConditionAndWipe(OperatorFunction f, char **field_value,
 
   // make single value case a little bit faster by taking it out of loop
   //
+  const char *lookup_field_value = uppercase_field_value ? uppercase_field_value : *field_value;
   if (m_num_values == 1) {
     switch (lc) {
     case DATA_LENGTH_EQUAL:
-      retVal = (field_value_length == *m_length ? ((*f)(*field_value, *val) == 0 ? true : false) : false);
+      retVal = (field_value_length == *m_length ? ((*f)(lookup_field_value, *val) == 0 ? true : false) : false);
       if (retVal) {
-        wipeField(field_value, *val, orig_field_value);
+        wipeField(field_value, *val, uppercase_field_value);
       }
       break;
     case DATA_LENGTH_LARGER:
-      retVal = (field_value_length > *m_length ? ((*f)(*field_value, *val) == 0 ? true : false) : false);
+      retVal = (field_value_length > *m_length ? ((*f)(lookup_field_value, *val) == 0 ? true : false) : false);
       if (retVal) {
-        wipeField(field_value, *val, orig_field_value);
+        wipeField(field_value, *val, uppercase_field_value);
       }
       break;
     default:
@@ -492,18 +492,18 @@ LogFilterString::_checkConditionAndWipe(OperatorFunction f, char **field_value,
     case DATA_LENGTH_EQUAL:
       for (i = 0; i < m_num_values; ++i) {
         // condition is satisfied if f returns zero
-        if (field_value_length == m_length[i] && (*f)(*field_value, val[i]) == 0) {
+        if (field_value_length == m_length[i] && (*f)(lookup_field_value, val[i]) == 0) {
           retVal = true;
-          wipeField(field_value, val[i], orig_field_value);
+          wipeField(field_value, val[i], uppercase_field_value);
         }
       }
       break;
     case DATA_LENGTH_LARGER:
       for (i = 0; i < m_num_values; ++i) {
         // condition is satisfied if f returns zero
-        if (field_value_length > m_length[i] && (*f)(*field_value, val[i]) == 0) {
+        if (field_value_length > m_length[i] && (*f)(lookup_field_value, val[i]) == 0) {
           retVal = true;
-          wipeField(field_value, val[i], orig_field_value);
+          wipeField(field_value, val[i], uppercase_field_value);
         }
       }
       break;