You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bn...@apache.org on 2026/10/19 21:15:22 UTC
(trafficserver) 19/22: Reuse an origin connection only after its request body is written
This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 88262127bc1e31869abcd1b117b285286b10b8ea
Author: Bryan Call <bc...@apache.org>
AuthorDate: Thu Sep 10 15:29:59 2026 -0700
Reuse an origin connection only after its request body is written
Reuse was decided from whether a request body tunnel was still alive, which is
a different question from whether the body reached the origin in full: with
Expect: 100-continue no such tunnel is ever opened. Decide it instead by
comparing the length declared to the origin against the bytes actually written,
exempting 100 and 103, which leave the exchange in progress. Adds a gold test.
(cherry picked from commit bafce5fb41d3fc05e93c340d9284927097825420)
---
include/proxy/http/HttpSM.h | 13 ++
src/proxy/http/HttpSM.cc | 57 +++++-
src/proxy/http/HttpTransact.cc | 31 ++-
tests/gold_tests/post/early_response_client.py | 224 +++++++++++++++++++++
tests/gold_tests/post/early_response_origin.py | 203 +++++++++++++++++++
.../post/post-early-response-100-continue.test.py | 183 +++++++++++++++++
tests/serial_tests.txt | 4 +
7 files changed, 713 insertions(+), 2 deletions(-)
diff --git a/include/proxy/http/HttpSM.h b/include/proxy/http/HttpSM.h
index 804520c9b6..970c60af4b 100644
--- a/include/proxy/http/HttpSM.h
+++ b/include/proxy/http/HttpSM.h
@@ -554,6 +554,19 @@ public:
bool server_connection_is_ssl = false;
bool is_waiting_for_full_body = false;
bool is_buffering_request_body = false;
+ // Set once the client's request declared a body that ATS undertook to forward, and the
+ // request head has been written to the origin. Cleared once that body has reached the
+ // origin in full, measured against the length in server_request. While it is set the
+ // origin's parser is mid-message and is still waiting for body bytes, so the connection
+ // cannot carry another request.
+ //
+ // The set side uses the same has_request_body() predicate that governs tunnel setup, so
+ // the two cannot diverge. attach_server_session() appends its chunked Transfer-Encoding
+ // under that same predicate, so an outbound-only body cannot arise there. The one
+ // outbound body not covered is the plugin-supplied one behind api_server_request_body_set,
+ // and that needs no debt: it is written into the same buffer as the request head, so
+ // WRITE_COMPLETE already means it went out in full.
+ bool server_owed_request_body = false;
// hooks_set records whether there are any hooks relevant
// to this transaction. Used to avoid costly calls
// do_api_callout_internal()
diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc
index 39de93f947..d5027db789 100644
--- a/src/proxy/http/HttpSM.cc
+++ b/src/proxy/http/HttpSM.cc
@@ -2278,6 +2278,11 @@ HttpSM::state_send_server_request_header(int event, void *data)
if (!t_state.api_server_request_body_set && method != HTTP_WKSIDX_TRACE &&
_ua.get_txn()->has_request_body(t_state.hdr_info.request_content_length,
t_state.client_info.transfer_encoding == HttpTransact::TransferEncoding_t::CHUNKED)) {
+ // The origin has now been told to expect a body. Record that it is owed those
+ // bytes, whether or not a tunnel to deliver them is started below. When the
+ // client asked for a 100 Continue the body is withheld until the origin sends
+ // one, so there is no tunnel to infer this from.
+ server_owed_request_body = true;
if (post_transform_info.vc) {
setup_transform_to_server_transfer();
} else {
@@ -4292,11 +4297,61 @@ HttpSM::tunnel_handler_post_server(int event, HttpTunnelConsumer *c)
ink_assert(tunnel.is_tunnel_alive() == false);
break;
- case VC_EVENT_WRITE_COMPLETE:
+ case VC_EVENT_WRITE_COMPLETE: {
// Completed successfully
c->write_success = true;
server_entry->in_tunnel = false;
+ // A tunnel can complete having written fewer bytes than the request head declared, so
+ // the debt is cleared on the byte count rather than on the event alone. Only a body
+ // matching the framing ATS declared leaves the origin able to parse a following request.
+ //
+ // Both routes to a short write here are narrow. A half close makes HttpTunnel clamp the
+ // consumer write to whatever had already arrived, but the only trigger for that flag
+ // before the request body tunnel starts is a plugin calling TSHttpTxnCloseAfterResponse.
+ // A client FIN is not one: it aborts the tunnel in tunnel_handler_post_ua, so this
+ // consumer never completes. A truncated chunked body sets chunked_handler.truncation,
+ // which reaches this event only when producer_handler_chunked sees a completion with the
+ // body unfinished. Neither route is exercised by post-early-response-100-continue.test.py,
+ // so the check rests on the code path above rather than on a regression test; it is kept
+ // because clearing the debt on a short body is exactly what pools a connection the
+ // origin is still parsing.
+ //
+ // The length has to come from server_request rather than from the client's
+ // request_content_length. A request body transform rewrites the outbound
+ // Content-Length to the transformed size and leaves the client's value alone, so
+ // comparing against the client's would hold the debt open for every transformed body
+ // that came out smaller than it went in.
+ // A null producer should not be read as a complete body, because the debt must only be
+ // cleared on positive evidence that the origin got what it was promised.
+ bool const body_truncated = c->producer == nullptr || c->producer->chunked_handler.truncation;
+ bool body_complete = !body_truncated;
+ int64_t declared_bytes = HTTP_UNDEFINED_CL;
+
+ // Note the truncation flag above is only set by HttpTunnel::producer_handler_chunked(),
+ // so it covers dechunking and chunked passthrough. When ATS is the one adding chunking
+ // the framing is correct by construction, because generate_chunked_content() emits the
+ // terminating chunk even for a body cut short. A chunked request carries no declared
+ // length either way, so there is no byte count to check against.
+ if (t_state.hdr_info.server_request.presence(MIME_PRESENCE_CONTENT_LENGTH)) {
+ declared_bytes = t_state.hdr_info.server_request.get_content_length();
+ }
+
+ if (body_complete && declared_bytes != HTTP_UNDEFINED_CL) {
+ // Count from the consumer rather than from server_request_body_bytes. That member is
+ // first write wins and is never re-armed, so on a retry it still holds the previous
+ // attempt's total and would clear the debt for a second attempt that fell short.
+ // HttpTunnel::reset() zeroes the consumers, so this count is per attempt.
+ body_complete = c->bytes_written >= declared_bytes;
+ }
+
+ if (body_complete) {
+ server_owed_request_body = false;
+ } else if (server_owed_request_body) {
+ SMDbg(dbg_ctl_http_tproxy, "Post tunnel completed but origin is still owed body bytes, wrote %" PRId64 " of %" PRId64,
+ c->bytes_written, declared_bytes);
+ }
break;
+ }
default:
ink_release_assert(0);
}
diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc
index e81204a020..2dc61ac65d 100644
--- a/src/proxy/http/HttpTransact.cc
+++ b/src/proxy/http/HttpTransact.cc
@@ -6260,6 +6260,36 @@ HttpTransact::initialize_state_variables_from_response(State *s, HTTPHdr *incomi
s->current.server->keep_alive = HTTPKeepAlive::NO_KEEPALIVE;
}
+ // The keep_alive recomputed above reflects only what the origin said. It says nothing
+ // about whether ATS held up its own end of the exchange. If a request head declaring a
+ // body went to the origin and that body was never written in full, the origin's parser
+ // is still waiting for those bytes, and reusing the connection would hand the next
+ // request to the origin as the leftover body of this one.
+ //
+ // Only the origin side is decided here. The client connection is handled later by
+ // do_drain_request_body(), reached from the SERVER_READ action, which consumes an
+ // undelivered body when all of it is already buffered and drops client keep-alive when it
+ // cannot. Forcing the client side here would run before that and leave its
+ // keep-alive-preserving case unreachable.
+ //
+ // The one exception is the branch just below, which drops client keep-alive when the
+ // client 4-tuple is also the outbound one. There the two connections are the same
+ // socket, so closing the origin side necessarily takes the client side with it.
+ //
+ // Only a response that leaves the exchange in progress is exempt, because there the body
+ // may still be on its way. That is the ordinary Expect: 100-continue case. The exempt
+ // set has to match the one handle_forward_server_connection_open() routes to
+ // handle_100_continue_response(), namely 100 and 103 alone. Every other 1xx is dispatched
+ // as a final response and ends the transaction, so testing for 1xx here would leave the
+ // connection poolable while the origin is still mid-message.
+ HTTPStatus const status_code = incoming_response->status_get();
+ bool const exchange_in_progress = status_code == HTTPStatus::CONTINUE || status_code == HTTPStatus::EARLY_HINTS;
+
+ if (s->state_machine->server_owed_request_body && !exchange_in_progress) {
+ TxnDbg(dbg_ctl_http_hdrs, "Origin is owed request body bytes that will not be sent, not reusing the connection");
+ s->current.server->keep_alive = HTTPKeepAlive::NO_KEEPALIVE;
+ }
+
if (s->current.server->keep_alive == HTTPKeepAlive::KEEPALIVE) {
TxnDbg(dbg_ctl_http_hdrs, "Server is keep-alive.");
} else if (s->state_machine->get_ua_txn() && s->state_machine->get_ua_txn()->is_outbound_transparent() &&
@@ -6271,7 +6301,6 @@ HttpTransact::initialize_state_variables_from_response(State *s, HTTPHdr *incomi
s->state_machine->t_state.client_info.keep_alive = HTTPKeepAlive::NO_KEEPALIVE;
}
- HTTPStatus status_code = incoming_response->status_get();
if (is_response_body_precluded(status_code, s->method)) {
s->hdr_info.response_content_length = 0;
s->hdr_info.trust_response_cl = true;
diff --git a/tests/gold_tests/post/early_response_client.py b/tests/gold_tests/post/early_response_client.py
new file mode 100644
index 0000000000..1653673489
--- /dev/null
+++ b/tests/gold_tests/post/early_response_client.py
@@ -0,0 +1,224 @@
+#!/usr/bin/env python3
+'''Drive a POST followed by an ordinary request, to probe origin connection reuse.
+
+Raw sockets rather than curl so that a declared request body can be left unsent,
+and so no proxy environment variables are consulted.
+
+In the default mode the POST declares a body and sends none of it, which leaves
+the origin owed those bytes once it answers early. With --complete-body the POST
+instead waits for the 100 Continue and sends the whole body, which is what an
+ordinary Expect: 100-continue POST does and must leave connection reuse intact.
+'''
+# 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.
+
+import argparse
+import re
+import socket
+import time
+
+WITHHELD_BODY_LENGTH = 100000
+COMPLETED_BODY_LENGTH = 16
+
+
+def read_response(conn: socket.socket) -> bytes:
+ '''Read one response head.
+
+ :param conn: Connection to read from.
+ :return: The bytes read, empty on timeout or close.
+ '''
+ buf = b''
+ while b'\r\n\r\n' not in buf:
+ try:
+ chunk = conn.recv(4096)
+ except (OSError, socket.timeout):
+ break
+ if not chunk:
+ break
+ buf += chunk
+ return buf
+
+
+def status_of(response: bytes) -> str:
+ '''Extract the status line of a response.
+
+ :param response: The response bytes.
+ :return: The status line, or NO-RESPONSE when nothing arrived.
+ '''
+ if not response:
+ return 'NO-RESPONSE'
+ return response.split(b'\r\n', 1)[0].decode(errors='replace')
+
+
+def closes_connection(response: bytes) -> bool:
+ '''Report whether a response asks to close the client connection.
+
+ :param response: The response bytes.
+ :return: True when Connection: close is present.
+ '''
+ return re.search(rb'\r\nconnection:\s*close', response, re.IGNORECASE) is not None
+
+
+class HeadReader:
+ """Read response heads from a connection without losing pipelined bytes.
+
+ An origin may send several interim responses back to back, and ATS forwards them the
+ same way, so more than one head can arrive in a single read. Reading straight into a
+ fresh buffer each time would discard the extra heads and then block forever waiting for
+ a response that already arrived.
+ """
+
+ def __init__(self, conn: socket.socket) -> None:
+ """Wrap a connection.
+
+ :param conn: Connection to read from.
+ """
+ self._conn = conn
+ self._buffer = b''
+
+ def next_head(self) -> bytes:
+ """Return the next response head, reading more only when needed.
+
+ :return: The head including its terminating blank line, empty on timeout or close.
+ """
+ while b'\r\n\r\n' not in self._buffer:
+ try:
+ chunk = self._conn.recv(4096)
+ except (OSError, socket.timeout):
+ return b''
+ if not chunk:
+ return b''
+ self._buffer += chunk
+
+ head, _, rest = self._buffer.partition(b'\r\n\r\n')
+ self._buffer = rest
+ return head + b'\r\n\r\n'
+
+
+def read_final_response(reader: HeadReader, label: str) -> bytes:
+ """Read responses until a final one arrives, reporting each interim response.
+
+ An origin may send more than one interim response before the final one, for instance
+ 103 Early Hints followed by 100 Continue, and ATS forwards each. Stopping at the first
+ response would leave the final one unread and close the connection mid transaction.
+
+ :param reader: Head reader wrapping the connection.
+ :param label: Prefix used in the log lines.
+ :return: The final response bytes, empty if none arrived.
+ """
+ while True:
+ response = reader.next_head()
+ if not response:
+ return b''
+ status = status_of(response)
+ if re.match(r'HTTP/1\.[01] 1\d\d', status):
+ print(f'{label} INTERIM {status}', flush=True)
+ continue
+ return response
+
+
+def send_withheld_post(port: int, path: str, expect_100_continue: bool, read_timeout: float) -> None:
+ '''Send a POST that declares a body and sends none of it.
+
+ :param port: Proxy port to connect to.
+ :param path: Request target, which selects the origin's behavior.
+ :param expect_100_continue: Whether to ask for a 100 Continue.
+ :param read_timeout: Seconds to wait for a response. Short where the response is
+ expected to be withheld, so the scenario does not pay a full timeout for nothing.
+ '''
+ expect_header = b'Expect: 100-continue\r\n' if expect_100_continue else b''
+ conn = socket.create_connection(('127.0.0.1', port), timeout=read_timeout)
+ conn.sendall(
+ f'POST {path} HTTP/1.1\r\n'.encode() + expect_header + b'Host: example.com\r\n'
+ b'Content-Length: ' + str(WITHHELD_BODY_LENGTH).encode() + b'\r\n'
+ b'\r\n')
+ response = read_response(conn)
+ print(f'EARLY-POST STATUS {status_of(response)}', flush=True)
+ # The client declared a body it never finished sending, so this connection is framed
+ # ambiguously for a following request and ATS must not offer to keep it alive. Report
+ # it so the client half of the decision is observable and not merely assumed.
+ print(f'EARLY-POST CLIENT-CLOSE {closes_connection(response)}', flush=True)
+ conn.close()
+
+
+def send_completed_post(port: int, path: str) -> None:
+ '''Send an ordinary Expect: 100-continue POST and complete its body.
+
+ :param port: Proxy port to connect to.
+ :param path: Request target, which selects the origin's behavior.
+ '''
+ conn = socket.create_connection(('127.0.0.1', port), timeout=10)
+ conn.sendall(
+ f'POST {path} HTTP/1.1\r\n'.encode() + b'Expect: 100-continue\r\n'
+ b'Host: example.com\r\n'
+ b'Content-Length: ' + str(COMPLETED_BODY_LENGTH).encode() + b'\r\n'
+ b'\r\n')
+
+ # Wait for the go-ahead, which may be preceded by other interim responses.
+ reader = HeadReader(conn)
+ while True:
+ response = reader.next_head()
+ if not response:
+ print('COMPLETE-POST INTERIM NO-RESPONSE', flush=True)
+ conn.close()
+ return
+ status = status_of(response)
+ print(f'COMPLETE-POST INTERIM {status}', flush=True)
+ if re.match(r'HTTP/1\.[01] 100', status):
+ break
+
+ conn.sendall(b'x' * COMPLETED_BODY_LENGTH)
+ print(f'COMPLETE-POST STATUS {status_of(read_final_response(reader, "COMPLETE-POST"))}', flush=True)
+ conn.close()
+
+
+def main() -> int:
+ '''Drive the POST and the following probe.
+
+ :return: Process exit status.
+ '''
+ parser = argparse.ArgumentParser()
+ parser.add_argument('port', type=int, help='Proxy port to connect to.')
+ parser.add_argument(
+ '--expect-100-continue', action='store_true', help='Ask for a 100 Continue so the body tunnel is never opened.')
+ parser.add_argument('--complete-body', action='store_true', help='Complete the declared body instead of withholding it.')
+ parser.add_argument('--path', default='/early', help="Request target, which selects the origin's behavior.")
+ parser.add_argument('--read-timeout', type=float, default=10.0, help='Seconds to wait for the first response.')
+ args = parser.parse_args()
+
+ if args.complete_body:
+ send_completed_post(args.port, args.path)
+ else:
+ send_withheld_post(args.port, args.path, args.expect_100_continue, args.read_timeout)
+
+ # Give ATS time to finish the transaction and decide what to do with the
+ # origin connection.
+ time.sleep(2)
+
+ # A separate, ordinary request on its own client connection. It must reach the
+ # origin as a request.
+ probe = socket.create_connection(('127.0.0.1', args.port), timeout=10)
+ probe.sendall(b'GET /probe HTTP/1.1\r\n'
+ b'Host: example.com\r\n'
+ b'\r\n')
+ print(f'PROBE STATUS {status_of(read_response(probe))}', flush=True)
+ probe.close()
+
+ return 0
+
+
+if __name__ == '__main__':
+ raise SystemExit(main())
diff --git a/tests/gold_tests/post/early_response_origin.py b/tests/gold_tests/post/early_response_origin.py
new file mode 100644
index 0000000000..49516fd9d7
--- /dev/null
+++ b/tests/gold_tests/post/early_response_origin.py
@@ -0,0 +1,203 @@
+#!/usr/bin/env python3
+'''Origin that can answer a POST before reading its body, or honor 100-continue.
+
+Two behaviors are modelled. On /early the response goes out before the request
+body has been read, the connection is offered for reuse, and the body is drained
+afterwards rather than the connection being closed. That is what nginx does via
+ngx_http_discard_request_body. On /complete the origin instead sends a 100
+Continue, reads the whole declared body, and answers normally, which is how an
+ordinary Expect: 100-continue POST proceeds.
+
+Request lines are logged with a per connection ordinal, so that reuse of a
+connection can be asserted without depending on connection numbering.
+'''
+# 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.
+
+import re
+import socket
+import sys
+import threading
+
+EARLY_RESPONSE = (b'HTTP/1.1 420 Be Calm\r\n'
+ b'Content-Length: 0\r\n'
+ b'Connection: keep-alive\r\n'
+ b'\r\n')
+
+CONTINUE_RESPONSE = b'HTTP/1.1 100 Continue\r\n\r\n'
+
+# A 1xx that is neither 100 nor 103. ATS dispatches those as final responses, so an origin
+# that sends one before reading the body is owed that body for good.
+INTERIM_RESPONSE = (b'HTTP/1.1 199 Hold On\r\n'
+ b'Content-Length: 0\r\n'
+ b'Connection: keep-alive\r\n'
+ b'\r\n')
+
+REQUEST_LINE = re.compile(rb'[A-Z]+ \S+ HTTP/1\.[01]\r?\n')
+
+
+def read_head(conn: socket.socket) -> tuple[bytes, bool]:
+ '''Read up to the end of a request head.
+
+ :param conn: Connection to read from.
+ :return: The bytes read, and whether a complete head was seen.
+ '''
+ buf = b''
+ while b'\r\n\r\n' not in buf:
+ try:
+ chunk = conn.recv(4096)
+ except (OSError, socket.timeout):
+ return buf, False
+ if not chunk:
+ return buf, False
+ buf += chunk
+ return buf, True
+
+
+def content_length_of(head: bytes) -> int:
+ '''Extract the declared Content-Length from a request head.
+
+ :param head: The request head bytes.
+ :return: The declared length, or 0 when absent.
+ '''
+ match = re.search(rb'\r\ncontent-length:\s*(\d+)', head, re.IGNORECASE)
+ return int(match.group(1)) if match else 0
+
+
+def drain(conn: socket.socket, count: int, conn_id: int) -> None:
+ '''Discard count bytes of request body, reporting anything request shaped.
+
+ A request line found inside these bytes means bytes that were meant to be a
+ request on this connection were consumed as this POST's body instead, which is
+ the framing error this guards against. It is reported as soon as it is seen rather
+ than after the drain finishes, because on a broken build the drain cannot
+ complete and only ends on a timeout.
+
+ :param conn: Connection to drain.
+ :param count: Number of bytes to discard.
+ :param conn_id: Identifier used in the log lines.
+ '''
+ window = b''
+ remaining = count
+ while remaining > 0:
+ try:
+ chunk = conn.recv(min(remaining, 4096))
+ except (OSError, socket.timeout):
+ break
+ if not chunk:
+ break
+ remaining -= len(chunk)
+
+ window += chunk
+ found = REQUEST_LINE.search(window)
+ if found:
+ print(f'CONN {conn_id} CONSUMED-AS-BODY {found.group(0).decode(errors="replace").strip()}', flush=True)
+ window = b''
+ else:
+ # Keep enough of a tail that a request line straddling two reads is still found.
+ window = window[-1024:]
+
+ print(f'CONN {conn_id} DRAINED {count - max(remaining, 0)} of {count}', flush=True)
+
+
+def read_body(conn: socket.socket, count: int) -> int:
+ '''Read exactly count body bytes.
+
+ :param conn: Connection to read from.
+ :param count: Number of bytes expected.
+ :return: The number of bytes actually read.
+ '''
+ remaining = count
+ while remaining > 0:
+ try:
+ chunk = conn.recv(min(remaining, 4096))
+ except (OSError, socket.timeout):
+ break
+ if not chunk:
+ break
+ remaining -= len(chunk)
+ return count - remaining
+
+
+def serve(conn: socket.socket, conn_id: int) -> None:
+ '''Serve requests on one connection until the peer stops sending.
+
+ :param conn: The accepted connection.
+ :param conn_id: Identifier used in the log lines.
+ '''
+ conn.settimeout(10)
+ ordinal = 0
+ with conn:
+ while True:
+ head, complete = read_head(conn)
+ if not complete:
+ return
+
+ ordinal += 1
+ request_line = head.split(b'\r\n', 1)[0].decode(errors='replace')
+ target = head.split(b'\r\n', 1)[0]
+ declared = content_length_of(head)
+ print(f'CONN {conn_id} REQUEST {ordinal} {request_line}', flush=True)
+ if re.search(rb'\r\nexpect:\s*100-continue', head, re.IGNORECASE):
+ print(f'CONN {conn_id} SAW-EXPECT {ordinal}', flush=True)
+
+ if b' /early' in target:
+ # Answer before reading the body, then discard it.
+ conn.sendall(EARLY_RESPONSE)
+ drain(conn, declared, conn_id)
+ elif b' /interim' in target:
+ # A 1xx outside the interim set, sent before reading the body. ATS treats
+ # this as a final response, so the declared body is never coming.
+ conn.sendall(INTERIM_RESPONSE)
+ drain(conn, declared, conn_id)
+ elif b' /complete' in target:
+ # Behave the way an ordinary Expect: 100-continue POST expects.
+ conn.sendall(CONTINUE_RESPONSE)
+ received = read_body(conn, declared)
+ print(f'CONN {conn_id} BODY-COMPLETE {received} of {declared}', flush=True)
+ conn.sendall(b'HTTP/1.1 200 OK\r\n'
+ b'Content-Length: 0\r\n'
+ b'Connection: keep-alive\r\n'
+ b'\r\n')
+ else:
+ conn.sendall(b'HTTP/1.1 200 OK\r\n'
+ b'Content-Length: 0\r\n'
+ b'Connection: keep-alive\r\n'
+ b'\r\n')
+
+
+def main() -> int:
+ '''Accept connections until stopped.
+
+ :return: Process exit status.
+ '''
+ port = int(sys.argv[1])
+ listener = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+ listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+ listener.bind(('127.0.0.1', port))
+ listener.listen(16)
+ print(f'LISTENING {port}', flush=True)
+
+ conn_id = 0
+ while True:
+ conn, _ = listener.accept()
+ conn_id += 1
+ print(f'CONN {conn_id} ACCEPTED', flush=True)
+ threading.Thread(target=serve, args=(conn, conn_id), daemon=True).start()
+
+
+if __name__ == '__main__':
+ sys.exit(main())
diff --git a/tests/gold_tests/post/post-early-response-100-continue.test.py b/tests/gold_tests/post/post-early-response-100-continue.test.py
new file mode 100644
index 0000000000..24d20f81de
--- /dev/null
+++ b/tests/gold_tests/post/post-early-response-100-continue.test.py
@@ -0,0 +1,183 @@
+'''Verify an origin owed a request body is never handed the next request.
+'''
+# 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.
+
+import sys
+
+Test.Summary = '''
+An origin that answers a POST before reading its body is still owed that body. ATS must
+not return such a connection to the session pool, or the next request written to it is
+consumed by the origin as the leftover body instead of being served. An origin that is
+sent the whole body must still get its connection reused.
+'''
+
+Test.ContinueOnFail = True
+
+Test.Setup.CopyAs('early_response_origin.py', Test.RunDirectory)
+Test.Setup.CopyAs('early_response_client.py', Test.RunDirectory)
+
+
+def AddScenario(name: str, description: str, client_args: str, guard_expected: bool | None) -> tuple:
+ '''Run the early response probe against its own ATS and origin.
+
+ Each scenario gets a dedicated ATS and origin so that a failure is attributable to the
+ scenario rather than to a connection left over from an earlier run.
+
+ :param name: Short identifier used for the process and port names.
+ :param description: Test run description.
+ :param client_args: Extra arguments passed to the client script.
+ :param guard_expected: Whether the owed body guard should fire, or None not to assert.
+ :return: The test run and the origin process, for further assertions.
+ '''
+ port_name = f'{name}_origin_port'
+ Test.GetTcpPort(port_name)
+ origin_port = getattr(Test.Variables, port_name)
+
+ origin = Test.Processes.Process(
+ f'{name}-origin', f'{sys.executable} {Test.RunDirectory}/early_response_origin.py {origin_port}')
+ origin.Ready = When.PortOpen(origin_port)
+
+ ts = Test.MakeATSProcess(f'ts-{name}', select_ports=True, enable_cache=False)
+ ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{origin_port}/')
+ ts.Disk.records_config.update(
+ {
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http_hdrs',
+ # The scenario depends on ATS forwarding Expect to the origin rather than
+ # answering it locally. That is the default, but pin it so this keeps
+ # exercising the forwarded Expect path even if the default changes.
+ 'proxy.config.http.send_100_continue_response': 0,
+ # One event thread and a single shared pool, so that the origin connection from
+ # the first request is the one offered to the second. With the default
+ # per-thread pool neither the reuse asserted against below nor the reuse
+ # asserted for can be provoked reliably, and the test would pass on a broken
+ # build.
+ 'proxy.config.exec_thread.limit': 1,
+ 'proxy.config.exec_thread.autoconfig.enabled': 0,
+ 'proxy.config.http.server_session_sharing.pool': 'global',
+ })
+
+ tr = Test.AddTestRun(description)
+ tr.Processes.Default.Command = (
+ f'{sys.executable} {Test.RunDirectory}/early_response_client.py {ts.Variables.port} {client_args}')
+ tr.Processes.Default.StartBefore(origin)
+ tr.Processes.Default.StartBefore(ts)
+ tr.Processes.Default.ReturnCode = 0
+ tr.StillRunningAfter = ts
+ tr.StillRunningAfter = origin
+
+ # Assert directly on the decision the fix adds, so that a scenario proves the changed
+ # code ran rather than only that the outcome happened to be right.
+ guard_message = 'Origin is owed request body bytes'
+ if guard_expected is True:
+ ts.Disk.traffic_out.Content += Testers.ContainsExpression(
+ guard_message, 'The owed body guard should refuse to reuse these connections.')
+ elif guard_expected is False:
+ ts.Disk.traffic_out.Content += Testers.ExcludesExpression(
+ guard_message, 'The owed body guard must not fire when the origin got the whole body.')
+
+ return tr, origin
+
+
+def AddWithheldBodyScenario(
+ name: str,
+ description: str,
+ client_args: str,
+ guard_expected: bool | None,
+ post_target: str = '/early',
+ early_status: str | None = None) -> None:
+ '''Assert that a POST whose body is withheld never poisons the origin connection.
+
+ :param name: Short identifier used for the process and port names.
+ :param description: Test run description.
+ :param client_args: Extra arguments passed to the client script.
+ :param guard_expected: Whether the owed body guard should fire.
+ :param post_target: The path the POST is sent to, which selects the origin's behavior.
+ :param early_status: If given, the status line the client must receive for the POST.
+ '''
+ tr, origin = AddScenario(name, description, client_args, guard_expected)
+
+ # Positive assertions first, so that an origin that never ran cannot satisfy the
+ # negative one below by staying silent.
+ origin.Streams.stdout += Testers.ContainsExpression(
+ f'REQUEST 1 POST {post_target}', 'The origin must have received the early POST.')
+ # The direct inverse of the broken behavior, which drains part of the declared body.
+ origin.Streams.stdout += Testers.ContainsExpression(r'DRAINED 0 of 100000', 'Nothing may be drained as this POST body.')
+ # Framing independent: the origin must never see a request line inside what it drains
+ # as the earlier POST's body.
+ origin.Streams.stdout += Testers.ExcludesExpression(
+ r'CONSUMED-AS-BODY', 'No request may be consumed as the body of the earlier POST.')
+ # The client connection must not be offered for reuse either, because the client
+ # declared a body it never finished sending. That is decided by do_drain_request_body()
+ # rather than by the change here, so this locks in behavior this change must not
+ # disturb rather than proving anything about the new guard.
+ tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
+ r'EARLY-POST CLIENT-CLOSE True', 'The client connection must not be offered for reuse.')
+ if early_status is not None:
+ tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
+ rf'EARLY-POST STATUS {early_status}', 'The early origin response must be forwarded to the client.')
+ # Normal traffic is not collateral damage. This is not a regression oracle on its own,
+ # because ATS retries the probe on a fresh connection even on a broken build.
+ tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
+ r'PROBE STATUS HTTP/1\.1 200 OK', 'The following request must still be served.')
+
+
+# The regression case. Expect: 100-continue is forwarded to the origin and the body is
+# withheld until a 100 arrives, so no request body tunnel exists when the early response
+# comes back. The decision to close must not depend on a live tunnel.
+AddWithheldBodyScenario(
+ 'expect', 'POST with Expect 100-continue answered early, then a following request', '--expect-100-continue', True)
+
+# The same withheld body without the Expect header, where a request body tunnel does
+# exist. Keeping it on the same build means a failure in the scenario above is
+# attributable to the header rather than to the harness.
+# The origin's early response ends the tunnel, and is forwarded to the client at once
+# with the client connection closed. The origin connection was still owed the rest of the
+# body, so the guard must keep it out of the pool.
+AddWithheldBodyScenario(
+ 'plain', 'POST answered early with no Expect header, then a following request', '', True, early_status=r'HTTP/1\.1 420')
+
+# A 1xx that is neither 100 nor 103. ATS dispatches those as final responses, so the
+# declared body is never coming and the connection must not be pooled. Without this the
+# exemption could be widened back to the whole 1xx range and nothing would fail.
+AddWithheldBodyScenario(
+ 'interim',
+ 'POST answered early with a 199, then a following request',
+ '--expect-100-continue --path /interim',
+ True,
+ post_target='/interim')
+
+# The other direction, and the one a stuck flag would break: an ordinary
+# Expect: 100-continue POST whose body is completed leaves the origin owed nothing, so its
+# connection must still be pooled and reused. Without this, closing every origin
+# connection unconditionally would pass the scenarios above.
+reuse_tr, reuse_origin = AddScenario(
+ 'reuse', 'POST with Expect 100-continue whose body completes, then a following request', '--complete-body --path /complete',
+ False)
+reuse_origin.Streams.stdout += Testers.ContainsExpression(
+ r'SAW-EXPECT 1', 'ATS must forward Expect to the origin rather than answering it locally.')
+reuse_origin.Streams.stdout += Testers.ContainsExpression(
+ r'BODY-COMPLETE 16 of 16', 'The whole declared body must reach the origin.')
+reuse_origin.Streams.stdout += Testers.ContainsExpression(
+ r'REQUEST 2 GET /probe', 'The origin connection must be reused for the following request.')
+reuse_tr.Processes.Default.Streams.stdout += Testers.ContainsExpression(
+ r'COMPLETE-POST STATUS HTTP/1\.1 200 OK', 'The completed POST must be answered normally.')
+
+# The 103 half of the exemption is deliberately not covered here. ATS treats any forwarded
+# interim response as the go-ahead and opens the request body tunnel in
+# tunnel_handler_100_continue(), so an origin that sends 103 before its 100 never gets the
+# body and the scenario cannot distinguish the exemption from its absence.
diff --git a/tests/serial_tests.txt b/tests/serial_tests.txt
index 9b74d16dab..c33cdd1d89 100644
--- a/tests/serial_tests.txt
+++ b/tests/serial_tests.txt
@@ -18,3 +18,7 @@ pluginTest/abuse_shield/abuse_shield_pacing.test.py
# Starts 15 ATS instances and TLS origins for retry admission scenarios.
h2/h2_to_origin_safe_retry.test.py
+
+# Pins exec_thread.limit to 1 and asserts origin connection reuse from a single
+# shared pool; parallel load skews the pooling window this depends on.
+post/post-early-response-100-continue.test.py