From 13ec5f27e9df7691fb92202ef83808a95e04fa81 Mon Sep 17 00:00:00 2001 From: clydebarrow <2366188+clydebarrow@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:56:36 +1100 Subject: [PATCH] [packet_interface] Leave no room, instead of underflowing, on a small link As in the other layers, addressing ports and fragmentation get no room when max_packet_size is smaller than their header, and fragmentation fails its setup instead of dividing by zero. Co-Authored-By: Claude Opus 5.5 --- esphome/components/packet_interface/basic_addressing.cpp | 4 +++- .../components/packet_interface/basic_fragmentation.cpp | 6 ++++++ .../components/packet_interface/basic_fragmentation.h | 5 ++++- tests/components/packet_interface/basic_addressing.cpp | 8 ++++++++ .../components/packet_interface/basic_fragmentation.cpp | 9 +++++++++ 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/esphome/components/packet_interface/basic_addressing.cpp b/esphome/components/packet_interface/basic_addressing.cpp index 1b5e66196a5..d8a8ebf85cf 100644 --- a/esphome/components/packet_interface/basic_addressing.cpp +++ b/esphome/components/packet_interface/basic_addressing.cpp @@ -67,7 +67,9 @@ AddressingPort::AddressingPort(BasicAddressing *addressing, uint16_t destination } else { this->inherit_from_(addressing->get_link(), CAPABILITY_ADDRESSING); } - this->max_packet_size_ = addressing->get_link()->get_max_packet_size() - BasicAddressing::HEADER_SIZE; + size_t link_size = addressing->get_link()->get_max_packet_size(); + // A link with no room leaves the layers above none either, so they fail their setup + this->max_packet_size_ = link_size > BasicAddressing::HEADER_SIZE ? link_size - BasicAddressing::HEADER_SIZE : 0; addressing->add_port(this); } diff --git a/esphome/components/packet_interface/basic_fragmentation.cpp b/esphome/components/packet_interface/basic_fragmentation.cpp index b587d1e3140..2513afbfd74 100644 --- a/esphome/components/packet_interface/basic_fragmentation.cpp +++ b/esphome/components/packet_interface/basic_fragmentation.cpp @@ -19,10 +19,16 @@ BasicFragmentation::BasicFragmentation(PacketInterface *lower, size_t max_packet size_t BasicFragmentation::fragments_for_(size_t size) const { size_t fragment_size = this->fragment_size_(); + if (fragment_size == 0) + return SIZE_MAX; return std::max(1, (size + fragment_size - 1) / fragment_size); } void BasicFragmentation::setup() { + if (this->fragment_size_() == 0) { + this->mark_failed(LOG_STR("max_packet_size too small for the configured layers")); + return; + } if (this->fragments_for_(this->max_packet_size_) > MAX_FRAGMENTS) { this->mark_failed(LOG_STR("max_packet_size needs more than 128 fragments")); return; diff --git a/esphome/components/packet_interface/basic_fragmentation.h b/esphome/components/packet_interface/basic_fragmentation.h index 4cc3ee3c952..a3c0652eba7 100644 --- a/esphome/components/packet_interface/basic_fragmentation.h +++ b/esphome/components/packet_interface/basic_fragmentation.h @@ -36,7 +36,10 @@ class BasicFragmentation : public PacketInterface { protected: void receive_(const PacketBuffer &data, const PacketMetaData &meta_data); - size_t fragment_size_() const { return this->lower_->get_max_packet_size() - HEADER_SIZE; } + size_t fragment_size_() const { + size_t lower_size = this->lower_->get_max_packet_size(); + return lower_size > HEADER_SIZE ? lower_size - HEADER_SIZE : 0; + } size_t fragments_for_(size_t size) const; PacketInterface *lower_; diff --git a/tests/components/packet_interface/basic_addressing.cpp b/tests/components/packet_interface/basic_addressing.cpp index 252fc92e594..13726622907 100644 --- a/tests/components/packet_interface/basic_addressing.cpp +++ b/tests/components/packet_interface/basic_addressing.cpp @@ -80,6 +80,14 @@ TEST(BasicAddressingTest, HeaderAndCapability) { EXPECT_EQ(a.link.sent, (std::vector{{0x78, 0x56, 0x34, 0x12, 0xAA}, {0x42, 0x00, 0x34, 0x12, 0xBB}})); } +TEST(BasicAddressingTest, LinkTooSmallLeavesNoRoom) { + MockLink link; + link.set_max_packet_size(BasicAddressing::HEADER_SIZE); + BasicAddressing addressing(&link, 0x0001); + AddressingPort port(&addressing, 0x0002, true); + EXPECT_EQ(port.get_max_packet_size(), 0); +} + TEST(BasicAddressingTest, BroadcastReachesEveryoneButSender) { Device a(1), b(2), c(3); a.port().send({0x10}); diff --git a/tests/components/packet_interface/basic_fragmentation.cpp b/tests/components/packet_interface/basic_fragmentation.cpp index 88d833d8e4f..2feab7ac2f0 100644 --- a/tests/components/packet_interface/basic_fragmentation.cpp +++ b/tests/components/packet_interface/basic_fragmentation.cpp @@ -191,4 +191,13 @@ TEST_F(BasicFragmentationTest, OversizedSingleFragmentDropped) { EXPECT_EQ(received, std::vector{Packet({1, 2, 3, 4})}); } +TEST_F(BasicFragmentationTest, LinkTooSmallFailsSetup) { + this->link_a.set_max_packet_size(BasicFragmentation::HEADER_SIZE); + BasicFragmentation fragmentation(&this->link_a, 100); + fragmentation.setup(); + EXPECT_TRUE(fragmentation.is_failed()); + Packet packet{1}; + EXPECT_FALSE(fragmentation.send_to_interface(PacketBuffer(packet), {})); +} + } // namespace esphome::packet_interface::fragmentation_testing