From 8b780124298e4aae3cad4c06c4becd4574ecf69f Mon Sep 17 00:00:00 2001
From: Jeff Elsloo <elsloo@apache.org>
Date: Wed, 29 Sep 2021 09:57:09 -0600
Subject: [PATCH] Added support for verifying cacheability before attempting to
 force an object into cache.

---
 .../plugins/cache_range_requests.en.rst        | 18 ++++++++++++++++++
 plugins/cache_range_requests/README.md         | 17 +++++++++++++++++
 .../cache_range_requests.cc                    | 15 +++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/doc/admin-guide/plugins/cache_range_requests.en.rst b/doc/admin-guide/plugins/cache_range_requests.en.rst
index 0189122f36a..eb01960f707 100644
--- a/doc/admin-guide/plugins/cache_range_requests.en.rst
+++ b/doc/admin-guide/plugins/cache_range_requests.en.rst
@@ -161,6 +161,24 @@ incorrectly *WILL* result in cache poisoning.
 generated in the logs and the cache_range_requests plugin will disable
 transaction caching in order to avoid cache poisoning.
 
+Verify Cacheability
+-------------------
+
+.. option:: --verify-cacheability
+.. option:: -v
+
+This option causes the plugin to verify whether the requested object is
+cacheable.
+
+By default, an object's cacheability is not verified after
+the plugin changes the response code of the upstream response from 206
+to 200 to force the object into cache. When this option is enabled,
+cacheability is considered, and if the object is not cacheable, the
+status code is reset back to 206, which leads to the object not being cached.
+
+This option is useful when used with other plugins, such as Cache Promote.
+
+
 Configuration examples
 ======================
 
diff --git a/plugins/cache_range_requests/README.md b/plugins/cache_range_requests/README.md
index ef567a70944..23250bef9d7 100644
--- a/plugins/cache_range_requests/README.md
+++ b/plugins/cache_range_requests/README.md
@@ -78,3 +78,20 @@ X-CRR-IMS header support
 
     Consider using the header_rewrite plugin to protect the parent
 		from using this option as an attack vector against an origin.
+
+Object Cacheability:
+    Normally objects are forced into the cache by changing the status code in the
+    response from the upstream host from 206 to 200. The default behavior is to
+    perform this operation blindly without checking cacheability. Add the `-v`
+    flag to cause the plugin to ensure the object is cacheable; when it is not,
+    the 206 status code is restored and the object will not be cached.
+
+    Global Plugin (plugin.config):
+
+      cache_range_requests.so --verify-cacheability
+      cache_range_requests.so -v
+
+    Remap Plugin (remap.config):
+
+      <from-url> <to-url> @plugin=cache_range_requests.so @pparam=--verify-cacheability
+      <from-url> <to-url> @plugin=cache_range_requests.so @pparam=-v
diff --git a/plugins/cache_range_requests/cache_range_requests.cc b/plugins/cache_range_requests/cache_range_requests.cc
index b3f397aadfa..1b777d3c0d0 100644
--- a/plugins/cache_range_requests/cache_range_requests.cc
+++ b/plugins/cache_range_requests/cache_range_requests.cc
@@ -51,11 +51,13 @@ struct pluginconfig {
   parent_select_mode_t ps_mode{PS_DEFAULT};
   bool consider_ims_header{false};
   bool modify_cache_key{true};
+  bool verify_cacheability{false};
 };
 
 struct txndata {
   std::string range_value;
   time_t ims_time{0};
+  bool verify_cacheability{false};
 };
 
 // Header for optional revalidation
@@ -99,6 +101,7 @@ create_pluginconfig(int argc, char *const argv[])
     {const_cast<char *>("ps-cachekey"), no_argument, nullptr, 'p'},
     {const_cast<char *>("consider-ims"), no_argument, nullptr, 'c'},
     {const_cast<char *>("no-modify-cachekey"), no_argument, nullptr, 'n'},
+    {const_cast<char *>("verify-cacheability"), no_argument, nullptr, 'v'},
     {nullptr, 0, nullptr, 0},
   };
 
@@ -125,6 +128,10 @@ create_pluginconfig(int argc, char *const argv[])
       DEBUG_LOG("Plugin doesn't modify cache key");
       pc->modify_cache_key = false;
     } break;
+    case 'v': {
+      DEBUG_LOG("Plugin verifies whether the object in the transaction is cacheable");
+      pc->verify_cacheability = true;
+    } break;
     default: {
     } break;
     }
@@ -254,6 +261,8 @@ range_header_check(TSHttpTxn txnp, pluginconfig *const pc)
                 }
               }
             }
+
+            txn_state->verify_cacheability = pc->verify_cacheability;
           }
 
           // remove the range request header.
@@ -371,6 +380,12 @@ handle_server_read_response(TSHttpTxn txnp, txndata *const txn_state)
       DEBUG_LOG("Set response header to TS_HTTP_STATUS_OK.");
       bool cacheable = TSHttpTxnIsCacheable(txnp, nullptr, resp_buf);
       DEBUG_LOG("range is cacheable: %d", cacheable);
+      DEBUG_LOG("verify cacheability: %d", txn_state->verify_cacheability);
+
+      if (txn_state->verify_cacheability && !cacheable) {
+        DEBUG_LOG("transaction is not cacheable; resetting status code to 206");
+        TSHttpHdrStatusSet(resp_buf, resp_loc, TS_HTTP_STATUS_PARTIAL_CONTENT);
+      }
     } else if (TS_HTTP_STATUS_OK == status) {
       DEBUG_LOG("The origin does not support range requests, attempting to disable cache write.");
       if (TS_SUCCESS == TSHttpTxnServerRespNoStoreSet(txnp, 1)) {