This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit bfd176c7421c09c7ced12cb354120006e98a52b0 Author: Masaori Koshiba <masa...@apache.org> AuthorDate: Wed Sep 17 15:04:12 2025 +0900 Fix background fill on TLS connections (#12502) * Fix background fill on TLS Co-authored-by: Masakazu Kitajo <mas...@apache.org> * Adjust http2_rst_stream AuTest * TLS Blind Tunnel Support * Adjust background_fill AuTests * Fix docs * Fix AuTest --------- Co-authored-by: Masakazu Kitajo <mas...@apache.org> (cherry picked from commit c56a55ac1b9313713fba3992a8cc04fafd459762) (cherry picked from commit 906da289cfed48b4600370e00cf6f5b5c9aff7d4) --- src/iocore/net/P_SSLNetVConnection.h | 1 + src/iocore/net/SSLNetVConnection.cc | 47 ++++++++++++++++++++++ src/iocore/net/UnixNetVConnection.cc | 5 +++ src/proxy/http/HttpSM.cc | 15 +++---- tests/gold_tests/cache/background_fill.test.py | 25 ++++++------ .../cache/gold/background_fill_0_stderr_H.gold | 6 +-- .../cache/gold/background_fill_0_stderr_W.gold | 15 ------- .../cache/gold/background_fill_1_stderr_H.gold | 6 +-- .../cache/gold/background_fill_1_stderr_W.gold | 15 ------- .../cache/gold/background_fill_2_stderr_H.gold | 6 +-- .../cache/gold/background_fill_2_stderr_W.gold | 15 ------- .../cache/gold/background_fill_3_stdout.gold | 4 +- tests/gold_tests/cache/replay/bg_fill.yaml | 2 + tests/gold_tests/h2/http2_rst_stream.test.py | 2 +- 14 files changed, 81 insertions(+), 83 deletions(-) diff --git a/src/iocore/net/P_SSLNetVConnection.h b/src/iocore/net/P_SSLNetVConnection.h index 8edc5c0e4e..105c023257 100644 --- a/src/iocore/net/P_SSLNetVConnection.h +++ b/src/iocore/net/P_SSLNetVConnection.h @@ -139,6 +139,7 @@ public: void net_read_io(NetHandler *nh) override; int64_t load_buffer_and_write(int64_t towrite, MIOBufferAccessor &buf, int64_t &total_written, int &needs) override; void do_io_close(int lerrno = -1) override; + void do_io_shutdown(ShutdownHowTo_t howto) override; //////////////////////////////////////////////////////////// // Instances of NetVConnection should be allocated // diff --git a/src/iocore/net/SSLNetVConnection.cc b/src/iocore/net/SSLNetVConnection.cc index 9244c2e8c2..bc9c1cce66 100644 --- a/src/iocore/net/SSLNetVConnection.cc +++ b/src/iocore/net/SSLNetVConnection.cc @@ -23,6 +23,7 @@ #include "BIO_fastopen.h" #include "P_UnixNet.h" +#include "P_UnixNetVConnection.h" #include "SSLStats.h" #include "P_Net.h" #include "P_SSLUtils.h" @@ -36,6 +37,7 @@ #include "iocore/net/ProxyProtocol.h" #include "iocore/net/SSLDiags.h" #include "iocore/net/SSLSNIConfig.h" +#include "iocore/net/SSLTypes.h" #include "iocore/net/TLSALPNSupport.h" #include "tscore/ink_config.h" #include "tscore/Layout.h" @@ -897,6 +899,51 @@ SSLNetVConnection::do_io_close(int lerrno) super::do_io_close(lerrno); } +void +SSLNetVConnection::do_io_shutdown(ShutdownHowTo_t howto) +{ + if (get_tunnel_type() == SNIRoutingType::BLIND) { + // we don't have TLS layer control of blind tunnel + UnixNetVConnection::do_io_shutdown(howto); + return; + } + + switch (howto) { + case IO_SHUTDOWN_READ: + // No need to call SSL API + // SSL_shutdown() sends the close_notify alert to the peer and it only closes the write direction. + // The read direction will be closed by the peer. + read.enabled = 0; + read.vio.buffer.clear(); + read.vio.nbytes = 0; + read.vio.cont = nullptr; + f.shutdown |= NetEvent::SHUTDOWN_READ; + break; + case IO_SHUTDOWN_WRITE: + SSL_shutdown(ssl); + write.enabled = 0; + write.vio.buffer.clear(); + write.vio.nbytes = 0; + write.vio.cont = nullptr; + f.shutdown |= NetEvent::SHUTDOWN_WRITE; + break; + case IO_SHUTDOWN_READWRITE: + SSL_shutdown(ssl); + read.enabled = 0; + write.enabled = 0; + read.vio.buffer.clear(); + read.vio.nbytes = 0; + write.vio.buffer.clear(); + write.vio.nbytes = 0; + read.vio.cont = nullptr; + write.vio.cont = nullptr; + f.shutdown = NetEvent::SHUTDOWN_READ | NetEvent::SHUTDOWN_WRITE; + break; + default: + ink_assert(!"not reached"); + } +} + void SSLNetVConnection::clear() { diff --git a/src/iocore/net/UnixNetVConnection.cc b/src/iocore/net/UnixNetVConnection.cc index 7e29f1e6fb..2d2be1e033 100644 --- a/src/iocore/net/UnixNetVConnection.cc +++ b/src/iocore/net/UnixNetVConnection.cc @@ -639,6 +639,11 @@ UnixNetVConnection::net_write_io(NetHandler *nh) NetState *s = &this->write; Continuation *c = this->write.vio.cont; + if (c == nullptr) { + // If Continuation to callback is nullptr, we can do nothing + return; + } + MUTEX_TRY_LOCK(lock, s->vio.mutex, thread); if (!lock.is_locked() || lock.get_mutex() != s->vio.mutex.get()) { diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 5d4e5fa49a..a85928dd22 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -868,20 +868,15 @@ HttpSM::state_watch_for_client_abort(int event, void *data) * client. */ case VC_EVENT_EOS: { - // We got an early EOS. If the tunnal has cache writer, don't kill it for background fill. + // We got an early EOS. To trigger background fill, do NOT kill HttpSM. if (!terminate_sm) { // Not done already NetVConnection *netvc = _ua.get_txn()->get_netvc(); - if (_ua.get_txn()->allow_half_open() || tunnel.has_consumer_besides_client()) { - if (netvc) { + if (netvc) { + if (_ua.get_txn()->allow_half_open()) { netvc->do_io_shutdown(IO_SHUTDOWN_READ); + } else { + netvc->do_io_shutdown(IO_SHUTDOWN_READWRITE); } - } else { - _ua.get_txn()->do_io_close(); - vc_table.cleanup_entry(_ua.get_entry()); - _ua.set_entry(nullptr); - tunnel.kill_tunnel(); - terminate_sm = true; // Just die already, the requester is gone - set_ua_abort(HttpTransact::ABORTED, event); } if (_ua.get_entry()) { _ua.get_entry()->eos = true; diff --git a/tests/gold_tests/cache/background_fill.test.py b/tests/gold_tests/cache/background_fill.test.py index 4c4fe2467d..abf09fbb80 100644 --- a/tests/gold_tests/cache/background_fill.test.py +++ b/tests/gold_tests/cache/background_fill.test.py @@ -72,6 +72,8 @@ class BackgroundFillTest: "proxy.config.diags.debug.tags": "http", }) + self.ts[name].Disk.plugin_config.AddLine('xdebug.so --enable=x-cache') + if name == 'for_httpbin' or name == 'default': self.ts[name].Disk.remap_config.AddLines([ f"map / http://127.0.0.1:{self.httpbin.Variables.Port}", @@ -110,14 +112,13 @@ class BackgroundFillTest: tr.MakeCurlCommandMulti( f""" {{curl}} -X PURGE --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4; -timeout 2 {{curl}} --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4; -sleep 4; -{{curl}} --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4 +timeout 1 {{curl}} --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4; +sleep 5; +{{curl}} --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4 -H "x-debug: x-cache" """, ts=self.ts['for_httpbin']) tr.Processes.Default.ReturnCode = 0 - tr.Processes.Default.Streams.stderr = Testers.Any( - "gold/background_fill_0_stderr_H.gold", "gold/background_fill_0_stderr_W.gold") + tr.Processes.Default.Streams.stderr = "gold/background_fill_0_stderr_H.gold" self.__checkProcessAfter(tr) def __testCase1(self): @@ -129,14 +130,13 @@ sleep 4; tr.MakeCurlCommandMulti( f""" {{curl}} -X PURGE --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; -timeout 3 {{curl}} --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; +timeout 1 {{curl}} --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; sleep 5; -{{curl}} --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4 +{{curl}} --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4 -H "x-debug: x-cache" """, ts=self.ts['for_httpbin']) tr.Processes.Default.ReturnCode = 0 - tr.Processes.Default.Streams.stderr = Testers.Any( - "gold/background_fill_1_stderr_H.gold", "gold/background_fill_1_stderr_W.gold") + tr.Processes.Default.Streams.stderr = "gold/background_fill_1_stderr_H.gold" self.__checkProcessAfter(tr) def __testCase2(self): @@ -148,14 +148,13 @@ sleep 5; tr.MakeCurlCommandMulti( f""" {{curl}} -X PURGE --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; -timeout 3 {{curl}} --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; +timeout 1 {{curl}} --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; sleep 5; -{{curl}} --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4 +{{curl}} --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4 -H "x-debug: x-cache" """, ts=self.ts['for_httpbin']) tr.Processes.Default.ReturnCode = 0 - tr.Processes.Default.Streams.stderr = Testers.Any( - "gold/background_fill_2_stderr_H.gold", "gold/background_fill_2_stderr_W.gold") + tr.Processes.Default.Streams.stderr = "gold/background_fill_2_stderr_H.gold" self.__checkProcessAfter(tr) def __testCase3(self): diff --git a/tests/gold_tests/cache/gold/background_fill_0_stderr_H.gold b/tests/gold_tests/cache/gold/background_fill_0_stderr_H.gold index 6c9b90e886..3a71e8ea16 100644 --- a/tests/gold_tests/cache/gold/background_fill_0_stderr_H.gold +++ b/tests/gold_tests/cache/gold/background_fill_0_stderr_H.gold @@ -7,11 +7,9 @@ `` > GET /drip?duration=4 HTTP/1.1 `` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cMsSfW]) -`` > GET /drip?duration=4 HTTP/1.1 `` -< HTTP/1.1 `` +< HTTP/1.1 200 OK `` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cHs f ]) +< X-Cache: hit-fresh `` diff --git a/tests/gold_tests/cache/gold/background_fill_0_stderr_W.gold b/tests/gold_tests/cache/gold/background_fill_0_stderr_W.gold deleted file mode 100644 index 383c9c7364..0000000000 --- a/tests/gold_tests/cache/gold/background_fill_0_stderr_W.gold +++ /dev/null @@ -1,15 +0,0 @@ -`` -> PURGE /drip?duration=4 HTTP/1.1 -`` -< HTTP/1.1 `` -`` -< Via: http/1.1 traffic_server (ApacheTrafficServer/``) -`` -> GET /drip?duration=4 HTTP/1.1 -`` -> GET /drip?duration=4 HTTP/1.1 -`` -< HTTP/1.1 `` -`` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cWs f ]) -`` diff --git a/tests/gold_tests/cache/gold/background_fill_1_stderr_H.gold b/tests/gold_tests/cache/gold/background_fill_1_stderr_H.gold index 6c9b90e886..3a71e8ea16 100644 --- a/tests/gold_tests/cache/gold/background_fill_1_stderr_H.gold +++ b/tests/gold_tests/cache/gold/background_fill_1_stderr_H.gold @@ -7,11 +7,9 @@ `` > GET /drip?duration=4 HTTP/1.1 `` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cMsSfW]) -`` > GET /drip?duration=4 HTTP/1.1 `` -< HTTP/1.1 `` +< HTTP/1.1 200 OK `` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cHs f ]) +< X-Cache: hit-fresh `` diff --git a/tests/gold_tests/cache/gold/background_fill_1_stderr_W.gold b/tests/gold_tests/cache/gold/background_fill_1_stderr_W.gold deleted file mode 100644 index 383c9c7364..0000000000 --- a/tests/gold_tests/cache/gold/background_fill_1_stderr_W.gold +++ /dev/null @@ -1,15 +0,0 @@ -`` -> PURGE /drip?duration=4 HTTP/1.1 -`` -< HTTP/1.1 `` -`` -< Via: http/1.1 traffic_server (ApacheTrafficServer/``) -`` -> GET /drip?duration=4 HTTP/1.1 -`` -> GET /drip?duration=4 HTTP/1.1 -`` -< HTTP/1.1 `` -`` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cWs f ]) -`` diff --git a/tests/gold_tests/cache/gold/background_fill_2_stderr_H.gold b/tests/gold_tests/cache/gold/background_fill_2_stderr_H.gold index ed4459477b..afa508e71c 100644 --- a/tests/gold_tests/cache/gold/background_fill_2_stderr_H.gold +++ b/tests/gold_tests/cache/gold/background_fill_2_stderr_H.gold @@ -7,11 +7,9 @@ `` > GET /drip?duration=4 HTTP/2 `` -< via: http/1.1 traffic_server (ApacheTrafficServer/`` [cMsSfW]) -`` > GET /drip?duration=4 HTTP/2 `` -< HTTP/2 `` +< HTTP/2 200`` `` -< via: http/1.1 traffic_server (ApacheTrafficServer/`` [cHs f ]) +< x-cache: hit-fresh `` diff --git a/tests/gold_tests/cache/gold/background_fill_2_stderr_W.gold b/tests/gold_tests/cache/gold/background_fill_2_stderr_W.gold deleted file mode 100644 index bff1dbb288..0000000000 --- a/tests/gold_tests/cache/gold/background_fill_2_stderr_W.gold +++ /dev/null @@ -1,15 +0,0 @@ -`` -> PURGE /drip?duration=4 HTTP/2 -`` -< HTTP/2 `` -`` -< via: http/1.1 traffic_server (ApacheTrafficServer/``) -`` -> GET /drip?duration=4 HTTP/2 -`` -> GET /drip?duration=4 HTTP/2 -`` -< HTTP/2 `` -`` -< via: http/1.1 traffic_server (ApacheTrafficServer/`` [cWs f ]) -`` diff --git a/tests/gold_tests/cache/gold/background_fill_3_stdout.gold b/tests/gold_tests/cache/gold/background_fill_3_stdout.gold index b0c5ed4be6..a492c2f88f 100644 --- a/tests/gold_tests/cache/gold/background_fill_3_stdout.gold +++ b/tests/gold_tests/cache/gold/background_fill_3_stdout.gold @@ -4,12 +4,12 @@ content-type: text/html content-length: 11 `` -via: https/2 traffic_server (ApacheTrafficServer/`` [cMsSfW]) +x-cache: miss `` [DEBUG]: Received an HTTP/2 response for key 2 with stream id 1: :status: 200 content-type: text/html content-length: 11 `` -via: http/1.1 traffic_server (ApacheTrafficServer/`` [cHs f ]) +x-cache: hit-fresh `` diff --git a/tests/gold_tests/cache/replay/bg_fill.yaml b/tests/gold_tests/cache/replay/bg_fill.yaml index 6a76f8a2be..f01a4f47a4 100644 --- a/tests/gold_tests/cache/replay/bg_fill.yaml +++ b/tests/gold_tests/cache/replay/bg_fill.yaml @@ -44,6 +44,7 @@ sessions: - [:path, /a/path] - [Content-Type, text/html] - [uuid, 1] + - [x-debug, x-cache] - RST_STREAM: delay: 1s error-code: INTERNAL_ERROR @@ -84,6 +85,7 @@ sessions: - [:path, /a/path] - [Content-Type, text/html] - [uuid, 2] + - [x-debug, x-cache] server-response: frames: diff --git a/tests/gold_tests/h2/http2_rst_stream.test.py b/tests/gold_tests/h2/http2_rst_stream.test.py index caf2a71141..1b5c6bb6cd 100644 --- a/tests/gold_tests/h2/http2_rst_stream.test.py +++ b/tests/gold_tests/h2/http2_rst_stream.test.py @@ -58,7 +58,7 @@ tr.Processes.Default.Streams.All += Testers.ContainsExpression( tr.Processes.Default.Streams.All += Testers.ContainsExpression( 'Submitted RST_STREAM frame for key 1 on stream 1.', 'Send RST_STREAM frame.') -server.Streams.All += Testers.ExcludesExpression('RST_STREAM', 'Server is not affected.') +server.Streams.All += Testers.ContainsExpression('RST_STREAM', 'Origin Server received RST_STREAM frame.') ts.Disk.traffic_out.Content += Testers.ContainsExpression('Received HEADERS frame', 'Received HEADERS frame.')