From 8c9b11de688c7053dbbda79539dad1aa287e40de Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Fri, 14 Nov 2025 23:26:44 +0000 Subject: [PATCH] negative_caching_lifetime and ttl-in-cache interaction When both negative_caching_lifetime and cache.config ttl-in-cache are configured, ttl-in-cache was incorrectly overriding negative_caching_lifetime for negative responses (404, 403, 500, etc.). This caused negative responses to be cached for the ttl-in-cache duration instead of the intended negative_caching_lifetime. The fix modifies what_is_document_freshness() to skip the ttl-in-cache check for negatively cached responses. --- src/proxy/http/HttpTransact.cc | 27 ++++-- .../gold_tests/cache/negative-caching.test.py | 29 ++++++ .../negative-caching-ttl-in-cache.replay.yaml | 96 +++++++++++++++++++ 3 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 tests/gold_tests/cache/replay/negative-caching-ttl-in-cache.replay.yaml diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc index 654acfa1797..8382564157a 100644 --- a/src/proxy/http/HttpTransact.cc +++ b/src/proxy/http/HttpTransact.cc @@ -7270,18 +7270,27 @@ HttpTransact::what_is_document_freshness(State *s, HTTPHdr *client_request, HTTP ////////////////////////////////////////////////////// // If config file has a ttl-in-cache field set, // // it has priority over any other http headers and // - // other configuration parameters. // + // other configuration parameters. Negative caching // + // is different however since the user would // + // rather expect their explicitly configured // + // negative_caching_lifetime to be used instead of // + // ttl-in-cache. // ////////////////////////////////////////////////////// if (s->cache_control.ttl_in_cache > 0) { - // what matters if ttl is set is not the age of the document - // but for how long it has been stored in the cache (resident time) - int resident_time = s->current.now - s->response_received_time; - - TxnDbg(dbg_ctl_http_match, "ttl-in-cache = %d, resident time = %d", s->cache_control.ttl_in_cache, resident_time); - if (resident_time > s->cache_control.ttl_in_cache) { - return (Freshness_t::STALE); + auto status = static_cast(cached_obj_response->status_get()); + if (s->txn_conf->negative_caching_enabled && s->txn_conf->negative_caching_list.contains(status)) { + TxnDbg(dbg_ctl_http_match, "ttl-in-cache set, but skipping for negative cached response %d", status); } else { - return (Freshness_t::FRESH); + // what matters if ttl is set is not the age of the document + // but for how long it has been stored in the cache (resident time) + int resident_time = s->current.now - s->response_received_time; + + TxnDbg(dbg_ctl_http_match, "ttl-in-cache = %d, resident time = %d", s->cache_control.ttl_in_cache, resident_time); + if (resident_time > s->cache_control.ttl_in_cache) { + return (Freshness_t::STALE); + } else { + return (Freshness_t::FRESH); + } } } diff --git a/tests/gold_tests/cache/negative-caching.test.py b/tests/gold_tests/cache/negative-caching.test.py index 1c5cf2ac0ed..bcd0868aefd 100644 --- a/tests/gold_tests/cache/negative-caching.test.py +++ b/tests/gold_tests/cache/negative-caching.test.py @@ -151,3 +151,32 @@ tr.AddVerifierServerProcess("server-no-timeout", replay_file, http_ports=[server_port]) tr.AddVerifierClientProcess("client-no-timeout", replay_file, http_ports=[ts.Variables.port]) tr.StillRunningAfter = ts + +# +# Verify that negative_caching_lifetime is respected even when cache.config +# has ttl-in-cache configured. +# +replay_file = "replay/negative-caching-ttl-in-cache.replay.yaml" +tr = Test.AddTestRun("Verify negative_caching_lifetime and ttl-in-cache interaction.") +dns = tr.MakeDNServer("dns", default="127.0.0.1") +server = tr.AddVerifierServerProcess("server-ttl-in-cache", replay_file) +server_port = server.Variables.http_port +ts = tr.MakeATSProcess("ts-ttl-in-cache") +ts.Disk.records_config.update( + { + 'proxy.config.dns.nameservers': f"127.0.0.1:{dns.Variables.Port}", + 'proxy.config.dns.resolv_conf': 'NULL', + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http', + 'proxy.config.http.insert_age_in_response': 0, + 'proxy.config.http.negative_caching_enabled': 1, + 'proxy.config.http.negative_caching_lifetime': 2 + }) +ts.Disk.remap_config.AddLine(f'map / http://backend.example.com:{server_port}') +# Configure cache.config with a long ttl-in-cache that should NOT override +# negative_caching_lifetime for negative responses. +ts.Disk.cache_config.AddLine('dest_domain=backend.example.com ttl-in-cache=30d') +p = tr.AddVerifierClientProcess("client-ttl-in-cache", replay_file, http_ports=[ts.Variables.port]) +p.StartBefore(dns) +p.StartBefore(server) +p.StartBefore(ts) diff --git a/tests/gold_tests/cache/replay/negative-caching-ttl-in-cache.replay.yaml b/tests/gold_tests/cache/replay/negative-caching-ttl-in-cache.replay.yaml new file mode 100644 index 00000000000..e10009844f0 --- /dev/null +++ b/tests/gold_tests/cache/replay/negative-caching-ttl-in-cache.replay.yaml @@ -0,0 +1,96 @@ +# 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. + +# +# Initial request that caches a 404 response. +# This tests that negative_caching_lifetime is respected even when +# ttl-in-cache is configured. +# + +meta: + version: "1.0" + +sessions: +- transactions: + + # + # Test 1: Cache a 404 response. + # + - client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/404 + headers: + fields: + - [ Host, example.com ] + - [ uuid, 100 ] + + server-response: + status: 404 + reason: "Not Found" + headers: + fields: + - [ Content-Length, 8 ] + + proxy-response: + status: 404 + + # Repeat the request and verify that the 404 is served from the cache. + - client-request: + # Add a delay so ATS has time to finish any caching IO for the previous + # transaction. + delay: 100ms + + method: "GET" + version: "1.1" + scheme: "http" + url: /path/404 + headers: + fields: + - [ Host, example.com ] + - [ uuid, 101 ] + + + # The following should not reach the server since the 404 is cached. + server-response: + status: 200 + reason: OK + + # Verify the cached 404 response is served. + proxy-response: + status: 404 + + + # Delay for 3 seconds to exceed negative_caching_lifetime and verify that the + # 200 OK is served from the server rather than the cached 404. + - client-request: + delay: 3s + method: "GET" + version: "1.1" + scheme: "http" + url: /path/404 + headers: + fields: + - [ Host, example.com ] + - [ uuid, 102 ] + + server-response: + status: 200 + reason: OK + + proxy-response: + status: 200