This is an automated email from the ASF dual-hosted git repository.
moonchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 95a13a552d ProxyProtocol: free pp_info heap on NetVConnection recycle
(#13293)
95a13a552d is described below
commit 95a13a552d02a5f4621a3c10127b7f16b4b47780
Author: Mo Chen <[email protected]>
AuthorDate: Thu Jun 18 16:13:16 2026 -0500
ProxyProtocol: free pp_info heap on NetVConnection recycle (#13293)
NetVConnection embeds a ProxyProtocol by value, and a PROXY v2 header parsed
by has_proxy_protocol() heap-allocates ProxyProtocol::additional_data (the
TLV
blob) plus the tlv map. The NetVConnection ClassAllocators are
Destruct_on_free=false, so ~ProxyProtocol never runs when a VC is recycled
and
UnixNetVConnection::clear() did not release pp_info -- the next
placement-new on
the recycled slot abandoned the buffer and map nodes, leaking once per
recycled
connection that carried a PROXY v2 header. Behind a PROXY-protocol load
balancer that is every inbound connection.
Add ProxyProtocol::reset() and call it from UnixNetVConnection::clear() (the
single recycle chokepoint for the Unix, SSL and QUIC VCs). reset() swaps the
members with empty containers rather than clear()ing them, because clear()
retains capacity that the recycle would still abandon.
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
---
include/iocore/net/ProxyProtocol.h | 19 ++++++++++
src/iocore/net/UnixNetVConnection.cc | 3 ++
src/iocore/net/unit_tests/test_ProxyProtocol.cc | 50 +++++++++++++++++++++++++
3 files changed, 72 insertions(+)
diff --git a/include/iocore/net/ProxyProtocol.h
b/include/iocore/net/ProxyProtocol.h
index d3a7c73671..7d7fc34e46 100644
--- a/include/iocore/net/ProxyProtocol.h
+++ b/include/iocore/net/ProxyProtocol.h
@@ -82,6 +82,25 @@ public:
other.tlv.clear();
}
~ProxyProtocol() = default;
+
+ /** Release owned heap and reset to the default-constructed state.
+ *
+ * Swap rather than clear() the containers: clear() retains capacity, which
would be abandoned
+ * (leaked) when the slot is reused, since the NetVConnection allocators are
Destruct_on_free=false
+ * and so never run ~ProxyProtocol.
+ */
+ void
+ reset()
+ {
+ std::string{}.swap(additional_data);
+ std::unordered_map<uint8_t, std::string_view>{}.swap(tlv);
+ version = ProxyProtocolVersion::UNDEFINED;
+ ip_family = AF_UNSPEC;
+ type = 0;
+ src_addr = {};
+ dst_addr = {};
+ }
+
int set_additional_data(std::string_view data);
void set_ipv4_addrs(in_addr_t src_addr, uint16_t src_port, in_addr_t
dst_addr, uint16_t dst_port);
void set_ipv6_addrs(const in6_addr &src_addr, uint16_t src_port, const
in6_addr &dst_addr, uint16_t dst_port);
diff --git a/src/iocore/net/UnixNetVConnection.cc
b/src/iocore/net/UnixNetVConnection.cc
index 29e1b4dead..8e18df53ed 100644
--- a/src/iocore/net/UnixNetVConnection.cc
+++ b/src/iocore/net/UnixNetVConnection.cc
@@ -1220,6 +1220,9 @@ UnixNetVConnection::clear()
read.vio.vc_server = nullptr;
write.vio.vc_server = nullptr;
options.reset();
+ // The VC allocator is Destruct_on_free=false (~ProxyProtocol never runs on
recycle), so release
+ // pp_info's heap explicitly here.
+ pp_info.reset();
if (netvc_context == NET_VCONNECTION_OUT) {
read.vio.buffer.clear();
write.vio.buffer.clear();
diff --git a/src/iocore/net/unit_tests/test_ProxyProtocol.cc
b/src/iocore/net/unit_tests/test_ProxyProtocol.cc
index ada8605f08..cc62e63e82 100644
--- a/src/iocore/net/unit_tests/test_ProxyProtocol.cc
+++ b/src/iocore/net/unit_tests/test_ProxyProtocol.cc
@@ -956,3 +956,53 @@ TEST_CASE("ProxyProtocol Rule of 5", "[ProxyProtocol]")
CHECK(orig_tlv.value() == "original");
}
}
+
+TEST_CASE("ProxyProtocol reset", "[ProxyProtocol]")
+{
+ // A PROXY v2 / TCP over IPv4 header carrying TLVs, so the parsed pp_info
owns both the
+ // additional_data string and the tlv map that reset() must release.
+ uint8_t raw_data[] = {
+ 0x0D, 0x0A, 0x0D, 0x0A, 0x00, 0x0D, 0x0A, 0x51, ///< preface
+ 0x55, 0x49, 0x54, 0x0A, ///<
+ 0x21, ///< version & command
+ 0x11, ///< protocol & family
+ 0x00, 0x32, ///< len
+ 0xC0, 0x00, 0x02, 0x01, ///< src_addr
+ 0xC6, 0x33, 0x64, 0x01, ///< dst_addr
+ 0xC3, 0x50, ///< src_port
+ 0x01, 0xBB, ///< dst_port
+ 0x01, 0x00, 0x02, 0x68, 0x32, /// PP2_TYPE_ALPN (h2)
+ 0x02, 0x00, 0x03, 0x61, 0x62, 0x63, /// PP2_TYPE_AUTHORITY
(abc)
+ 0x20, 0x00, 0x18, 0x01, 0x00, 0x00, 0x00, 0x00, /// PP2_TYPE_SSL
(client=0x01, verify=0)
+ 0x23, 0x00, 0x03, 0x58, 0x59, 0x5A, /// PP2_SUBTYPE_SSL_CIPHER
(XYZ)
+ 0x26, 0x00, 0x04, 0x58, 0x31, 0x32, 0x33, /// PP2_SUBTYPE_SSL_GROUP
(X123)
+ 0x21, 0x00, 0x03, 0x54, 0x4C, 0x53, ///
PP2_SUBTYPE_SSL_VERSION (TLS)
+ };
+
+ swoc::TextView tv(reinterpret_cast<char *>(raw_data), sizeof(raw_data));
+
+ ProxyProtocol pp_info;
+ REQUIRE(proxy_protocol_parse(&pp_info, tv) == tv.size());
+
+ // Precondition: pp_info is fully populated, including its heap-owning
members.
+ REQUIRE(pp_info.version == ProxyProtocolVersion::V2);
+ REQUIRE(pp_info.ip_family == AF_INET);
+ REQUIRE(pp_info.type == SOCK_STREAM);
+ REQUIRE_FALSE(pp_info.tlv.empty());
+ REQUIRE(pp_info.get_tlv(PP2_TYPE_ALPN).has_value());
+
+ pp_info.reset();
+
+ // Every field is back to the default-constructed state and the backing
storage is released.
+ CHECK(pp_info.version == ProxyProtocolVersion::UNDEFINED);
+ CHECK(pp_info.ip_family == AF_UNSPEC);
+ CHECK(pp_info.type == 0);
+ CHECK_FALSE(ats_is_ip(&pp_info.src_addr));
+ CHECK_FALSE(ats_is_ip(&pp_info.dst_addr));
+ CHECK(pp_info.tlv.empty());
+ CHECK_FALSE(pp_info.get_tlv(PP2_TYPE_ALPN).has_value());
+
+ // reset() is idempotent and safe to call on an already-empty object.
+ pp_info.reset();
+ CHECK(pp_info.tlv.empty());
+}
