From 062e796aba2f2bfa6ca5c8b5f90a60801e18e30e Mon Sep 17 00:00:00 2001
From: Gancho Tenev <gancho@apache.org>
Date: Wed, 21 Aug 2019 16:11:48 -0700
Subject: [PATCH] cachekey: added --canonical-prefix parameter

In certain use-cases when calculating the prefix (the initial value
of the new cache key) we need to have the scheme, host and port in
their original form from the request URI, i.e. when hosting.config
is used the cache key is expected to contain a valid URI authority
element used for volume selection.

More details about the new parameter and its functionality can be
found in doc/admin-guide/plugins/cachekey.en.rst
---
 doc/admin-guide/plugins/cachekey.en.rst | 21 ++++--
 plugins/cachekey/cachekey.cc            | 90 ++++++++++++++++++-------
 plugins/cachekey/cachekey.h             |  3 +-
 plugins/cachekey/configs.cc             | 11 +++
 plugins/cachekey/configs.h              |  6 ++
 plugins/cachekey/plugin.cc              |  2 +-
 6 files changed, 102 insertions(+), 31 deletions(-)

diff --git a/doc/admin-guide/plugins/cachekey.en.rst b/doc/admin-guide/plugins/cachekey.en.rst
index 5342d8b52b8..51f4ef174a8 100644
--- a/doc/admin-guide/plugins/cachekey.en.rst
+++ b/doc/admin-guide/plugins/cachekey.en.rst
@@ -70,20 +70,29 @@ Cache key structure and related plugin parameters
 
 ::
 
-  Optional components      | ┌─────────────────┬──────────────────┬──────────────────────┐
+  Optional components      | ┌─────────────────┬────────────── ───┬──────────────────────┐
   (included in this order) | │ --static-prefix | --capture-prefix │ --capture-prefix-uri │
                            | ├─────────────────┴──────────────────┴──────────────────────┤
-  Default values if no     | │ /host/port                                                |
+  Default values if no     | │ /host/port or scheme://host:port (see the table below)    |
   optional components      | └───────────────────────────────────────────────────────────┘
   configured               |
 
+  ┌────────────────────┬─────────────────────────┬──────────────────────┐
+  │ --canonical-prefix |  default value if no    │ input used for       │
+  │                    |  prefix parameters used │ --capture-prefix     │
+  ├────────────────────┴─────────────────────────┴──────────────────────┤
+  │ fasle              | /host/port              | host:port            |
+  ├────────────────────┴─────────────────────────┴──────────────────────┤
+  │ true               | scheme://host:port      | scheme://host:port   |
+  └──────────────────────────────────────────────┴──────────────────────┘
+
+
 * ``--static-prefix=<value>`` (default: empty string) - if specified and not an empty string the ``<value>`` will be added to the cache key.
-* ``--capture-prefix=<capture_definition>`` (default: empty string) - if specified and not empty then strings are captured from ``host:port`` based on the ``<capture_definition>`` and are added to the cache key.
+* ``--capture-prefix=<capture_definition>`` (default: empty string) - if specified and not empty then strings are captured based on the value of ``--canonical-prefix`` parameter (see the table above) and ``<capture_definition>`` and are added to the cache key.
 * ``--capture-prefix-uri=<capture_definition>`` (default: empty string) - if specified and not empty then strings are captured from the entire URI based on the ``<capture_definition>`` and are added to the cache key.
 * If any of the "Prefix" related plugin parameters are used together in the plugin configuration they are added to the cache key in the order shown in the diagram.
-* ``--remove-prefix=<true|false|yes|no|0|1`` (default: false) - if specified the prefix elements (host, port) are not processed nor appended to the cachekey. All prefix related plugin parameters are ignored if this parameter is ``true``, ``yes`` or ``1``.
-
-
+* ``--remove-prefix=true|false|yes|no|0|1`` (default: false) - if specified the prefix elements (host, port) are not processed nor appended to the cachekey. All prefix related plugin parameters are ignored if this parameter is ``true``, ``yes`` or ``1``.
+* ``--canonical-prefix=true|false|yes|no|0|1`` (default: false) - impacts the input of regex operation when ``--capture-prefix`` is used and the default value if no prefix parameters are used (see the table above).
 
 "User-Agent" section
 ^^^^^^^^^^^^^^^^^^^^
diff --git a/plugins/cachekey/cachekey.cc b/plugins/cachekey/cachekey.cc
index 9cf4454210f..95640e19005 100644
--- a/plugins/cachekey/cachekey.cc
+++ b/plugins/cachekey/cachekey.cc
@@ -188,6 +188,51 @@ getUri(TSMBuffer buf, TSMLoc url)
   return uri;
 }
 
+static String
+getCanonicalUrl(TSMBuffer buf, TSMLoc url, bool canonicalPrefix, bool provideDefaultKey)
+{
+  String canonicalUrl;
+
+  String scheme;
+  int schemeLen;
+  const char *schemePtr = TSUrlSchemeGet(buf, url, &schemeLen);
+  if (nullptr != schemePtr && 0 != schemeLen) {
+    scheme.assign(schemePtr, schemeLen);
+  } else {
+    CacheKeyError("failed to get scheme");
+    return canonicalUrl;
+  }
+
+  String host;
+  int hostLen;
+  const char *hostPtr = TSUrlHostGet(buf, url, &hostLen);
+  if (nullptr != hostPtr && 0 != hostLen) {
+    host.assign(hostPtr, hostLen);
+  } else {
+    CacheKeyError("failed to get host");
+    return canonicalUrl;
+  }
+
+  String port;
+  int portInt = TSUrlPortGet(buf, url);
+  ::append(port, portInt);
+
+  if (canonicalPrefix) {
+    /* return the same for both regex input or default key, results in 'scheme://host:port' */
+    canonicalUrl.assign(scheme).append("://").append(host).append(":").append(port);
+  } else {
+    if (provideDefaultKey) {
+      /* return the key default - results in '/host/port' */
+      canonicalUrl.assign("/").append(host).append("/").append(port);
+    } else {
+      /* return regex input string - results in 'host:port' (use-case kept for compatibility reasons) */
+      canonicalUrl.assign(host).append(":").append(port);
+    }
+  }
+
+  return canonicalUrl;
+}
+
 /**
  * @brief Constructor setting up the cache key prefix, initializing request info.
  * @param txn transaction handle.
@@ -289,6 +334,16 @@ CacheKey::append(const String &s)
   ::appendEncoded(_key, s.data(), s.size());
 }
 
+void
+CacheKey::append(const String &s, bool useSeparator)
+{
+  if (useSeparator) {
+    append(s);
+  } else {
+    _key.append(s);
+  }
+}
+
 /**
  * @brief Append null-terminated C-style string to the key.
  * @param s null-terminated C-style string.
@@ -319,42 +374,31 @@ CacheKey::append(const char *s, unsigned n)
  * @param prefix if not empty string will append the static prefix to the cache key.
  * @param prefixCapture if not empty will append regex capture/replacement from the host:port.
  * @param prefixCaptureUri if not empty will append regex capture/replacement from the whole URI.
+ * @param canonicalPrefix false - use 'host:port' as starting point of all transformations, true - use 'scheme://host:port'
  * @note if both prefix and pattern are not empty prefix will be added first, followed by the results from pattern.
  */
 void
-CacheKey::appendPrefix(const String &prefix, Pattern &prefixCapture, Pattern &prefixCaptureUri)
+CacheKey::appendPrefix(const String &prefix, Pattern &prefixCapture, Pattern &prefixCaptureUri, bool canonicalPrefix)
 {
-  // "true" would mean that the plugin config meant to override the default prefix (host:port).
+  // "true" would mean that the plugin config meant to override the default prefix, "false" means use default.
   bool customPrefix = false;
-  String host;
-  int port = 0;
+
+  /* For all the following operations if a canonical prefix is required then appned to the key with no separator
+   * to leave the door open for potential valid host name formed in the final resulting cache key. */
 
   if (!prefix.empty()) {
     customPrefix = true;
-    append(prefix);
+    append(prefix, /* useSeparator */ !canonicalPrefix);
     CacheKeyDebug("added static prefix, key: '%s'", _key.c_str());
   }
 
-  int hostLen;
-  const char *hostPtr = TSUrlHostGet(_buf, _url, &hostLen);
-  if (nullptr != hostPtr && 0 != hostLen) {
-    host.assign(hostPtr, hostLen);
-  } else {
-    CacheKeyError("failed to get host");
-  }
-  port = TSUrlPortGet(_buf, _url);
-
   if (!prefixCapture.empty()) {
     customPrefix = true;
 
-    String hostAndPort;
-    hostAndPort.append(host).append(":");
-    ::append(hostAndPort, port);
-
     StringVector captures;
-    if (prefixCapture.process(hostAndPort, captures)) {
+    if (prefixCapture.process(getCanonicalUrl(_buf, _url, canonicalPrefix, /* provideDefaultKey */ false), captures)) {
       for (auto &capture : captures) {
-        append(capture);
+        append(capture, /* useSeparator */ !canonicalPrefix);
       }
       CacheKeyDebug("added host:port capture prefix, key: '%s'", _key.c_str());
     }
@@ -368,7 +412,7 @@ CacheKey::appendPrefix(const String &prefix, Pattern &prefixCapture, Pattern &pr
       StringVector captures;
       if (prefixCaptureUri.process(uri, captures)) {
         for (auto &capture : captures) {
-          append(capture);
+          append(capture, /* useSeparator */ !canonicalPrefix);
         }
         CacheKeyDebug("added URI capture prefix, key: '%s'", _key.c_str());
       }
@@ -376,8 +420,8 @@ CacheKey::appendPrefix(const String &prefix, Pattern &prefixCapture, Pattern &pr
   }
 
   if (!customPrefix) {
-    append(host);
-    append(port);
+    /* nothing was customized => default prefix */
+    append(getCanonicalUrl(_buf, _url, canonicalPrefix, /* provideDefaultKey */ true), /* useSeparator */ false);
     CacheKeyDebug("added default prefix, key: '%s'", _key.c_str());
   }
 }
diff --git a/plugins/cachekey/cachekey.h b/plugins/cachekey/cachekey.h
index 7ea058cb6cf..0922ed104ab 100644
--- a/plugins/cachekey/cachekey.h
+++ b/plugins/cachekey/cachekey.h
@@ -55,9 +55,10 @@ class CacheKey
 
   void append(unsigned number);
   void append(const String &);
+  void append(const String &s, bool useSeparator);
   void append(const char *s);
   void append(const char *n, unsigned s);
-  void appendPrefix(const String &prefix, Pattern &prefixCapture, Pattern &prefixCaptureUri);
+  void appendPrefix(const String &prefix, Pattern &prefixCapture, Pattern &prefixCaptureUri, bool canonicalPrefix);
   void appendPath(Pattern &pathCapture, Pattern &pathCaptureUri);
   void appendHeaders(const ConfigHeaders &config);
   void appendQuery(const ConfigQuery &config);
diff --git a/plugins/cachekey/configs.cc b/plugins/cachekey/configs.cc
index 2320db14336..a56565c7d93 100644
--- a/plugins/cachekey/configs.cc
+++ b/plugins/cachekey/configs.cc
@@ -397,6 +397,8 @@ Configs::init(int argc, const char *argv[], bool perRemapConfig)
     {const_cast<char *>("separator"), optional_argument, nullptr, 's'},
     {const_cast<char *>("uri-type"), optional_argument, nullptr, 't'},
     {const_cast<char *>("capture-header"), optional_argument, nullptr, 'u'},
+    {const_cast<char *>("canonical-prefix"), optional_argument, nullptr, 'v'},
+    /* reserve 'z' for 'config' files */
     {nullptr, 0, nullptr, 0},
   };
 
@@ -504,6 +506,9 @@ Configs::init(int argc, const char *argv[], bool perRemapConfig)
     case 'u': /* capture-header */
       _headers.addCapture(optarg);
       break;
+    case 'v': /* canonical-prefix */
+      _canonicalPrefix = isTrue(optarg);
+      break;
     }
   }
 
@@ -535,6 +540,12 @@ Configs::pathToBeRemoved()
   return _pathToBeRemoved;
 }
 
+bool
+Configs::canonicalPrefix()
+{
+  return _canonicalPrefix;
+}
+
 void
 Configs::setSeparator(const char *arg)
 {
diff --git a/plugins/cachekey/configs.h b/plugins/cachekey/configs.h
index 947b21931ea..5ff41f03ccc 100644
--- a/plugins/cachekey/configs.h
+++ b/plugins/cachekey/configs.h
@@ -162,6 +162,11 @@ class Configs
    */
   bool pathToBeRemoved();
 
+  /**
+   * @brief keep URI scheme and authority elements.
+   */
+  bool canonicalPrefix();
+
   /**
    * @brief set the cache key elements separator string.
    */
@@ -205,6 +210,7 @@ class Configs
 
   bool _prefixToBeRemoved  = false; /**< @brief instructs the prefix (i.e. host:port) not to added to the cache key */
   bool _pathToBeRemoved    = false; /**< @brief instructs the path not to added to the cache key */
+  bool _canonicalPrefix    = false; /**< @brief keep the URI scheme and authority element used as input to transforming into key */
   String _separator        = "/";   /**< @brief a separator used to separate the cache key elements extracted from the URI */
   CacheKeyUriType _uriType = REMAP; /**< @brief shows which URI the cache key will be based on */
 };
diff --git a/plugins/cachekey/plugin.cc b/plugins/cachekey/plugin.cc
index 43bad79e3c0..afb9503b0ce 100644
--- a/plugins/cachekey/plugin.cc
+++ b/plugins/cachekey/plugin.cc
@@ -43,7 +43,7 @@ setCacheKey(TSHttpTxn txn, Configs *config, TSRemapRequestInfo *rri = nullptr)
 
   /* Append custom prefix or the host:port */
   if (!config->prefixToBeRemoved()) {
-    cachekey.appendPrefix(config->_prefix, config->_prefixCapture, config->_prefixCaptureUri);
+    cachekey.appendPrefix(config->_prefix, config->_prefixCapture, config->_prefixCaptureUri, config->canonicalPrefix());
   }
   /* Classify User-Agent and append the class name to the cache key if matched. */
   cachekey.appendUaClass(config->_classifier);