THRIFT-6060: Reconnect THttpClient after Connection: close

[1fanwang]

A C++ THttpClient can complete one RPC, then fail the next when an HTTP/1.1 server sends Connection: close. The client leaves the socket marked open and writes the next request to a connection the server already closed.

The client now records close from comma-separated or repeated Connection headers. It requires the exact field name, so headers such as Connection-Timeout cannot trigger a reconnect. Before the next non-empty request, it clears buffered HTTP state, closes the old socket, and reconnects. Empty flushes remain no-ops, including when a buffered wrapper closes.

Jira: https://issues.apache.org/jira/browse/THRIFT-6060

Testing

cmake -S . -B build -G Ninja -DBUILD_TESTING=ON
cmake --build build --target UnitTests
build/bin/UnitTests --run_test=OneWayHTTPTest --log_level=test_suite
build/bin/UnitTests

Raw logs

On upstream/master, with only the regression test applied:

error: in "OneWayHTTPTest/HTTP_ClientReconnectsAfterConnectionClose":
unexpected exception thrown by client.roundTripRPC()
check acceptedCount() == 2U has failed [1 != 2]
TSocket::write_partial() send(): Broken pipe
fatal error: TTransportException: write() send(): Broken pipe

On dfb55a2b8009537e55314d8a31f4130984ea4d9c, the exact-name regression fails:

check !client.closesAfterHeader("Connection-Timeout: close") has failed
*** 1 failure is detected in the test module "thrift"

On this branch:

Running 4 test cases...
*** No errors detected

Running 89 test cases...
*** No errors detected

• Jira ticket exists and the PR title uses the THRIFT-NNNN prefix.
• Every commit is signed.
• The change preserves the HTTP/1.1 keep-alive path and adds no breaking API change.
• This change includes code, so [skip ci] does not apply.

[Copilot]

Pull request overview

Fixes a C++ HTTP transport edge case where THttpClient could attempt to reuse a socket after an HTTP/1.1 server responded with Connection: close, causing subsequent RPCs to write to a connection the server had already closed.

Changes:

• Track Connection: close across repeated and comma-separated Connection headers and trigger a reconnect before sending the next non-empty request.
• Reset buffered HTTP parsing/transfer state on THttpClient::close() to avoid carrying stale HTTP state across reconnects.
• Add a regression test that simulates a server sending Connection: keep-alive, close (and repeated Connection headers) and validates the client reconnects cleanly.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
lib/cpp/test/OneWayHTTPTest.cpp	Adds a regression test server transport that advertises Connection: close and verifies the client reconnects across multiple RPCs.
lib/cpp/src/thrift/transport/THttpClient.h	Adds closeAfterResponse_ state to track server-directed connection closure.
lib/cpp/src/thrift/transport/THttpClient.cpp	Implements parsing of Connection headers and reconnect-on-next-request behavior, plus HTTP state reset on close.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

lib/cpp/src/thrift/transport/THttpClient.cpp:82

• parseHeader() still uses prefix matching for Transfer-Encoding and Content-Length (via boost::istarts_with). This can misinterpret custom headers like Content-Length-Mismatch: / Transfer-Encoding-Other: as the real hop-by-hop headers, similar to the Connection-Timeout problem this PR fixes for Connection. Consider parsing the header name up to the colon once (and trimming optional whitespace) and then doing exact case-insensitive comparisons for all supported header names.

  if (boost::istarts_with(header, "Transfer-Encoding")) {
    if (boost::iends_with(value, "chunked")) {
      chunked_ = true;
    }
  } else if (boost::istarts_with(header, "Content-Length")) {