You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by wk...@apache.org on 2021/02/25 17:55:28 UTC

[trafficserver] branch master updated: Change cookie_remap plugin to allow use of pre-remap URL (and components). (#7519)

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

wkaras 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 40de57b  Change cookie_remap plugin to allow use of pre-remap URL (and components). (#7519)
40de57b is described below

commit 40de57b047a2b83e023404c3c64f7e3a81b38e64
Author: Walt Karas <wk...@verizonmedia.com>
AuthorDate: Thu Feb 25 11:55:18 2021 -0600

    Change cookie_remap plugin to allow use of pre-remap URL (and components). (#7519)
---
 doc/admin-guide/plugins/cookie_remap.en.rst        |  14 +
 plugins/experimental/cookie_remap/cookie_remap.cc  | 486 ++++++++++++++-------
 .../cookie_remap/configs/pcollapseconfig.txt       |   7 +
 .../cookie_remap/configs/psubstituteconfig.txt     |  21 +
 .../pluginTest/cookie_remap/gold/psubstitute.gold  |   9 +
 .../cookie_remap/pcollapseslashes.test.py          |  74 ++++
 .../pluginTest/cookie_remap/psubstitute.test.py    | 130 ++++++
 7 files changed, 579 insertions(+), 162 deletions(-)

diff --git a/doc/admin-guide/plugins/cookie_remap.en.rst b/doc/admin-guide/plugins/cookie_remap.en.rst
index c06d2c3..7b9dd54 100644
--- a/doc/admin-guide/plugins/cookie_remap.en.rst
+++ b/doc/admin-guide/plugins/cookie_remap.en.rst
@@ -32,6 +32,7 @@ Cookie Based Routing Inside TrafficServer Using cookie_remap
 
     * `Comments <#comments>`_
     * `cookie: X|X.Y <#cookie-xxy>`_
+    * `target: purl <#purl>`_
     * `operation: exists|not exists|string|regex|bucket <#operation-existsnot-existsstringregexbucket>`_
     * `match: str <#match-str>`_
     * `regex: str <#regex-str>`_
@@ -135,6 +136,13 @@ e.g
    B.f will operate on fsub
    </pre>
 
+target: puri
+^^^^^^^^^^^^
+
+----
+
+When the cookie key is omitted, the operation is applied to the request uri instead.  If this key and value
+is specified, the uri for the operation will be the pre-remapped, rather than the remapped, uri.
 
 operation: exists|not exists|string|regex|bucket
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -359,6 +367,12 @@ and a request like `http://finance.yahoo.com/photos/what/ever/ <http://foo.com/p
 
 will become `http://foo.com/what/ever/matches/x/y/z <http://foo.com/what/ever/matches/x/y/z>`_
 
+Alternatives using pre-remapped URL
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+$cr_req_url, $path and $unamatched_path are based on the remapped URL.  To use the pre-remapped
+URL, instead use $cr_req_purl, $ppath and $unmatched_ppath, respectively.
+
 An example configuration file
 -----------------------------
 
diff --git a/plugins/experimental/cookie_remap/cookie_remap.cc b/plugins/experimental/cookie_remap/cookie_remap.cc
index cdbe897..6e9dd9b 100644
--- a/plugins/experimental/cookie_remap/cookie_remap.cc
+++ b/plugins/experimental/cookie_remap/cookie_remap.cc
@@ -28,19 +28,16 @@
 #include <ts/remap.h>
 #include <yaml-cpp/yaml.h>
 
-#include <algorithm>
-#include <list>
-#include <map>
-#include <set>
-#include <cstring>
 #include <string>
-#include <unistd.h>
 #include <vector>
+#include <string_view>
+#include <cstddef>
 #include "hash.h"
 
-using namespace std;
+#undef FMT_SV
+#define FMT_SV(SV) static_cast<int>((SV).size()), (SV).data()
 
-// for bucketizing
+using namespace std;
 
 #define MY_NAME "cookie_remap"
 
@@ -60,59 +57,129 @@ ink_free(void *s)
 {
   return TSfree(s);
 }
-///////////////////////////////////////////////////////////////////////////////
-// Class holding one request URL's component, to simplify the code and
-// length calculations (we need all of them).
-//
-struct UrlComponents {
-  UrlComponents() = default;
+
+class UrlComponents
+{
+public:
+  UrlComponents(TSRemapRequestInfo *rri, TSHttpTxn txn) : _rri(rri), _txn(txn) {}
+
+  std::string const &
+  path(bool pre_remap)
+  {
+    if (_d[pre_remap].path_str.empty()) {
+      auto urlh = _get_url(pre_remap);
+      // based on RFC2396, matrix params are part of path segments so
+      // we will just
+      // append them to the path
+      _d[pre_remap].path_str = _get_url_comp(urlh, TSUrlPathGet);
+      auto matrix            = _get_url_comp(urlh, TSUrlHttpParamsGet);
+      if (!matrix.empty()) {
+        _d[pre_remap].path_str.append(";", 1).append(matrix);
+      }
+    }
+    return _d[pre_remap].path_str;
+  }
+
+  std::string_view
+  query(bool pre_remap)
+  {
+    if (_d[pre_remap].query.empty()) {
+      _d[pre_remap].query = _get_url_comp(_get_url(pre_remap), TSUrlHttpQueryGet);
+    }
+    return _d[pre_remap].query;
+  }
+
+  std::string_view
+  from_path()
+  {
+    if (_from_path.empty()) {
+      _UrlHandle urlh{_rri->requestBufp, _rri->mapFromUrl};
+      _from_path = _get_url_comp(urlh, TSUrlPathGet);
+    }
+    return _from_path;
+  }
+
+  std::string_view
+  url(bool pre_remap)
+  {
+    if (_d[pre_remap].url.empty()) {
+      auto urlh = _get_url(pre_remap);
+      int length;
+      auto data         = TSUrlStringGet(urlh.bufp, urlh.urlp, &length);
+      _d[pre_remap].url = std::string_view(data, length);
+    }
+    return _d[pre_remap].url;
+  }
+
+  // No copying/moving.
+  //
+  UrlComponents(UrlComponents const &) = delete;
+  UrlComponents &operator=(UrlComponents const &) = delete;
 
   ~UrlComponents()
   {
-    if (url != nullptr) {
-      TSfree((void *)url);
+    // Not calling TSHandleMLocRelease() for the URL TSMLoc pointers because it doesn't do anything.
+
+    if (_d[0].url.data() != nullptr) {
+      TSfree(const_cast<char *>(_d[0].url.data()));
+    }
+    if (_d[1].url.data() != nullptr) {
+      TSfree(const_cast<char *>(_d[1].url.data()));
     }
   }
 
-  void
-  populate(TSRemapRequestInfo *rri)
+private:
+  TSRemapRequestInfo *_rri;
+  TSHttpTxn _txn;
+
+  struct _UrlHandle {
+    TSMBuffer bufp = nullptr;
+    TSMLoc urlp;
+  };
+
+  // Buffer any data that's likely to be used more than once.
+
+  struct _Data {
+    _UrlHandle urlh;
+    std::string path_str;
+    std::string_view url;
+    std::string_view query;
+  };
+
+  // index 0 - remapped
+  // index 1 - pre-remap
+  //
+  _Data _d[2];
+
+  std::string_view _from_path;
+
+  _UrlHandle
+  _get_url(bool pre_remap)
   {
-    scheme    = TSUrlSchemeGet(rri->requestBufp, rri->requestUrl, &scheme_len);
-    host      = TSUrlHostGet(rri->requestBufp, rri->requestUrl, &host_len);
-    tspath    = TSUrlPathGet(rri->requestBufp, rri->requestUrl, &tspath_len);
-    query     = TSUrlHttpQueryGet(rri->requestBufp, rri->requestUrl, &query_len);
-    matrix    = TSUrlHttpParamsGet(rri->requestBufp, rri->requestUrl, &matrix_len);
-    port      = TSUrlPortGet(rri->requestBufp, rri->requestUrl);
-    url       = TSUrlStringGet(rri->requestBufp, rri->requestUrl, &url_len);
-    from_path = TSUrlPathGet(rri->requestBufp, rri->mapFromUrl, &from_path_len);
-
-    // based on RFC2396, matrix params are part of path segments so
-    // we will just
-    // append them to the path
-    path.assign(tspath, tspath_len);
-    if (matrix_len) {
-      path += ';';
-      path.append(matrix, matrix_len);
+    _UrlHandle h = _d[pre_remap].urlh;
+
+    if (!h.bufp) {
+      if (pre_remap) {
+        if (TSHttpTxnPristineUrlGet(_txn, &h.bufp, &h.urlp) != TS_SUCCESS) {
+          TSError("%s: Plugin is unable to get pristine url", MY_NAME);
+          return _UrlHandle();
+        }
+      } else {
+        h.bufp = _rri->requestBufp;
+        h.urlp = _rri->requestUrl;
+      }
+      _d[pre_remap].urlh = h;
     }
+    return h;
   }
 
-  const char *scheme = nullptr;
-  const char *host   = nullptr;
-  const char *tspath = nullptr;
-  std::string path{};
-  const char *query     = nullptr;
-  const char *matrix    = nullptr;
-  const char *url       = nullptr;
-  const char *from_path = nullptr;
-  int port              = 0;
-
-  int scheme_len    = 0;
-  int host_len      = 0;
-  int tspath_len    = 0;
-  int query_len     = 0;
-  int matrix_len    = 0;
-  int url_len       = 0;
-  int from_path_len = 0;
+  static std::string_view
+  _get_url_comp(_UrlHandle urlh, char const *(*comp_func)(TSMBuffer, TSMLoc, int *))
+  {
+    int length;
+    auto data = comp_func(urlh.bufp, urlh.urlp, &length);
+    return std::string_view(data, length);
+  }
 };
 
 void
@@ -127,6 +194,7 @@ enum operation_type { UNKNOWN = -1, EXISTS = 1, NOTEXISTS, REGEXP, STRING, BUCKE
 enum target_type {
   COOKIE = 1,
   URI, // URI = PATH + QUERY
+  PRE_REMAP_URI,
   UNKNOWN_TARGET
 };
 
@@ -172,13 +240,25 @@ dec_to_hex(type _num, char *hdigits)
 void
 urlencode(std::string &str)
 {
-  size_t pos = 0;
-  for (; pos < str.length(); pos++) {
-    if (!isalnum(str[pos])) {
-      char dec[2];
-      dec_to_hex(str[pos], dec);
-      std::string replacement = "%" + std::string(dec, 2);
-      str.replace(pos, 1, replacement);
+  auto orig = str.size();
+  auto enc  = orig;
+  for (auto c : str) {
+    if (!isalnum(c)) {
+      enc += 2;
+    }
+  }
+  if (enc == orig) {
+    // No changes needed.
+    return;
+  }
+  str.resize(enc);
+  while (orig--) {
+    if (!isalnum(str[orig])) {
+      enc -= 3;
+      dec_to_hex(str[orig], &(str[enc + 1]));
+      str[enc] = '%';
+    } else {
+      str[--enc] = str[orig];
     }
   }
 }
@@ -274,6 +354,8 @@ public:
   {
     if (s == "uri") {
       target = URI;
+    } else if (s == "puri") {
+      target = PRE_REMAP_URI;
     } else {
       target = COOKIE;
     }
@@ -475,7 +557,7 @@ public:
   }
 
   bool
-  process(CookieJar &jar, std::string &dest, TSHttpStatus &retstat, TSRemapRequestInfo *rri) const
+  process(CookieJar &jar, std::string &dest, TSHttpStatus &retstat, TSRemapRequestInfo *rri, UrlComponents &req_url) const
   {
     if (sendto == "") {
       return false; // guessing every operation must have a
@@ -567,7 +649,7 @@ public:
         } // handled EXISTS / NOTEXISTS subops
 
         TSDebug(MY_NAME, "processing cookie data: \"%s\"", cookie_data.c_str());
-      } else {
+      } else if (target != PRE_REMAP_URI) {
         target = URI;
       }
 
@@ -593,19 +675,18 @@ public:
       std::string request_uri; // only set the value if we
                                // need it; we might
       // match the cookie data instead
-      const std::string &string_to_match(target == URI ? request_uri : cookie_data);
-      if (target == URI) {
-        UrlComponents req_url;
-        req_url.populate(rri);
-
-        TSDebug(MY_NAME, "process req_url.path = %s", req_url.path.c_str());
-        request_uri = req_url.path;
+      bool use_url = (target == URI) || (target == PRE_REMAP_URI);
+      const std::string &string_to_match(use_url ? request_uri : cookie_data);
+      if (use_url) {
+        request_uri = req_url.path(target == PRE_REMAP_URI);
+        TSDebug(MY_NAME, "process req_url.path = %s", request_uri.c_str());
         if (request_uri.length() && request_uri[0] != '/') {
           request_uri.insert(0, 1, '/');
         }
-        if (req_url.query_len > 0) {
+        auto query = req_url.query(target == PRE_REMAP_URI);
+        if (query.size() > 0) {
           request_uri += '?';
-          request_uri += std::string(req_url.query, req_url.query_len);
+          request_uri += query;
         }
         object_name = "request uri";
       }
@@ -766,6 +847,8 @@ build_op(op &o, OpMap const &q)
     std::string const &key = pr.first;
     std::string const &val = pr.second;
 
+    TSDebug(MY_NAME, "build_op: key=%s val=%s", key.c_str(), val.c_str());
+
     if (key == "cookie") {
       if (!sub->empty()) {
         TSDebug(MY_NAME, "ERROR: you need to define a connector");
@@ -904,103 +987,182 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char *errbuf, int errbuf_s
   return TS_SUCCESS;
 }
 
-//----------------------------------------------------------------------------
-// called whenever we need to perform substitutions on a string; used to replace
-// things like
-//  $url, $unmatched_path, $cr_req_url, and $cr_url_encode
-// returns 0 if no substitutions, 1 otw.
-int
-cr_substitutions(std::string &obj, TSRemapRequestInfo *rri)
+namespace
 {
-  int retval = 0;
+std::string
+unmatched_path(UrlComponents &req_url, bool pre_remap)
+{
+  std::string path           = req_url.path(pre_remap);
+  std::string_view from_path = req_url.from_path();
 
-  size_t pos      = obj.find('$');
-  size_t tmp_pos  = 0;
-  size_t last_pos = pos;
+  std::size_t pos = path.find(from_path);
+  if (pos != std::string::npos) {
+    path.erase(pos, from_path.size());
+  }
+  TSDebug(MY_NAME, "from_path: %*s", FMT_SV(from_path));
+  TSDebug(MY_NAME, "%s: %s", pre_remap ? "unmatched_ppath" : "unmatched_path", path.c_str());
 
-  UrlComponents req_url;
-  req_url.populate(rri);
-  TSDebug(MY_NAME, "x req_url.path: %s %zu", req_url.path.c_str(), req_url.path.length());
-  TSDebug(MY_NAME, "x req_url.url: %s %d", std::string(req_url.url, req_url.url_len).c_str(), req_url.url_len);
+  return path;
+}
 
-  while (pos < obj.length()) {
-    if (pos + 1 >= obj.length()) {
-      break; // trailing ?
-    }
+int const sub_req_url_id         = 0;
+int const sub_req_purl_id        = -1;
+int const sub_path_id            = -2;
+int const sub_ppath_id           = -3;
+int const sub_unmatched_path_id  = -4;
+int const sub_unmatched_ppath_id = -5;
+int const sub_url_encode_id      = -6;
 
-    switch (obj[pos + 1]) {
-    case 'p':
-      if (obj.substr(pos + 1, 4) == "path") {
-        obj.replace(pos, 5, req_url.path);
-        pos += req_url.path.length();
-      }
-      break;
-    case 'u':
-      if (obj.substr(pos + 1, 14) == "unmatched_path") {
-        // we found $unmatched_path inside the sendto
-        // url
-        std::string unmatched_path(req_url.path);
-        TSDebug(MY_NAME, "unmatched_path: %s", unmatched_path.c_str());
-        TSDebug(MY_NAME, "from_path: %s", req_url.from_path);
-
-        tmp_pos = unmatched_path.find(req_url.from_path, 0, req_url.from_path_len); // from patch
-        if (tmp_pos != std::string::npos) {
-          unmatched_path.erase(tmp_pos, req_url.from_path_len);
-        }
-        TSDebug(MY_NAME, "unmatched_path: %s", unmatched_path.c_str());
-        TSDebug(MY_NAME, "obj: %s", obj.c_str());
-        obj.replace(pos, 15, unmatched_path);
-        TSDebug(MY_NAME, "obj: %s", obj.c_str());
-        pos += unmatched_path.length();
-      }
-      break;
-    case 'c':
-      if (obj.substr(pos + 1, 3) == "cr_") {
-        switch (obj[pos + 4]) {
-        case 'r':
-          if (obj.substr(pos + 4, 7) == "req_url") {
-            // we found $cr_req_url inside
-            // the sendto url
-            std::string request_url;
-            if (req_url.url && req_url.url_len > 0) {
-              request_url = std::string(req_url.url, req_url.url_len);
-            }
-            TSDebug(MY_NAME, "req_url.url: %s %d", std::string(req_url.url, req_url.url_len).c_str(), req_url.url_len);
-            obj.replace(pos, 11, request_url);
-            pos += request_url.length();
-          }
-          break;
-        case 'u':
-          // note that this allows for variables
-          // nested in the function, but not
-          // for functions nested in the function
-          if (obj.substr(pos + 4, 10) == "urlencode(" && (tmp_pos = obj.find(')', pos + 14)) != std::string::npos) {
-            std::string str = obj.substr(pos + 14, tmp_pos - pos - 14); // the string to
-                                                                        // encode
-            cr_substitutions(str, rri);                                 // expand any
-                                                                        // variables as
-                                                                        // necessary
-                                                                        // before
-                                                                        // encoding
-            urlencode(str);
-            obj.replace(pos, tmp_pos - pos + 1, str); // +1 for the
-                                                      // closing
-                                                      // parens
-            pos += str.length();
-          }
-          break;
-        }
+struct CompNext {
+  std::string_view const comp;
+  int const *const next;
+
+  CompNext(std::string_view p, int const *n) : comp(p), next(n) {}
+};
+
+struct {
+  int count = 2;
+  CompNext o1{"ath", &sub_unmatched_path_id};
+  CompNext o2{"path", &sub_unmatched_ppath_id};
+} const sub_unmatched;
+
+struct {
+  int count = 2;
+  CompNext o1{"ath", &sub_path_id};
+  CompNext o2{"path", &sub_ppath_id};
+} const sub_p;
+
+struct {
+  int count = 2;
+  CompNext o1{"url", &sub_req_url_id};
+  CompNext o2{"purl", &sub_req_purl_id};
+} const sub_cr_req;
+
+struct {
+  int count = 2;
+  CompNext o1{"req_", &sub_cr_req.count};
+  CompNext o2{"urlencode(", &sub_url_encode_id};
+} const sub_cr;
+
+struct {
+  int count = 3;
+  CompNext o1{"cr_", &sub_cr.count};
+  CompNext o2{"p", &sub_p.count};
+  CompNext o3{"unmatched_p", &sub_unmatched.count};
+} const sub;
+
+int
+sub_lookup(char const *targ, int targ_len)
+{
+  int count = sub.count;
+  auto opt  = &sub.o1;
+  for (;;) {
+    while ((targ_len < static_cast<int>(opt->comp.size())) || (std::string_view(targ, opt->comp.size()) != opt->comp)) {
+      if (!--count) {
+        return 1; // Failed lookup, return some positive numver.
       }
-      break;
+      ++opt;
     }
-    if (last_pos == pos) {
-      pos++; // don't search the same place again and again
+    count = *opt->next;
+    if (count <= 0) {
+      break;
     }
-    last_pos = pos;
-    pos      = obj.find('$', pos);
+    targ += opt->comp.size();
+    targ_len -= opt->comp.size();
+    opt = reinterpret_cast<CompNext const *>(reinterpret_cast<char const *>(opt->next) + offsetof(decltype(sub), o1));
+  }
+  return count;
+}
+
+} // end anonymous namespace
+
+//----------------------------------------------------------------------------
+// called whenever we need to perform substitutions on a string; used to replace
+// things like
+//  $path, $ppath, $unmatched_path, $unmatched_ppath, $cr_req_url, $cr_req_purl, and $cr_url_encode
+void
+cr_substitutions(std::string &obj, UrlComponents &req_url)
+{
+  {
+    auto path = req_url.path(false);
+    TSDebug(MY_NAME, "x req_url.path: %*s %d", FMT_SV(path), static_cast<int>(path.size()));
+    auto url = req_url.url(false);
+    TSDebug(MY_NAME, "x req_url.url: %*s %d", FMT_SV(url), static_cast<int>(url.size()));
   }
 
-  return retval;
+  auto npos = std::string::npos;
+  std::string tmp;
+  std::size_t pos = 0;
+  for (;;) {
+    pos = obj.find('$', pos);
+    if (npos == pos) {
+      break;
+    }
+    std::string_view variable, value;
+    switch (sub_lookup(obj.data() + pos + 1, static_cast<int>(obj.size()) - pos - 1)) {
+    case sub_req_url_id: {
+      variable = "$cr_req_url";
+      value    = req_url.url(false);
+    } break;
+
+    case sub_req_purl_id: {
+      variable = "$cr_req_purl";
+      value    = req_url.url(true);
+    } break;
+
+    case sub_path_id: {
+      variable = "$path";
+      value    = req_url.path(false);
+    } break;
+
+    case sub_ppath_id: {
+      variable = "$ppath";
+      value    = req_url.path(true);
+    } break;
+
+    case sub_unmatched_path_id: {
+      variable = "$unmatched_path";
+      tmp      = unmatched_path(req_url, false);
+      value    = tmp;
+    } break;
+
+    case sub_unmatched_ppath_id: {
+      variable = "$unmatched_ppath";
+      tmp      = unmatched_path(req_url, true);
+      value    = tmp;
+    } break;
+
+    case sub_url_encode_id: {
+      std::size_t bpos = pos + sizeof("cr_urlencode(") - 1;
+      std::size_t epos = obj.find(')', bpos);
+      if (npos == epos) {
+        variable = "$";
+        value    = variable;
+      } else {
+        variable = std::string_view(obj.data() + pos, epos + 1 - pos);
+
+        tmp = obj.substr(bpos, epos - bpos);
+        cr_substitutions(tmp, req_url);
+        urlencode(tmp);
+        value = tmp;
+      }
+    } break;
+
+    default: {
+      variable = "$";
+      value    = variable;
+
+    } break;
+
+    } // end switch
+
+    TSDebug(MY_NAME, "%*s => %*s", FMT_SV(variable), FMT_SV(value));
+
+    obj.replace(pos, variable.size(), value);
+
+    pos += value.size();
+
+  } // end for (;;)
 }
 
 //----------------------------------------------------------------------------
@@ -1012,8 +1174,7 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
   OpsQueue *ops       = static_cast<OpsQueue *>(ih);
   TSHttpStatus status = TS_HTTP_STATUS_NONE;
 
-  UrlComponents req_url;
-  req_url.populate(rri);
+  UrlComponents req_url{rri, txnp};
 
   if (ops == (OpsQueue *)nullptr) {
     TSError("serious error with encountered while attempting to "
@@ -1024,9 +1185,10 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
 
   // get any query params..we will append that to the answer (possibly)
   std::string client_req_query_params;
-  if (req_url.query_len) {
+  auto query = req_url.query(false);
+  if (!query.empty()) {
     client_req_query_params = "?";
-    client_req_query_params += std::string(req_url.query, req_url.query_len);
+    client_req_query_params += query;
   }
   TSDebug(MY_NAME, "Query Parameters: %s", client_req_query_params.c_str());
 
@@ -1051,8 +1213,8 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
 
   for (auto &op : *ops) {
     TSDebug(MY_NAME, ">>> processing new operation");
-    if (op->process(jar, rewrite_to, status, rri)) {
-      cr_substitutions(rewrite_to, rri);
+    if (op->process(jar, rewrite_to, status, rri, req_url)) {
+      cr_substitutions(rewrite_to, req_url);
 
       size_t pos = 7;                             // 7 because we want to ignore the // in
                                                   // http:// :)
diff --git a/tests/gold_tests/pluginTest/cookie_remap/configs/pcollapseconfig.txt b/tests/gold_tests/pluginTest/cookie_remap/configs/pcollapseconfig.txt
new file mode 100644
index 0000000..65c508c
--- /dev/null
+++ b/tests/gold_tests/pluginTest/cookie_remap/configs/pcollapseconfig.txt
@@ -0,0 +1,7 @@
+# This is a test configuration
+
+# Do a regex against the request url
+op:
+  target: puri
+  regex: /orig_
+  sendto: http://127.0.0.10:$PORT/i/////////like/cheetos?.done=http://finance.yahoo.com
diff --git a/tests/gold_tests/pluginTest/cookie_remap/configs/psubstituteconfig.txt b/tests/gold_tests/pluginTest/cookie_remap/configs/psubstituteconfig.txt
new file mode 100644
index 0000000..243b23d
--- /dev/null
+++ b/tests/gold_tests/pluginTest/cookie_remap/configs/psubstituteconfig.txt
@@ -0,0 +1,21 @@
+# This is a test configuration (version using pre-remap URL)
+
+# Do a regex against the cookie
+op:
+  cookie: fpbeta
+  operation: exists
+  sendto: http://127.0.0.10:$PORT/photos/search?query=$ppath
+op:
+  cookie: oxalpha
+  operation: exists
+  sendto: http://127.0.0.10:$PORT/photos/search?query=$unmatched_ppath
+op:
+  cookie: acgamma
+  operation: exists
+  sendto: http://127.0.0.10:$PORT/photos/search/cr_substitutions?query=$cr_urlencode($cr_req_purl)
+# Regex against url and path is substituted in outgoing path
+op:
+  operation: regex
+  regex: foobar
+  sendto: http://127.0.0.10:$PORT/photos/search/$ppath
+
diff --git a/tests/gold_tests/pluginTest/cookie_remap/gold/psubstitute.gold b/tests/gold_tests/pluginTest/cookie_remap/gold/psubstitute.gold
new file mode 100644
index 0000000..fc07cc7
--- /dev/null
+++ b/tests/gold_tests/pluginTest/cookie_remap/gold/psubstitute.gold
@@ -0,0 +1,9 @@
+``
+Serving GET /photos/search?query=magic... Finished
+``
+Serving GET /photos/search?query=/theunmatchedpath... Finished
+``
+Serving GET /photos/search/cr_substitutions?query=%28http%3A%2F%2Fwww%2Eexample%2Ecom%2Fmagic... Finished
+``
+Serving GET /photos/search/magic/foobar... Finished
+``
diff --git a/tests/gold_tests/pluginTest/cookie_remap/pcollapseslashes.test.py b/tests/gold_tests/pluginTest/cookie_remap/pcollapseslashes.test.py
new file mode 100644
index 0000000..ec46d1b
--- /dev/null
+++ b/tests/gold_tests/pluginTest/cookie_remap/pcollapseslashes.test.py
@@ -0,0 +1,74 @@
+'''
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+import os
+Test.Summary = '''
+
+'''
+Test.SkipUnless(Condition.PluginExists('cookie_remap.so'))
+Test.ContinueOnFail = True
+Test.testName = "cookie_remap: plugin collapses consecutive slashes (pre-remap URI)"
+
+# Define default ATS
+ts = Test.MakeATSProcess("ts")
+
+# We only run a server to capture ATS outbound requests
+# and verify it collapsed the double //
+server = Test.MakeOriginServer("server", ip='127.0.0.10')
+
+request_header = {"headers": "GET /i/like/cheetos?.done=http://finance.yahoo.com HTTP/1.1\r\nHost: www.example.com\r\n\r\n",
+                  "timestamp": "1469733493.993", "body": ""}
+# expected response from the origin server
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+
+# add response to the server dictionary
+server.addResponse("sessionfile.log", request_header, response_header)
+
+# Setup the remap configuration
+config_path = os.path.join(Test.TestDirectory, "configs/pcollapseconfig.txt")
+with open(config_path, 'r') as config_file:
+    config1 = config_file.read()
+
+ts.Disk.records_config.update({
+    'proxy.config.diags.debug.enabled': 1,
+    'proxy.config.diags.debug.tags': 'cookie_remap.*|http.*|dns.*',
+})
+
+config1 = config1.replace("$PORT", str(server.Variables.Port))
+
+ts.Disk.File(ts.Variables.CONFIGDIR + "/collapseconfig.txt", exists=False, id="config1")
+ts.Disk.config1.WriteOn(config1)
+
+ts.Disk.remap_config.AddLine(
+    'map http://www.example.com/orig_path http://shouldnothit.com/magic @plugin=cookie_remap.so @pparam=config/collapseconfig.txt'
+)
+
+tr = Test.AddTestRun("collapse consecutive forward slashes")
+tr.Processes.Default.Command = '''
+curl \
+--proxy 127.0.0.1:{0} \
+"http://www.example.com/orig_path" \
+-H "Proxy-Connection: keep-alive" \
+--verbose \
+'''.format(ts.Variables.port)
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port))
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.StillRunningAfter = ts
+
+server.Streams.All = "gold/collapseslashes.gold"
diff --git a/tests/gold_tests/pluginTest/cookie_remap/psubstitute.test.py b/tests/gold_tests/pluginTest/cookie_remap/psubstitute.test.py
new file mode 100644
index 0000000..3e71495
--- /dev/null
+++ b/tests/gold_tests/pluginTest/cookie_remap/psubstitute.test.py
@@ -0,0 +1,130 @@
+'''
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+import os
+Test.Summary = '''
+
+'''
+Test.SkipUnless(Condition.PluginExists('cookie_remap.so'))
+Test.ContinueOnFail = True
+Test.testName = "cookie_remap: Substitute variables (pre-remap URL)"
+
+# Define default ATS
+ts = Test.MakeATSProcess("ts")
+
+server = Test.MakeOriginServer("server", ip='127.0.0.10')
+
+request_header = {"headers": "GET /photos/search?query=magic HTTP/1.1\r\nHost: www.example.com\r\n\r\n",
+                  "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+
+server.addResponse("sessionfile.log", request_header, response_header)
+
+request_header = {"headers": "GET /photos/search?query=/theunmatchedpath HTTP/1.1\r\nHost: www.example.com\r\n\r\n",
+                  "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+
+server.addResponse("sessionfile.log", request_header, response_header)
+
+request_header = {"headers": "GET /photos/search/magic/foobar HTTP/1.1\r\nHost: www.example.com\r\n\r\n",
+                  "timestamp": "1469733493.993", "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+
+server.addResponse("sessionfile.log", request_header, response_header)
+
+request_header = {
+    "headers": "GET /photos/search/cr_substitutions?query=%28http%3A%2F%2Fwww%2Eexample%2Ecom%2Fmagic HTTP/1.1\r\nHost: www.example.com\r\n\r\n",
+    "timestamp": "1469733493.993",
+    "body": ""}
+response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""}
+
+server.addResponse("sessionfile.log", request_header, response_header)
+
+# Setup the remap configuration
+config_path = os.path.join(Test.TestDirectory, "configs/psubstituteconfig.txt")
+with open(config_path, 'r') as config_file:
+    config1 = config_file.read()
+
+ts.Disk.records_config.update({
+    'proxy.config.diags.debug.enabled': 1,
+    'proxy.config.diags.debug.tags': 'cookie_remap.*|http.*|dns.*',
+})
+
+config1 = config1.replace("$PORT", str(server.Variables.Port))
+
+ts.Disk.File(ts.Variables.CONFIGDIR + "/substituteconfig.txt", exists=False, id="config1")
+ts.Disk.config1.WriteOn(config1)
+
+ts.Disk.remap_config.AddLine(
+    'map http://www.example.com/magic http://shouldnothit.com/not-used @plugin=cookie_remap.so @pparam=config/substituteconfig.txt'
+)
+
+tr = Test.AddTestRun("Substitute $ppath in the dest query")
+tr.Processes.Default.Command = '''
+curl \
+--proxy 127.0.0.1:{0} \
+"http://www.example.com/magic" \
+-H"Cookie: fpbeta=abcd" \
+-H "Proxy-Connection: keep-alive" \
+--verbose \
+'''.format(ts.Variables.port)
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port))
+tr.Processes.Default.StartBefore(Test.Processes.ts)
+tr.StillRunningAfter = ts
+tr.StillRunningAfter = server
+
+tr = Test.AddTestRun("Substitute $unmatched_ppath in the dest query")
+tr.Processes.Default.Command = '''
+curl \
+--proxy 127.0.0.1:{0} \
+"http://www.example.com/magic/theunmatchedpath" \
+-H"Cookie: oxalpha=3333" \
+-H "Proxy-Connection: keep-alive" \
+--verbose \
+'''.format(ts.Variables.port)
+tr.Processes.Default.ReturnCode = 0
+tr.StillRunningAfter = ts
+tr.StillRunningAfter = server
+
+tr = Test.AddTestRun("Substitute $cr_req_purl using $cr_urlencode")
+tr.Processes.Default.Command = '''
+curl \
+--proxy 127.0.0.1:{0} \
+"http://www.example.com/magic" \
+-H"Cookie: acgamma=dfndfdfd" \
+-H "Proxy-Connection: keep-alive" \
+--verbose \
+'''.format(ts.Variables.port)
+tr.Processes.Default.ReturnCode = 0
+tr.StillRunningAfter = ts
+tr.StillRunningAfter = server
+
+tr = Test.AddTestRun("Substitute $ppath as is in outgoing path")
+tr.Processes.Default.Command = '''
+curl \
+--proxy 127.0.0.1:{0} \
+"http://www.example.com/magic/foobar" \
+-H "Proxy-Connection: keep-alive" \
+--verbose \
+'''.format(ts.Variables.port)
+tr.Processes.Default.ReturnCode = 0
+tr.StillRunningAfter = ts
+tr.StillRunningAfter = server
+
+server.Streams.All = "gold/psubstitute.gold"