git commit: TS-898: stop the esi plugin referencing invalidated strings

[jp...@apache.org]

Repository: trafficserver
Updated Branches:
  refs/heads/master 270d0a0f6 -> 07a108186


TS-898: stop the esi plugin referencing invalidated strings

The hackery that the esi plugin does to avoid any string copying
in cookie value extraction leads Coverity to believe that invalidated
std::string data is referenced. This is is probably fine in practice,
but it's still easy to fix.

Coverity #1200048


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/07a10818
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/07a10818
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/07a10818

Branch: refs/heads/master
Commit: 07a10818623ab5660bf79aa33a203ef211ad46fa
Parents: 270d0a0
Author: James Peach <jp...@apache.org>
Authored: Thu May 1 12:37:00 2014 -0700
Committer: James Peach <jp...@apache.org>
Committed: Fri May 2 13:15:04 2014 -0700

----------------------------------------------------------------------
 CHANGES                                   |  2 ++
 plugins/experimental/esi/lib/Variables.cc | 21 ++++++++++++---------
 plugins/experimental/esi/lib/Variables.h  |  2 +-
 3 files changed, 15 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/07a10818/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index baba84b..36aed43 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 5.0.0
 
+  *) [TS-898] Stop the esi plugin referencing invalidated strings.
+
   *) [TS-2778] Websockets remap doesn't properly handle the implicit port for wss.
 
   *) [TS-2774] TS::AdminClient.pm's get_stat API broken in 5.0

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/07a10818/plugins/experimental/esi/lib/Variables.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/esi/lib/Variables.cc b/plugins/experimental/esi/lib/Variables.cc
index dfc6824..0bf93dd 100644
--- a/plugins/experimental/esi/lib/Variables.cc
+++ b/plugins/experimental/esi/lib/Variables.cc
@@ -308,6 +308,7 @@ Variables::_getSubCookieValue(const string &cookie_str, size_t cookie_part_divid
   // character, and we don't need to create a copy of the string for
   // that; hence this shortcut
   string &non_const_cookie_str = const_cast<string &>(cookie_str);
+  StringHash::const_iterator it_part;
     
   non_const_cookie_str[cookie_part_divider] = '\0'; // make sure cookie name is NULL terminated
   const char *cookie_name = non_const_cookie_str.data(); /* above NULL will take effect */
@@ -317,25 +318,27 @@ Variables::_getSubCookieValue(const string &cookie_str, size_t cookie_part_divid
   if (it_cookie == _sub_cookies.end()) {
       _debugLog(_debug_tag, "[%s] Could not find value for cookie [%s]", 
               __FUNCTION__, cookie_name);
-      return EMPTY_STRING;
+      goto fail;
   }
 
-  non_const_cookie_str[cookie_part_divider] = ';'; // restore before returning
-
-  StringHash::const_iterator it_part = it_cookie->second.find(part_name);
+  it_part = it_cookie->second.find(part_name);
   if (it_part == it_cookie->second.end()) {
       _debugLog(_debug_tag, "[%s] Could not find value for part [%s] of cookie [%.*s]", __FUNCTION__,
               part_name, cookie_part_divider, cookie_name);
-      return EMPTY_STRING;
+      goto fail;
   }
 
   _debugLog(_debug_tag, "[%s] Got value [%s] for cookie name [%.*s] and part [%s]",
           __FUNCTION__, it_part->second.c_str(), cookie_part_divider, cookie_name, part_name);
 
-  // we need to do this as have to return a string reference
-  string &retval = const_cast<Variables &>(*this)._cached_sub_cookie_value;
-  retval.assign(it_part->second);
-  return retval;
+  non_const_cookie_str[cookie_part_divider] = ';'; // restore before returning
+
+  this->_cached_sub_cookie_value.assign(it_part->second);
+  return this->_cached_sub_cookie_value;
+
+fail:
+  non_const_cookie_str[cookie_part_divider] = ';'; // restore before returning
+  return EMPTY_STRING;
 }
 
 void

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/07a10818/plugins/experimental/esi/lib/Variables.h
----------------------------------------------------------------------
diff --git a/plugins/experimental/esi/lib/Variables.h b/plugins/experimental/esi/lib/Variables.h
index 15bfe8b..6462662 100644
--- a/plugins/experimental/esi/lib/Variables.h
+++ b/plugins/experimental/esi/lib/Variables.h
@@ -151,7 +151,7 @@ private:
     }
   }
 
-  std::string _cached_sub_cookie_value;
+  mutable std::string _cached_sub_cookie_value;
   const std::string &_getSubCookieValue(const std::string &cookie_str, size_t cookie_part_divider) const;
 
 };