From 0c86bee4ffeb0cd2b3006f4d8aa205a943353b32 Mon Sep 17 00:00:00 2001 From: bneradt Date: Mon, 20 Jul 2026 16:58:48 -0500 Subject: [PATCH] Cache empty chunked responses Empty chunked responses can complete without starting a cache write VIO, or can start with an unknown length that is later finalized at zero. ATS treats both as empty unwritten entries and sends later requests back to origin. AuTests can also cross a log-rolling boundary before checking custom logs, producing an unrelated intermittent failure. This patch starts zero-byte cache writes and recognizes successfully closed write VIOs whose final length is zero. This preserves the empty-document state while keeping header-only cache updates distinct. This also disables log rolling for stale-response log assertions and covers negative and successful empty responses. Fixes: #11313 --- src/iocore/cache/CacheVC.cc | 19 ++++++- src/iocore/cache/P_CacheInternal.h | 1 + src/proxy/http/HttpTunnel.cc | 14 +++-- ...ive-caching-300-second-timeout.replay.yaml | 51 ++++++++++++++++++- .../stale_response/stale_response.test.py | 2 + 5 files changed, 82 insertions(+), 5 deletions(-) diff --git a/src/iocore/cache/CacheVC.cc b/src/iocore/cache/CacheVC.cc index 83bc9ea8d6a..b7443703b14 100644 --- a/src/iocore/cache/CacheVC.cc +++ b/src/iocore/cache/CacheVC.cc @@ -213,6 +213,14 @@ CacheVC::do_io_write(Continuation *c, int64_t nbytes, IOBufferReader *abuf, bool #ifdef DEBUG ink_assert(!c || c->mutex->thread_holding); #endif + if (nbytes == 0) { + // A zero-byte write represents an empty document, while closing without a + // write represents a header-only update. + f.allow_empty_doc = 1; + if (alternate.valid()) { + alternate.object_size_set(0); + } + } if (c && !trigger && !recursive) { trigger = c->mutex->thread_holding->schedule_imm_local(this); } @@ -223,6 +231,14 @@ void CacheVC::do_io_close(int alerrno) { ink_assert(mutex->thread_holding == this_ethread()); + if (alerrno == -1 && vio.op == VIO::WRITE && vio.get_reader() != nullptr && vio.nbytes == 0) { + // The write may have started with an unknown length and been finalized + // at zero after the response framing was parsed. + f.allow_empty_doc = 1; + if (alternate.valid()) { + alternate.object_size_set(0); + } + } int previous_closed = closed; closed = (alerrno == -1) ? 1 : -1; // Stupid default arguments DDbg(dbg_ctl_cache_close, "do_io_close %p %d %d", this, alerrno, closed); @@ -1063,7 +1079,8 @@ CacheVC::set_http_info(CacheHTTPInfo *ainfo) } MIMEField *field = ainfo->m_alt->m_response_hdr.field_find(static_cast(MIME_FIELD_CONTENT_LENGTH)); - if ((field && !field->value_get_int64()) || ainfo->m_alt->m_response_hdr.status_get() == HTTPStatus::NO_CONTENT) { + if ((field && !field->value_get_int64()) || ainfo->m_alt->m_response_hdr.status_get() == HTTPStatus::NO_CONTENT || + (f.allow_empty_doc && vio.nbytes == 0)) { f.allow_empty_doc = 1; // Set the object size here to zero in case this is a cache replace where the new object // length is zero but the old object was not. diff --git a/src/iocore/cache/P_CacheInternal.h b/src/iocore/cache/P_CacheInternal.h index c030fe211d2..f831ed51587 100644 --- a/src/iocore/cache/P_CacheInternal.h +++ b/src/iocore/cache/P_CacheInternal.h @@ -323,6 +323,7 @@ CacheVC::die() { if (vio.op == VIO::WRITE) { if (f.update && total_len) { + ink_assert(alternate.valid()); alternate.object_key_set(earliest_key); } if (!is_io_in_progress()) { diff --git a/src/proxy/http/HttpTunnel.cc b/src/proxy/http/HttpTunnel.cc index d177b10603c..815ab07c081 100644 --- a/src/proxy/http/HttpTunnel.cc +++ b/src/proxy/http/HttpTunnel.cc @@ -1109,9 +1109,17 @@ HttpTunnel::producer_run(HttpTunnelProducer *p) } if (c_write == 0) { - // Nothing to do, call back the cleanup handlers - c->write_vio = nullptr; - consumer_handler(VC_EVENT_WRITE_COMPLETE, c); + // Cache writes need a VIO even when the body is empty so that closing the + // cache VC commits the response metadata instead of aborting the write. + if (c->vc_type == HttpTunnelType_t::CACHE_WRITE) { + c->write_vio = c->vc->do_io_write(this, 0, c->buffer_reader); + if (c->write_vio == nullptr) { + consumer_handler(VC_EVENT_ERROR, c); + } + } else { + c->write_vio = nullptr; + consumer_handler(VC_EVENT_WRITE_COMPLETE, c); + } } else { // In the client half close case, all the data that will be sent // from the client is already in the buffer. Go ahead and set diff --git a/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml b/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml index 53c7d58b165..f7774728f7d 100644 --- a/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml +++ b/tests/gold_tests/cache/replay/negative-caching-300-second-timeout.replay.yaml @@ -40,6 +40,18 @@ meta: # transaction. delay: 100ms + - request_200_item: &request_200_item + client-request: + method: "GET" + version: "1.1" + scheme: "http" + url: /path/200_empty_chunked + headers: + fields: + - [ Host, example.com ] + + delay: 100ms + sessions: - transactions: @@ -47,13 +59,16 @@ sessions: <<: *request_404_item # Populate the cache with a 404 response. + # Verify that an empty chunked response is cached (issue #11313). server-response: status: 404 reason: "Not Found" headers: fields: - - [ Content-Length, 32 ] + - [ Transfer-Encoding, chunked ] - [ Cache-Control, max-age=300 ] + content: + size: 0 proxy-response: status: 404 @@ -77,3 +92,37 @@ sessions: # Expect the cached 404 response. proxy-response: status: 404 + + - all: { headers: { fields: [[ uuid, 23 ]]}} + <<: *request_200_item + + # The empty chunked-body behavior is not specific to negative responses. + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Transfer-Encoding, chunked ] + - [ Cache-Control, max-age=300 ] + content: + size: 0 + + proxy-response: + status: 200 + + - all: { headers: { fields: [[ uuid, 24 ]]}} + <<: *request_200_item + + proxy-request: + expect: absent + + server-response: + status: 502 + reason: Bad Gateway + headers: + fields: + - [ Content-Length, 0 ] + + # Expect the cached 200 response. + proxy-response: + status: 200 diff --git a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py index b073c45760b..fd52200e95b 100644 --- a/tests/gold_tests/pluginTest/stale_response/stale_response.test.py +++ b/tests/gold_tests/pluginTest/stale_response/stale_response.test.py @@ -126,6 +126,8 @@ def setupTS(self) -> None: "proxy.config.http.server_session_sharing.pool": "global", # Turn off negative revalidating so that we can test stale-if-error. "proxy.config.http.negative_revalidating_enabled": 0, + # Keep the active log filename available for the final content check if the test spans UTC midnight. + "proxy.config.log.rolling_enabled": 0, }) ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self._server.Variables.http_port}/ {remap_plugin_config}")