You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2017/12/15 03:23:36 UTC

[trafficserver] branch 7.1.x updated (e26157e -> d7013fa)

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

zwoop pushed a change to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git.


    from e26157e  Only allow the gzip/brotli transformation on 200 OK responses
     new ad0b4c0  Coverity - 1021688 Uninitialized scalar field.
     new 1de128d  Coverity 1021688: Uninitialized scalar field - Cleaning up the constructor
     new d7013fa  Removes the limit on the number of remap plugins per rule

The 3 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/http/remap/RemapPlugins.cc |  7 +----
 proxy/http/remap/UrlMapping.cc   | 61 ++++++++++++++--------------------------
 proxy/http/remap/UrlMapping.h    | 54 +++++++++++++++++------------------
 3 files changed, 49 insertions(+), 73 deletions(-)

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].

[trafficserver] 02/03: Coverity 1021688: Uninitialized scalar field - Cleaning up the constructor

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

zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 1de128d3ab6750c597ed9a2866a786879aa91b4b
Author: Steven Feltner <sf...@godaddy.com>
AuthorDate: Wed May 10 08:30:33 2017 -0700

    Coverity 1021688: Uninitialized scalar field - Cleaning up the constructor
    
    (cherry picked from commit 3550957de15429fde28b3fc46bdb80eda598e06b)
---
 proxy/http/remap/UrlMapping.cc | 2 +-
 proxy/http/remap/UrlMapping.h  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/proxy/http/remap/UrlMapping.cc b/proxy/http/remap/UrlMapping.cc
index e324c1c..e83d1c8 100644
--- a/proxy/http/remap/UrlMapping.cc
+++ b/proxy/http/remap/UrlMapping.cc
@@ -28,7 +28,7 @@
 /**
  *
 **/
-url_mapping::url_mapping(int rank /* = 0 */)
+url_mapping::url_mapping()
 {
   memset(_plugin_list, 0, sizeof(_plugin_list));
   memset(_instance_data, 0, sizeof(_instance_data));
diff --git a/proxy/http/remap/UrlMapping.h b/proxy/http/remap/UrlMapping.h
index f8c18c8..68f2db3 100644
--- a/proxy/http/remap/UrlMapping.h
+++ b/proxy/http/remap/UrlMapping.h
@@ -79,7 +79,7 @@ public:
 class url_mapping
 {
 public:
-  url_mapping(int rank = 0);
+  url_mapping();
   ~url_mapping();
 
   bool add_plugin(remap_plugin_info *i, void *ih);
@@ -126,7 +126,7 @@ public:
 private:
   remap_plugin_info *_plugin_list[MAX_REMAP_PLUGIN_CHAIN];
   void *_instance_data[MAX_REMAP_PLUGIN_CHAIN];
-  int _rank;
+  int _rank = 0;
 };
 
 /**

-- 
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.

[trafficserver] 03/03: Removes the limit on the number of remap plugins per rule

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

zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit d7013faf0c8a151299adf2eecf3c99140768ad15
Author: Leif Hedstrom <zw...@apache.org>
AuthorDate: Thu Dec 14 14:35:20 2017 -0700

    Removes the limit on the number of remap plugins per rule
    
    Note: This retains as much as possible of the old code, such that we
    can cherry pick this reasonably safely to 7.x. Long term, we should change
    this to be something like
    
         using PluginInstance = std::pair<remap_plugin_info *, void *>;
    
    and iterate over those instead of the indices.
    
    (cherry picked from commit da827dee6c8c6b177a0f59f53b7f43b9b28e517d)
---
 proxy/http/remap/RemapPlugins.cc |  7 +------
 proxy/http/remap/UrlMapping.cc   | 45 +++++++++++++++++++---------------------
 proxy/http/remap/UrlMapping.h    | 24 ++++++++++-----------
 3 files changed, 34 insertions(+), 42 deletions(-)

diff --git a/proxy/http/remap/RemapPlugins.cc b/proxy/http/remap/RemapPlugins.cc
index 97327dd..3690f6d 100644
--- a/proxy/http/remap/RemapPlugins.cc
+++ b/proxy/http/remap/RemapPlugins.cc
@@ -135,12 +135,7 @@ RemapPlugins::run_single_remap()
     return 1;
   }
 
-  if (_cur > MAX_REMAP_PLUGIN_CHAIN) {
-    Error("called %s more than %u times; stopping this remap insanity now", __func__, MAX_REMAP_PLUGIN_CHAIN);
-    return 1;
-  }
-
-  if (_cur >= map->_plugin_count) {
+  if (_cur >= map->plugin_count()) {
     // Normally, we would callback into this function but we dont have anything more to do!
     Debug("url_rewrite", "completed all remap plugins for rule id %d", map->map_id);
     return 1;
diff --git a/proxy/http/remap/UrlMapping.cc b/proxy/http/remap/UrlMapping.cc
index e83d1c8..079baa0 100644
--- a/proxy/http/remap/UrlMapping.cc
+++ b/proxy/http/remap/UrlMapping.cc
@@ -25,14 +25,6 @@
 #include "UrlMapping.h"
 #include "I_RecCore.h"
 #include "ts/ink_cap.h"
-/**
- *
-**/
-url_mapping::url_mapping()
-{
-  memset(_plugin_list, 0, sizeof(_plugin_list));
-  memset(_instance_data, 0, sizeof(_instance_data));
-}
 
 /**
  *
@@ -40,13 +32,8 @@ url_mapping::url_mapping()
 bool
 url_mapping::add_plugin(remap_plugin_info *i, void *ih)
 {
-  if (_plugin_count >= MAX_REMAP_PLUGIN_CHAIN) {
-    return false;
-  }
-
-  _plugin_list[_plugin_count]   = i;
-  _instance_data[_plugin_count] = ih;
-  ++_plugin_count;
+  _plugin_list.push_back(i);
+  _instance_data.push_back(ih);
 
   return true;
 }
@@ -55,14 +42,22 @@ url_mapping::add_plugin(remap_plugin_info *i, void *ih)
  *
 **/
 remap_plugin_info *
-url_mapping::get_plugin(unsigned int index) const
+url_mapping::get_plugin(std::size_t index) const
 {
-  Debug("url_rewrite", "get_plugin says we have %d plugins and asking for plugin %d", _plugin_count, index);
-  if ((_plugin_count == 0) || unlikely(index > _plugin_count)) {
-    return nullptr;
+  Debug("url_rewrite", "get_plugin says we have %zu plugins and asking for plugin %zu", plugin_count(), index);
+  if (index < _plugin_list.size()) {
+    return _plugin_list[index];
   }
+  return nullptr;
+}
 
-  return _plugin_list[index];
+void *
+url_mapping::get_instance(std::size_t index) const
+{
+  if (index < _instance_data.size()) {
+    return _instance_data[index];
+  }
+  return nullptr;
 }
 
 /**
@@ -101,8 +96,10 @@ url_mapping::~url_mapping()
     delete rc;
   }
 
-  // Delete all instance data
-  for (unsigned int i = 0; i < _plugin_count; ++i) {
+  // Delete all instance data, this gets ugly because to delete the instance data, we also
+  // must know which plugin this is associated with. Hence, looping with index instead of a
+  // normal iterator. ToDo: Maybe we can combine them into another container.
+  for (std::size_t i = 0; i < plugin_count(); ++i) {
     delete_instance(i);
   }
 
@@ -124,8 +121,8 @@ url_mapping::Print()
 
   fromURL.string_get_buf(from_url_buf, (int)sizeof(from_url_buf));
   toUrl.string_get_buf(to_url_buf, (int)sizeof(to_url_buf));
-  printf("\t %s %s=> %s %s <%s> [plugins %s enabled; running with %u plugins]\n", from_url_buf, unique ? "(unique)" : "",
-         to_url_buf, homePageRedirect ? "(R)" : "", tag ? tag : "", _plugin_count > 0 ? "are" : "not", _plugin_count);
+  printf("\t %s %s=> %s %s <%s> [plugins %s enabled; running with %zu plugins]\n", from_url_buf, unique ? "(unique)" : "",
+         to_url_buf, homePageRedirect ? "(R)" : "", tag ? tag : "", plugin_count() > 0 ? "are" : "not", plugin_count());
 }
 
 /**
diff --git a/proxy/http/remap/UrlMapping.h b/proxy/http/remap/UrlMapping.h
index 68f2db3..e05601e 100644
--- a/proxy/http/remap/UrlMapping.h
+++ b/proxy/http/remap/UrlMapping.h
@@ -24,6 +24,8 @@
 #ifndef _URL_MAPPING_H_
 #define _URL_MAPPING_H_
 
+#include <vector>
+
 #include "ts/ink_config.h"
 #include "AclFiltering.h"
 #include "Main.h"
@@ -32,8 +34,6 @@
 #include "ts/Regex.h"
 #include "ts/List.h"
 
-static const unsigned int MAX_REMAP_PLUGIN_CHAIN = 10;
-
 /**
  * Used to store http referer strings (and/or regexp)
 **/
@@ -79,17 +79,18 @@ public:
 class url_mapping
 {
 public:
-  url_mapping();
   ~url_mapping();
 
   bool add_plugin(remap_plugin_info *i, void *ih);
-  remap_plugin_info *get_plugin(unsigned int) const;
+  remap_plugin_info *get_plugin(std::size_t) const;
+  void *get_instance(std::size_t) const;
 
-  void *
-  get_instance(unsigned int index) const
+  std::size_t
+  plugin_count() const
   {
-    return _instance_data[index];
-  };
+    return _plugin_list.size();
+  }
+
   void delete_instance(unsigned int index);
   void Print();
 
@@ -109,8 +110,7 @@ public:
   redirect_tag_str *redir_chunk_list = nullptr;
   bool ip_allow_check_enabled_p      = false;
   acl_filter_rule *filter            = nullptr; // acl filtering (list of rules)
-  unsigned int _plugin_count         = 0;
-  LINK(url_mapping, link); // For use with the main Queue linked list holding all the mapping
+  LINK(url_mapping, link);                      // For use with the main Queue linked list holding all the mapping
 
   int
   getRank() const
@@ -124,8 +124,8 @@ public:
   };
 
 private:
-  remap_plugin_info *_plugin_list[MAX_REMAP_PLUGIN_CHAIN];
-  void *_instance_data[MAX_REMAP_PLUGIN_CHAIN];
+  std::vector<remap_plugin_info *> _plugin_list;
+  std::vector<void *> _instance_data;
   int _rank = 0;
 };
 

-- 
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.

[trafficserver] 01/03: Coverity - 1021688 Uninitialized scalar field.

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

zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit ad0b4c0dd64c219a6d0713fc0dded93c4bf82217
Author: Steven Feltner <sf...@godaddy.com>
AuthorDate: Tue May 9 08:09:34 2017 -0700

    Coverity - 1021688 Uninitialized scalar field.
    
    (cherry picked from commit cec48015cd389513a5282b2d6833f1b0f35df47b)
---
 proxy/http/remap/UrlMapping.cc | 16 ----------------
 proxy/http/remap/UrlMapping.h  | 30 +++++++++++++++---------------
 2 files changed, 15 insertions(+), 31 deletions(-)

diff --git a/proxy/http/remap/UrlMapping.cc b/proxy/http/remap/UrlMapping.cc
index 08c335b..e324c1c 100644
--- a/proxy/http/remap/UrlMapping.cc
+++ b/proxy/http/remap/UrlMapping.cc
@@ -29,22 +29,6 @@
  *
 **/
 url_mapping::url_mapping(int rank /* = 0 */)
-  : from_path_len(0),
-    fromURL(),
-    toUrl(),
-    homePageRedirect(false),
-    unique(false),
-    default_redirect_url(false),
-    optional_referer(false),
-    negative_referer(false),
-    wildcard_from_scheme(false),
-    tag(nullptr),
-    filter_redirect_url(nullptr),
-    referer_list(nullptr),
-    redir_chunk_list(nullptr),
-    filter(nullptr),
-    _plugin_count(0),
-    _rank(rank)
 {
   memset(_plugin_list, 0, sizeof(_plugin_list));
   memset(_instance_data, 0, sizeof(_instance_data));
diff --git a/proxy/http/remap/UrlMapping.h b/proxy/http/remap/UrlMapping.h
index 73f87c7..f8c18c8 100644
--- a/proxy/http/remap/UrlMapping.h
+++ b/proxy/http/remap/UrlMapping.h
@@ -93,23 +93,23 @@ public:
   void delete_instance(unsigned int index);
   void Print();
 
-  int from_path_len;
+  int from_path_len = 0;
   URL fromURL;
   URL toUrl; // Default TO-URL (from remap.config)
-  bool homePageRedirect;
-  bool unique; // INKqa11970 - unique mapping
-  bool default_redirect_url;
-  bool optional_referer;
-  bool negative_referer;
-  bool wildcard_from_scheme; // from url is '/foo', only http or https for now
-  char *tag;                 // tag
-  char *filter_redirect_url; // redirect url when referer filtering enabled
-  unsigned int map_id;
-  referer_info *referer_list;
-  redirect_tag_str *redir_chunk_list;
-  bool ip_allow_check_enabled_p;
-  acl_filter_rule *filter; // acl filtering (list of rules)
-  unsigned int _plugin_count;
+  bool homePageRedirect              = false;
+  bool unique                        = false; // INKqa11970 - unique mapping
+  bool default_redirect_url          = false;
+  bool optional_referer              = false;
+  bool negative_referer              = false;
+  bool wildcard_from_scheme          = false;   // from url is '/foo', only http or https for now
+  char *tag                          = nullptr; // tag
+  char *filter_redirect_url          = nullptr; // redirect url when referer filtering enabled
+  unsigned int map_id                = 0;
+  referer_info *referer_list         = nullptr;
+  redirect_tag_str *redir_chunk_list = nullptr;
+  bool ip_allow_check_enabled_p      = false;
+  acl_filter_rule *filter            = nullptr; // acl filtering (list of rules)
+  unsigned int _plugin_count         = 0;
   LINK(url_mapping, link); // For use with the main Queue linked list holding all the mapping
 
   int

-- 
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.