Support transforming range requests when origin returns full resource.

doc/admin-guide/files/records.config.en.rst

@@ -2278,10 +2278,11 @@ Cache Control
    :overridable:
 
    When enabled (``1``), |TS| will attempt to write (lock) the URL
-   to cache. This is rarely useful (at the moment), since it'll only be able
-   to write to cache if the origin has ignored the ``Range:`` header. For a use
-   case where you know the origin will respond with a full (``200``) response,
-   you can turn this on to allow it to be cached.
+   to cache for a request specifying a range. This is useful when the origin server
+   might ignore a range request and respond with a full (``200``) response.
+   Additionally, this setting will attempt to transform a 200 response from the origin
+   server to a partial (``206``) response, honoring the requested range, while
+   caching the full response.
 
 .. ts:cv:: CONFIG proxy.config.http.cache.ignore_accept_mismatch INT 2
    :reloadable:

proxy/http/HttpSM.cc

@@ -4594,15 +4594,19 @@ HttpSM::parse_range_and_compare(MIMEField *field, int64_t content_length)
     ranges[nr]._end   = end;
     ++nr;
 
-    if (!cache_sm.cache_read_vc->is_pread_capable() && cache_config_read_while_writer == 2) {
-      // write in progress, check if request range not in cache yet
-      HTTPInfo::FragOffset *frag_offset_tbl = t_state.cache_info.object_read->get_frag_table();
-      int frag_offset_cnt                   = t_state.cache_info.object_read->get_frag_offset_count();
-
-      if (!frag_offset_tbl || !frag_offset_cnt || (frag_offset_tbl[frag_offset_cnt - 1] < static_cast<uint64_t>(end))) {
-        Debug("http_range", "request range in cache, end %" PRId64 ", frg_offset_cnt %d" PRId64, end, frag_offset_cnt);
-        t_state.range_in_cache = false;
+    if (cache_sm.cache_read_vc) {
+      if (!cache_sm.cache_read_vc->is_pread_capable() && cache_config_read_while_writer == 2) {
+        // write in progress, check if request range not in cache yet
+        HTTPInfo::FragOffset *frag_offset_tbl = t_state.cache_info.object_read->get_frag_table();
+        int frag_offset_cnt                   = t_state.cache_info.object_read->get_frag_offset_count();
+
+        if (!frag_offset_tbl || !frag_offset_cnt || (frag_offset_tbl[frag_offset_cnt - 1] < static_cast<uint64_t>(end))) {
+          Debug("http_range", "request range in cache, end %" PRId64 ", frg_offset_cnt %d" PRId64, end, frag_offset_cnt);
+          t_state.range_in_cache = false;
+        }
       }
+    } else {
+      t_state.range_in_cache = false;
     }
   }
 
@@ -4656,10 +4660,16 @@ HttpSM::calculate_output_cl(int64_t num_chars_for_ct, int64_t num_chars_for_cl)
 void
 HttpSM::do_range_parse(MIMEField *range_field)
 {
-  int num_chars_for_ct = 0;
-  t_state.cache_info.object_read->response_get()->value_get(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE, &num_chars_for_ct);
+  int num_chars_for_ct   = 0;
+  int64_t content_length = 0;
 
-  int64_t content_length   = t_state.cache_info.object_read->object_size_get();
+  if (t_state.cache_info.object_read != nullptr) {
+    t_state.cache_info.object_read->response_get()->value_get(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE, &num_chars_for_ct);
+    content_length = t_state.cache_info.object_read->object_size_get();
+  } else {
+    content_length = t_state.hdr_info.server_response.get_content_length();
+    t_state.hdr_info.server_response.value_get(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE, &num_chars_for_ct);
+  }
   int64_t num_chars_for_cl = num_chars_for_int(content_length);
 
   parse_range_and_compare(range_field, content_length);
@@ -4674,8 +4684,6 @@ HttpSM::do_range_setup_if_necessary()
 {
   MIMEField *field;
 
-  ink_assert(t_state.cache_info.object_read != nullptr);
-
   field = t_state.hdr_info.client_request.field_find(MIME_FIELD_RANGE, MIME_LEN_RANGE);
   ink_assert(field != nullptr);
 
@@ -4687,7 +4695,7 @@ HttpSM::do_range_setup_if_necessary()
     if (t_state.range_setup == HttpTransact::RANGE_REQUESTED) {
       bool do_transform = false;
 
-      if (!t_state.range_in_cache) {
+      if (!t_state.range_in_cache && t_state.cache_info.object_read) {
         Debug("http_range", "range can't be satisfied from cache, force origin request");
         t_state.cache_lookup_result = HttpTransact::CACHE_LOOKUP_MISS;
         return;
@@ -4705,7 +4713,12 @@ HttpSM::do_range_setup_if_necessary()
           t_state.range_setup      = HttpTransact::RANGE_NOT_SATISFIABLE;
         }
       } else {
-        if (cache_sm.cache_read_vc->is_pread_capable()) {
+        // if revalidating and cache is stale we want to transform
+        if (t_state.cache_info.action == HttpTransact::CACHE_DO_REPLACE &&
+            t_state.hdr_info.server_response.status_get() == HTTP_STATUS_OK) {
+          Debug("http_range", "Serving transform after stale cache re-serve");
+          do_transform = true;
+        } else if (cache_sm.cache_read_vc && cache_sm.cache_read_vc->is_pread_capable()) {
           // If only one range entry and pread is capable, no need transform range
           t_state.range_setup = HttpTransact::RANGE_NOT_TRANSFORM_REQUESTED;
         } else {
@@ -4717,15 +4730,34 @@ HttpSM::do_range_setup_if_necessary()
       if (do_transform) {
         if (api_hooks.get(TS_HTTP_RESPONSE_TRANSFORM_HOOK) == nullptr) {
           int field_content_type_len = -1;
-          const char *content_type   = t_state.cache_info.object_read->response_get()->value_get(
-            MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE, &field_content_type_len);
+          const char *content_type   = nullptr;
+          int64_t content_length     = 0;
+
+          if (t_state.cache_info.object_read && t_state.cache_info.action != HttpTransact::CACHE_DO_REPLACE) {
+            content_type = t_state.cache_info.object_read->response_get()->value_get(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE,
+                                                                                     &field_content_type_len);
+            content_length = t_state.cache_info.object_read->object_size_get();
+          } else {
+            // We don't want to transform a range request if the server response has a content encoding.
+            if (t_state.hdr_info.server_response.presence(MIME_PRESENCE_CONTENT_ENCODING)) {
+              Debug("http_trans", "Cannot setup range transform for server response with content encoding");
+              t_state.range_setup = HttpTransact::RANGE_NONE;
+              return;
+            }
+
+            // Since we are transforming the range from the server, we want to cache the original response
+            t_state.api_info.cache_untransformed = true;
+            content_type =
+              t_state.hdr_info.server_response.value_get(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE, &field_content_type_len);
+            content_length = t_state.hdr_info.server_response.get_content_length();
+          }
 
           Debug("http_trans", "Unable to accelerate range request, fallback to transform");
 
           // create a Range: transform processor for requests of type Range: bytes=1-2,4-5,10-100 (eg. multiple ranges)
-          INKVConnInternal *range_trans = transformProcessor.range_transform(
-            mutex.get(), t_state.ranges, t_state.num_range_fields, &t_state.hdr_info.transform_response, content_type,
-            field_content_type_len, t_state.cache_info.object_read->object_size_get());
+          INKVConnInternal *range_trans = transformProcessor.range_transform(mutex.get(), t_state.ranges, t_state.num_range_fields,
+                                                                             &t_state.hdr_info.transform_response, content_type,
+                                                                             field_content_type_len, content_length);
           api_hooks.append(TS_HTTP_RESPONSE_TRANSFORM_HOOK, range_trans);
         } else {
           // ToDo: Do we do something here? The theory is that multiple transforms do not behave well with
@@ -6080,7 +6112,8 @@ HttpSM::perform_transform_cache_write_action()
           HttpDebugNames::get_cache_action_name(t_state.cache_info.action));
 
   if (t_state.range_setup) {
-    return;
+    SMDebug("http", "[%" PRId64 "] perform_transform_cache_write_action %s (with range setup)", sm_id,
+            HttpDebugNames::get_cache_action_name(t_state.cache_info.action));
   }
 
   switch (t_state.cache_info.transform_action) {
@@ -6091,10 +6124,12 @@ HttpSM::perform_transform_cache_write_action()
   }
 
   case HttpTransact::CACHE_DO_WRITE: {
-    transform_cache_sm.close_read();
-    t_state.cache_info.transform_write_status = HttpTransact::CACHE_WRITE_IN_PROGRESS;
-    setup_cache_write_transfer(&transform_cache_sm, transform_info.entry->vc, &t_state.cache_info.transform_store,
-                               client_response_hdr_bytes, "cache write t");
+    if (t_state.api_info.cache_untransformed == false) {
+      transform_cache_sm.close_read();
+      t_state.cache_info.transform_write_status = HttpTransact::CACHE_WRITE_IN_PROGRESS;
+      setup_cache_write_transfer(&transform_cache_sm, transform_info.entry->vc, &t_state.cache_info.transform_store,
+                                 client_response_hdr_bytes, "cache write t");
+    }
     break;
   }
 

proxy/http/HttpTransact.cc

@@ -4558,6 +4558,10 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s)
       } else {
         ink_assert(s->cache_info.object_read != nullptr);
         s->cache_info.action = CACHE_DO_REPLACE;
+
+        if (s->hdr_info.client_request.presence(MIME_PRESENCE_RANGE)) {
+          s->state_machine->do_range_setup_if_necessary();
+        }
       }
 
     } else if (s->cache_info.action == CACHE_DO_WRITE) {
@@ -4567,6 +4571,9 @@ HttpTransact::handle_cache_operation_on_forward_server_response(State *s)
         s->cache_info.action = CACHE_DO_NO_ACTION;
       } else {
         s->cache_info.action = CACHE_DO_WRITE;
+        if (s->hdr_info.client_request.presence(MIME_PRESENCE_RANGE)) {
+          s->state_machine->do_range_setup_if_necessary();
+        }
       }
 
     } else if (s->cache_info.action == CACHE_DO_DELETE) {

tests/gold_tests/cache/cache-range-response.test.py

@@ -0,0 +1,40 @@
+'''
+Verify that caching a range request when origin returns full response works.
+'''
+#  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.
+
+Test.Summary = '''
+Verify correct caching behavior for range requests.
+'''
+
+ts = Test.MakeATSProcess("ts")
+replay_file = "replay/cache-range-response.replay.yaml"
+server = Test.MakeVerifierServerProcess("server0", replay_file)
+ts.Disk.records_config.update({
+    'proxy.config.diags.debug.enabled': 1,
+    'proxy.config.diags.debug.tags': 'http.*|cache.*',
+    'proxy.config.http.cache.range.write': 1,
+    'proxy.config.http.cache.when_to_revalidate': 4,
+    'proxy.config.http.insert_response_via_str': 3,
+})
+ts.Disk.remap_config.AddLine(
+    f'map / http://127.0.0.1:{server.Variables.http_port}'
+)
+tr = Test.AddTestRun("Verify range request is transformed from a 200 response")
+tr.Processes.Default.StartBefore(server)
+tr.Processes.Default.StartBefore(ts)
+tr.AddVerifierClientProcess("client0", replay_file, http_ports=[ts.Variables.port])

tests/gold_tests/cache/replay/cache-range-response.replay.yaml

@@ -0,0 +1,113 @@
+#  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.
+
+---
+meta:
+  version: "1.0"
+
+sessions:
+  - transactions:
+      # Populate the cache with a response to a GET request.
+      - client-request:
+          method: "GET"
+          version: "1.1"
+          url: /some/path
+          headers:
+            fields:
+              - [ Host, example.com ]
+              - [ uuid, 1 ]
+              - [ Range, bytes=0-10 ]
+        server-response:
+          status: 200
+          reason: OK
+          headers:
+            fields:
+              - [ Content-Length, 16 ]
+              - [ Cache-Control, max-age=300 ]
+              - [ X-Response, first_get_response ]
+        proxy-response:
+          status: 206
+          headers:
+            fields:
+              - [ X-Response, { value: first_get_response, as: equal} ]
+              - [ Content-Range, { value: "bytes 0-10/16", as: equal}]
+      # Subsequent range request served from cache
+      - client-request:
+          method: "GET"
+          version: "1.1"
+          url: /some/path
+          headers:
+            fields:
+              - [ Host, example.com ]
+              - [ uuid, 2 ]
+              - [ Range, bytes=0-5 ]
+        server-response:
+          status: 500
+          reason: OK
+          headers:
+            fields:
+              - [ X-Response, internal_server_error ]
+        proxy-response:
+          status: 206
+          headers:
+            fields:
+              - [ X-Response, { value: first_get_response, as: equal} ]
+              - [ Content-Range, { value: "bytes 0-5/16", as: equal}]
+      # Should get full response from cache without a range header
+      - client-request:
+          method: "GET"
+          version: "1.1"
+          url: /some/path
+          headers:
+            fields:
+              - [ Host, example.com ]
+              - [ uuid, 3 ]
+        server-response:
+          status: 500
+          reason: OK
+          headers:
+            fields:
+              - [ X-Response, internal_server_error ]
+        proxy-response:
+          status: 200
+          headers:
+            fields:
+              - [ X-Response, { value: first_get_response, as: equal} ]
+              - [ Content-Length, { value: "16", as: equal}]
+      # Revalidate and replace cache still returns 206
+      - client-request:
+          method: "GET"
+          version: "1.1"
+          url: /some/path
+          headers:
+            fields:
+              - [ Host, example.com ]
+              - [ uuid, 4 ]
+              - [ Range, bytes=0-10 ]
+              - [ If-Modified-Since, "Wed, 16 Mar 2022 22:52:09 GMT"]
+        server-response:
+          status: 200
+          reason: OK
+          headers:
+            fields:
+              - [ Content-Length, 100 ]
+              - [ Cache-Control, max-age=300 ]
+        proxy-response:
+          status: 206
+          headers:
+            fields:
+              - [ Content-Range, { value: "bytes 0-10/100", as: equal}]
+              - [ Via, { value: "uIcSsSfUpSeN:t cCSp sS", as: contains }]