diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index cc0747d3adf..18c0e2a863f 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/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: diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 1a1236bbce5..93d77230dfc 100644 --- a/proxy/http/HttpSM.cc +++ b/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(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(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; } diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index aeb91b35d9c..77418971aa2 100644 --- a/proxy/http/HttpTransact.cc +++ b/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) { diff --git a/tests/gold_tests/cache/cache-range-response.test.py b/tests/gold_tests/cache/cache-range-response.test.py new file mode 100644 index 00000000000..4175bb5d9a6 --- /dev/null +++ b/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]) diff --git a/tests/gold_tests/cache/replay/cache-range-response.replay.yaml b/tests/gold_tests/cache/replay/cache-range-response.replay.yaml new file mode 100644 index 00000000000..768f0ddd187 --- /dev/null +++ b/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 }]