Add range request support to cache fill + global plugin support (#12391)

* Add range request support to cache fill

* Update cache_fill.test.py

* Add config option to make cache_fill only work for range requests

* Update docs

* Add --cache-range-req config option

Update cache_fill.en.rst

Update cache_fill.en.rst

Update cache_fill.en.rst

* Revert test back to original

wont pass in jenkins, passes locally

Fix test

Update cache_fill.test.py

Update cache_fill.test.py

Revert test back to original

Author: jasmine-nahrain

doc/admin-guide/plugins/cache_fill.en.rst

@@ -24,20 +24,69 @@ The initial version of this plugin relays the initial request to the origin serv
 This plugin doesn't provide any improvement for smaller objects but could also degrade the performance as two outgoing requests for every cache update.
 
 
-Using the plugin
-----------------
+Configuration
+-------------
+This plugin functions as either a global or per remap plugin, and it takes an optional argument for
+specifying a config file with inclusion or exclusion criteria. The config file can be specified both
+via an absolute path or via a relative path to the install dir
 
-This plugin functions as a per remap plugin.
+To activate the plugin in global mode, in :file:`plugin.config`, simply add::
 
-To activate the plugin, in :file:`remap.config`, simply append the
+   cache_fill.so --config <config-file>
+
+To activate the plugin in per remap mode, in :file:`remap.config`, simply append the
 below to the specific remap line::
 
    @plugin=cache_fill.so @pparam=<config-file>
 
+include/exclude
+---------------
+The plugin supports a config file that can specify exclusion or inclusion of background fetch
+based on any arbitrary header or client-ip
+
+The contents of the config-file could be as below::
+
+   include User-Agent ABCDEF
+   exclude User-Agent *
+   exclude Content-Type text
+   exclude X-Foo-Bar text
+   exclude Content-Length <1000
+   exclude Client-IP 127.0.0.1
+   include Client-IP 10.0.0.0/16
+
+The ``include`` configuration directive is only used when there is a corresponding ``exclude`` to exempt.
+For example, a single line directive, ``include Host example.com`` would not make the plugin
+*only* act on example.com. To achieve classic allow (only) lists, one would need to have a broad
+exclude line, such as::
+
+   exclude Host *
+   include Host example.com
+
+range-request-only
+------------------
+When set to ``true``, this plugin will only trigger a background fetch if a range header is present.
+Range headers include ``Range``, ``If-Match``, ``If-Modified-Since``, ``If-None-Match``, ``If-Range``
+and ``If-Unmodified-Since``. By default, this is set to false.
+
+This would look like::
+
+    @plugin=cache_fill.so @pparam=--range-request-only
+
+cache-range-req
+---------------
+When set to ``false``. this plugin will not trigger a background fetch for range requests. By default,
+this is set to true.
+Note: you cannot set this to false and ``range-request-only`` to true.
+
+This would look like::
+
+    @plugin=cache_fill.so @pparam=--cache-range-req=false
+
 Functionality
 -------------
 
 Plugin decides to trigger a background fetch of the original (Client) request if the request/response is cacheable and cache status is TS_CACHE_LOOKUP_MISS/TS_CACHE_LOOKUP_HIT_STALE.
+This will work for range requests by making a background fetch and removing the range header. To disable this feature, set ``--cache-range-req=false``
 
 Future additions
 ----------------

plugins/experimental/cache_fill/background_fetch.cc

@@ -45,6 +45,27 @@ namespace cache_fill_ns
 DbgCtl dbg_ctl{PLUGIN_NAME};
 }
 
+///////////////////////////////////////////////////////////////////////////
+// Remove a header (fully) from an TSMLoc / TSMBuffer. Return the number
+// of fields (header values) we removed.
+int
+remove_header(TSMBuffer bufp, TSMLoc hdr_loc, const char *header, int len)
+{
+  TSMLoc field = TSMimeHdrFieldFind(bufp, hdr_loc, header, len);
+  int    cnt   = 0;
+
+  while (field) {
+    TSMLoc tmp = TSMimeHdrFieldNextDup(bufp, hdr_loc, field);
+
+    ++cnt;
+    TSMimeHdrFieldDestroy(bufp, hdr_loc, field);
+    TSHandleMLocRelease(bufp, hdr_loc, field);
+    field = tmp;
+  }
+
+  return cnt;
+}
+
 ///////////////////////////////////////////////////////////////////////////
 // Set a header to a specific value. This will avoid going to through a
 // remove / add sequence in case of an existing header.
@@ -176,6 +197,12 @@ BgFetchData::initialize(TSMBuffer request, TSMLoc req_hdr, TSHttpTxn txnp)
             if (set_header(mbuf, hdr_loc, TS_MIME_FIELD_HOST, TS_MIME_LEN_HOST, hostp, len)) {
               Dbg(dbg_ctl, "Set header Host: %.*s", len, hostp);
             }
+            // Next, remove the Range headers and IMS (conditional) headers from the request
+            for (auto const &header : FILTER_HEADERS) {
+              if (remove_header(mbuf, hdr_loc, header.data(), header.size()) > 0) {
+                Dbg(dbg_ctl, "Removed the %s header from request", header.data());
+              }
+            }
             ret = true;
           }
         }

plugins/experimental/cache_fill/background_fetch.h

@@ -43,6 +43,17 @@
 using OutstandingRequests = std::unordered_map<std::string, bool>;
 const char PLUGIN_NAME[]  = "cache_fill";
 
+// This is the list of all headers that must be removed when we make the actual background
+// fetch request for range requests.
+static const std::array<const std::string_view, 6> FILTER_HEADERS{
+  {{TS_MIME_FIELD_RANGE, static_cast<size_t>(TS_MIME_LEN_RANGE)},
+   {TS_MIME_FIELD_IF_MATCH, static_cast<size_t>(TS_MIME_LEN_IF_MATCH)},
+   {TS_MIME_FIELD_IF_MODIFIED_SINCE, static_cast<size_t>(TS_MIME_LEN_IF_MODIFIED_SINCE)},
+   {TS_MIME_FIELD_IF_NONE_MATCH, static_cast<size_t>(TS_MIME_LEN_IF_NONE_MATCH)},
+   {TS_MIME_FIELD_IF_RANGE, static_cast<size_t>(TS_MIME_LEN_IF_RANGE)},
+   {TS_MIME_FIELD_IF_UNMODIFIED_SINCE, static_cast<size_t>(TS_MIME_LEN_IF_UNMODIFIED_SINCE)}}
+};
+
 namespace cache_fill_ns
 {
 extern DbgCtl dbg_ctl;

plugins/experimental/cache_fill/cache_fill.cc

@@ -41,6 +41,9 @@
 #include "background_fetch.h"
 #include "configs.h"
 
+// Global config, if we don't have a remap specific config.
+static BgFetchConfig *gConfig = nullptr;
+
 static const char *
 getCacheLookupResultName(TSCacheLookupResult result)
 {
@@ -137,6 +140,34 @@ cont_handle_cache(TSCont contp, TSEvent event, void *edata)
   return 0;
 }
 
+///////////////////////////////////////////////////////////////////////////
+// Setup global hooks
+void
+TSPluginInit(int argc, const char *argv[])
+{
+  TSPluginRegistrationInfo info;
+
+  info.plugin_name   = (char *)PLUGIN_NAME;
+  info.vendor_name   = (char *)"Apache Software Foundation";
+  info.support_email = (char *)"dev@trafficserver.apache.org";
+
+  if (TS_SUCCESS != TSPluginRegister(&info)) {
+    TSError("[%s] Plugin registration failed", PLUGIN_NAME);
+  }
+
+  TSCont cont = TSContCreate(cont_handle_cache, nullptr);
+
+  gConfig = new BgFetchConfig(cont);
+
+  if (gConfig->parseOptions(argc, argv)) {
+    Dbg(dbg_ctl, "cache fill plugin is successfully initialized globally");
+    TSHttpHookAdd(TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, cont);
+  } else {
+    // ToDo: Hmmm, no way to fail a global plugin here?
+    Dbg(dbg_ctl, "Failed to initialize as global plugin");
+  }
+}
+
 ///////////////////////////////////////////////////////////////////////////
 // Setup Remap mode
 ///////////////////////////////////////////////////////////////////////////////

plugins/experimental/cache_fill/configs.cc

@@ -42,19 +42,19 @@ bool
 BgFetchConfig::parseOptions(int argc, const char *argv[])
 {
   static const struct option longopt[] = {
-    {const_cast<char *>("log"),       required_argument, nullptr, 'l' },
-    {const_cast<char *>("config"),    required_argument, nullptr, 'c' },
-    {const_cast<char *>("allow-304"), no_argument,       nullptr, 'a' },
-    {nullptr,                         no_argument,       nullptr, '\0'}
+    {const_cast<char *>("log"),             required_argument, nullptr, 'l' },
+    {const_cast<char *>("config"),          required_argument, nullptr, 'c' },
+    {const_cast<char *>("range-req-only"),  optional_argument, nullptr, 'r' },
+    {const_cast<char *>("cache-range-req"), optional_argument, nullptr, 'a' },
+    {nullptr,                               no_argument,       nullptr, '\0'},
   };
 
   while (true) {
-    int opt = getopt_long(argc, const_cast<char *const *>(argv), "lc", longopt, nullptr);
+    int opt = getopt_long(argc, const_cast<char *const *>(argv), "", longopt, nullptr);
 
     if (opt == -1) {
       break;
     }
-
     switch (opt) {
     case 'l':
       Dbg(dbg_ctl, "option: log file specified: %s", optarg);
@@ -67,9 +67,13 @@ BgFetchConfig::parseOptions(int argc, const char *argv[])
         return false;
       }
       break;
+    case 'r':
+      Dbg(dbg_ctl, "option: --range-req-only set");
+      _range_req_only = isTrue(optarg);
+      break;
     case 'a':
-      Dbg(dbg_ctl, "option: --allow-304 set");
-      _allow_304 = true;
+      Dbg(dbg_ctl, "option: --cache-range-req set");
+      _cache_range_req = isTrue(optarg);
       break;
     default:
       TSError("[%s] invalid plugin option: %c", PLUGIN_NAME, opt);
@@ -78,6 +82,11 @@ BgFetchConfig::parseOptions(int argc, const char *argv[])
     }
   }
 
+  if (_range_req_only && !_cache_range_req) {
+    TSError("[%s] Cannot define _range_req_only=true and _cache_range_req=false", PLUGIN_NAME);
+    return false;
+  }
+
   return true;
 }
 
@@ -198,8 +207,29 @@ BgFetchConfig::bgFetchAllowed(TSHttpTxn txnp) const
     return false;
   }
 
-  bool allow_bg_fetch = true;
+  if (_range_req_only || !_cache_range_req) {
+    TSMBuffer bufp;
+    TSMLoc    hdr_loc;
+    if (TSHttpTxnClientReqGet(txnp, &bufp, &hdr_loc) == TS_SUCCESS) {
+      bool hasRangeHdrs = false;
+      for (auto const &header : FILTER_HEADERS) {
+        if (TSMimeHdrFieldFind(bufp, hdr_loc, header.data(), header.size() == TS_SUCCESS)) {
+          hasRangeHdrs = true;
+          break;
+        }
+      }
+      if (!hasRangeHdrs && _range_req_only) {
+        Dbg(dbg_ctl, "_range_req_only=true; This transaction is not a range request");
+        return false;
+      }
+      if (hasRangeHdrs && !_cache_range_req) {
+        Dbg(dbg_ctl, "_cache_range_req=false; This transaction is a range request");
+        return false;
+      }
+    }
+  }
 
+  bool allow_bg_fetch = true;
   // We could do this recursively, but following the linked list is probably more efficient.
   for (auto const &r : _rules) {
     if (r.check_field_configured(txnp)) {

plugins/experimental/cache_fill/configs.h

@@ -70,10 +70,10 @@ class BgFetchConfig
     return _log_file;
   }
 
-  bool
-  allow304() const
+  static bool
+  isTrue(const char *arg)
   {
-    return _allow_304;
+    return (nullptr == arg || 0 == strncasecmp("true", arg, 4) || 0 == strncasecmp("1", arg, 1) || 0 == strncasecmp("yes", arg, 3));
   }
 
   // This parses and populates the BgFetchRule linked list (_rules).
@@ -84,6 +84,7 @@ class BgFetchConfig
 private:
   TSCont      _cont = nullptr;
   list_type   _rules;
-  bool        _allow_304 = false;
+  bool        _range_req_only  = false;
+  bool        _cache_range_req = true;
   std::string _log_file;
 };