From 84a1af1cc035ba5541ff9335ab12ba5e100b60b8 Mon Sep 17 00:00:00 2001 From: clydebarrow <2366188+clydebarrow@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:42:09 +1100 Subject: [PATCH] [tcp] Take the packet size from the receive buffer The link takes the buffer as a std::span and derives max_packet_size from its size, so a length beyond the buffer is refused and the connection is closed. Co-Authored-By: Claude Code --- .../tcp/packet_interface/tcp_packet_interface.cpp | 7 ++++--- .../tcp/packet_interface/tcp_packet_interface.h | 7 ++++--- tests/components/tcp/tcp_packet_interface.cpp | 15 +++++++++------ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/esphome/components/tcp/packet_interface/tcp_packet_interface.cpp b/esphome/components/tcp/packet_interface/tcp_packet_interface.cpp index 9fc0082145b..1536bd52d49 100644 --- a/esphome/components/tcp/packet_interface/tcp_packet_interface.cpp +++ b/esphome/components/tcp/packet_interface/tcp_packet_interface.cpp @@ -15,7 +15,8 @@ static constexpr size_t LENGTH_SIZE = 2; // The peer is gone when nothing has been received for this many keep-alive intervals static constexpr uint32_t KEEP_ALIVE_MISSES = 3; -TcpPacketInterface::TcpPacketInterface(uint8_t *rx_buffer) : rx_buffer_(rx_buffer) { +TcpPacketInterface::TcpPacketInterface(std::span rx_buffer) + : packet_interface::PacketInterface(rx_buffer.size()), rx_buffer_(rx_buffer) { this->capabilities_ = packet_interface::CAPABILITY_RELIABILITY | packet_interface::CAPABILITY_SESSION | packet_interface::CAPABILITY_ADDRESSING; } @@ -107,7 +108,7 @@ void TcpPacketInterface::read_() { if (this->rx_have_ < LENGTH_SIZE) { count = this->link_.read(this->rx_header_ + this->rx_have_, LENGTH_SIZE - this->rx_have_); } else { - count = this->link_.read(this->rx_buffer_ + this->rx_have_ - LENGTH_SIZE, + count = this->link_.read(this->rx_buffer_.data() + this->rx_have_ - LENGTH_SIZE, this->rx_length_ + LENGTH_SIZE - this->rx_have_); } // Nothing more for now; a dropped link is seen on the next loop @@ -133,7 +134,7 @@ void TcpPacketInterface::read_() { this->rx_have_ = 0; // A zero length packet is a keep-alive if (this->rx_length_ != 0) - this->on_receive_from_interface_(PacketBuffer(this->rx_buffer_, this->rx_length_), {}); + this->on_receive_from_interface_(PacketBuffer(this->rx_buffer_.data(), this->rx_length_), {}); } } } diff --git a/esphome/components/tcp/packet_interface/tcp_packet_interface.h b/esphome/components/tcp/packet_interface/tcp_packet_interface.h index 228495fc3e6..e83258cf870 100644 --- a/esphome/components/tcp/packet_interface/tcp_packet_interface.h +++ b/esphome/components/tcp/packet_interface/tcp_packet_interface.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "esphome/components/packet_interface/packet_interface.h" #include "esphome/components/socket/tcp_client_link.h" @@ -30,8 +31,8 @@ namespace esphome::tcp { */ class TcpPacketInterface : public packet_interface::PacketInterface { public: - /// @param rx_buffer Holds max_packet_size bytes of the packet being received; must outlive the link - explicit TcpPacketInterface(uint8_t *rx_buffer); + /// @param rx_buffer Holds the packet being received; its size is the largest packet, and it must outlive the link + explicit TcpPacketInterface(std::span rx_buffer); void setup() override; void loop() override; @@ -69,7 +70,7 @@ class TcpPacketInterface : public packet_interface::PacketInterface { #ifdef USE_BINARY_SENSOR binary_sensor::BinarySensor *connected_sensor_{nullptr}; #endif - uint8_t *const rx_buffer_; + const std::span rx_buffer_; uint32_t keep_alive_ms_{10000}; uint32_t last_rx_ms_{0}; uint32_t last_tx_ms_{0}; diff --git a/tests/components/tcp/tcp_packet_interface.cpp b/tests/components/tcp/tcp_packet_interface.cpp index 9eaef586414..b3ec17a172e 100644 --- a/tests/components/tcp/tcp_packet_interface.cpp +++ b/tests/components/tcp/tcp_packet_interface.cpp @@ -27,23 +27,26 @@ using packet_interface::PacketBuffer; using packet_interface::PacketMetaData; using Packet = std::vector; -class TestTcp : public TcpPacketInterface { +// The receive buffer is a base, so it is set up before the link's constructor sees it +struct TcpBufferStorage { + uint8_t rx_buffer_storage[64]{}; +}; + +class TestTcp : public TcpBufferStorage, public TcpPacketInterface { public: TestTcp() : TcpPacketInterface(this->rx_buffer_storage) { - this->set_max_packet_size(64); this->set_host("peer"); this->set_port(1); } socket::TcpClientLink &link() { return this->link_; } - - // The base class only keeps the pointer, so the storage can be a member set up after it - uint8_t rx_buffer_storage[64]{}; }; // A layer stacked on the link, to check that the session state reaches it class UpperLayer : public packet_interface::PacketInterface { public: - explicit UpperLayer(PacketInterface *lower) { this->stack_on_(lower, packet_interface::CAPABILITY_ENCRYPTION); } + explicit UpperLayer(PacketInterface *lower) : packet_interface::PacketInterface(lower->get_max_packet_size()) { + this->stack_on_(lower, packet_interface::CAPABILITY_ENCRYPTION); + } bool send_to_interface(const PacketBuffer &, const PacketMetaData &) override { return true; } };