From cf404c34d01bbad30954cdfe1fcac71476f89471 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 18 Dec 2025 22:13:27 -1000 Subject: [PATCH] [api] Use stack buffers for peername logging to reduce per-connection memory --- esphome/components/api/api_connection.cpp | 38 +++++++++++------ esphome/components/api/api_connection.h | 14 +++++-- esphome/components/api/api_frame_helper.cpp | 10 ++++- esphome/components/api/api_frame_helper.h | 1 + .../components/api/api_frame_helper_noise.cpp | 10 ++++- .../api/api_frame_helper_plaintext.cpp | 10 ++++- esphome/components/api/api_server.cpp | 14 ++++--- .../components/socket/bsd_sockets_impl.cpp | 34 +++++++++++---- .../components/socket/lwip_raw_tcp_impl.cpp | 28 ++++++++++--- .../components/socket/lwip_sockets_impl.cpp | 42 ++++++++++++++----- esphome/components/socket/socket.h | 13 ++++++ 11 files changed, 165 insertions(+), 49 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 85f4566f3c..4f93055100 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -128,8 +128,10 @@ void APIConnection::start() { this->fatal_error_with_log_(LOG_STR("Helper init failed"), err); return; } - this->client_info_.peername = helper_->getpeername(); - this->client_info_.name = this->client_info_.peername; + // Initialize client name with peername (IP address) until Hello message provides actual name + char peername[socket::PEERNAME_MAX_LEN]; + this->helper_->getpeername_to(peername); + this->client_info_.name = peername; } APIConnection::~APIConnection() { @@ -210,8 +212,7 @@ void APIConnection::loop() { // Disconnect if not responded within 2.5*keepalive if (now - this->last_traffic_ > KEEPALIVE_DISCONNECT_TIMEOUT) { on_fatal_error(); - ESP_LOGW(TAG, "%s (%s) is unresponsive; disconnecting", this->client_info_.name.c_str(), - this->client_info_.peername.c_str()); + this->log_client_(ESPHOME_LOG_LEVEL_WARN, LOG_STR("is unresponsive; disconnecting")); } } else if (now - this->last_traffic_ > KEEPALIVE_TIMEOUT_MS && !this->flags_.remove) { // Only send ping if we're not disconnecting @@ -261,7 +262,7 @@ bool APIConnection::send_disconnect_response(const DisconnectRequest &msg) { // remote initiated disconnect_client // don't close yet, we still need to send the disconnect response // close will happen on next loop - ESP_LOGD(TAG, "%s (%s) disconnected", this->client_info_.name.c_str(), this->client_info_.peername.c_str()); + this->log_client_(ESPHOME_LOG_LEVEL_DEBUG, LOG_STR("disconnected")); this->flags_.next_close = true; DisconnectResponse resp; return this->send_message(resp, DisconnectResponse::MESSAGE_TYPE); @@ -1395,9 +1396,10 @@ void APIConnection::complete_authentication_() { } this->flags_.connection_state = static_cast(ConnectionState::AUTHENTICATED); - ESP_LOGD(TAG, "%s (%s) connected", this->client_info_.name.c_str(), this->client_info_.peername.c_str()); + this->log_client_(ESPHOME_LOG_LEVEL_DEBUG, LOG_STR("connected")); #ifdef USE_API_CLIENT_CONNECTED_TRIGGER - this->parent_->get_client_connected_trigger()->trigger(this->client_info_.name, this->client_info_.peername); + // Trigger expects std::string, get fresh peername from socket + this->parent_->get_client_connected_trigger()->trigger(this->client_info_.name, this->helper_->getpeername()); #endif #ifdef USE_HOMEASSISTANT_TIME if (homeassistant::global_homeassistant_time != nullptr) { @@ -1413,11 +1415,12 @@ void APIConnection::complete_authentication_() { bool APIConnection::send_hello_response(const HelloRequest &msg) { this->client_info_.name.assign(reinterpret_cast(msg.client_info), msg.client_info_len); - this->client_info_.peername = this->helper_->getpeername(); this->client_api_version_major_ = msg.api_version_major; this->client_api_version_minor_ = msg.api_version_minor; + char peername[socket::PEERNAME_MAX_LEN]; + this->helper_->getpeername_to(peername); ESP_LOGV(TAG, "Hello from client: '%s' | %s | API Version %" PRIu32 ".%" PRIu32, this->client_info_.name.c_str(), - this->client_info_.peername.c_str(), this->client_api_version_major_, this->client_api_version_minor_); + peername, this->client_api_version_major_, this->client_api_version_minor_); HelloResponse resp; resp.api_version_major = 1; @@ -1724,12 +1727,12 @@ bool APIConnection::send_buffer(ProtoWriteBuffer buffer, uint8_t message_type) { #ifdef USE_API_PASSWORD void APIConnection::on_unauthenticated_access() { this->on_fatal_error(); - ESP_LOGD(TAG, "%s (%s) no authentication", this->client_info_.name.c_str(), this->client_info_.peername.c_str()); + this->log_client_(ESPHOME_LOG_LEVEL_DEBUG, LOG_STR("no authentication")); } #endif void APIConnection::on_no_setup_connection() { this->on_fatal_error(); - ESP_LOGD(TAG, "%s (%s) no connection setup", this->client_info_.name.c_str(), this->client_info_.peername.c_str()); + this->log_client_(ESPHOME_LOG_LEVEL_DEBUG, LOG_STR("no connection setup")); } void APIConnection::on_fatal_error() { this->helper_->close(); @@ -1977,9 +1980,18 @@ void APIConnection::process_state_subscriptions_() { } #endif // USE_API_HOMEASSISTANT_STATES +void APIConnection::log_client_(int level, const LogString *message) { + char peername[socket::PEERNAME_MAX_LEN]; + this->helper_->getpeername_to(peername); + esp_log_printf_(level, TAG, __LINE__, ESPHOME_LOG_FORMAT("%s (%s): %s"), this->client_info_.name.c_str(), peername, + LOG_STR_ARG(message)); +} + void APIConnection::log_warning_(const LogString *message, APIError err) { - ESP_LOGW(TAG, "%s (%s): %s %s errno=%d", this->client_info_.name.c_str(), this->client_info_.peername.c_str(), - LOG_STR_ARG(message), LOG_STR_ARG(api_error_to_logstr(err)), errno); + char peername[socket::PEERNAME_MAX_LEN]; + this->helper_->getpeername_to(peername); + ESP_LOGW(TAG, "%s (%s): %s %s errno=%d", this->client_info_.name.c_str(), peername, LOG_STR_ARG(message), + LOG_STR_ARG(api_error_to_logstr(err)), errno); } } // namespace esphome::api diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index b50be5d0d4..1f0450917e 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -17,8 +17,9 @@ namespace esphome::api { // Client information structure struct ClientInfo { - std::string name; // Client name from Hello message - std::string peername; // IP:port from socket + std::string name; // Client name from Hello message + // Note: peername (IP address) is not stored here to save memory. + // Use helper_->getpeername_to() or helper_->getpeername() when needed. }; // Keepalive timeout in milliseconds @@ -281,7 +282,12 @@ class APIConnection final : public APIServerConnection { bool send_buffer(ProtoWriteBuffer buffer, uint8_t message_type) override; const std::string &get_name() const { return this->client_info_.name; } - const std::string &get_peername() const { return this->client_info_.peername; } + /// Get peer name (IP address) into a stack buffer - avoids heap allocation + size_t get_peername_to(std::span buf) const { + return this->helper_->getpeername_to(buf); + } + /// Get peer name as std::string - use sparingly, allocates on heap + std::string get_peername() const { return this->helper_->getpeername(); } protected: // Helper function to handle authentication completion @@ -726,6 +732,8 @@ class APIConnection final : public APIServerConnection { return this->schedule_batch_(); } + // Helper function to log client messages with name and peername + void log_client_(int level, const LogString *message); // Helper function to log API errors with errno void log_warning_(const LogString *message, APIError err); // Helper to handle fatal errors with logging diff --git a/esphome/components/api/api_frame_helper.cpp b/esphome/components/api/api_frame_helper.cpp index 20f8fcaf61..d4801fb63a 100644 --- a/esphome/components/api/api_frame_helper.cpp +++ b/esphome/components/api/api_frame_helper.cpp @@ -13,8 +13,16 @@ namespace esphome::api { static const char *const TAG = "api.frame_helper"; +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERY_VERBOSE #define HELPER_LOG(msg, ...) \ - ESP_LOGVV(TAG, "%s (%s): " msg, this->client_info_->name.c_str(), this->client_info_->peername.c_str(), ##__VA_ARGS__) + do { \ + char peername__[socket::PEERNAME_MAX_LEN]; \ + this->socket_->getpeername_to(peername__); \ + ESP_LOGVV(TAG, "%s (%s): " msg, this->client_info_->name.c_str(), peername__, ##__VA_ARGS__); \ + } while (0) +#else +#define HELPER_LOG(msg, ...) ((void) 0) +#endif #ifdef HELPER_LOG_PACKETS #define LOG_PACKET_RECEIVED(buffer) ESP_LOGVV(TAG, "Received frame: %s", format_hex_pretty(buffer).c_str()) diff --git a/esphome/components/api/api_frame_helper.h b/esphome/components/api/api_frame_helper.h index b582bcea9a..85a74e32cf 100644 --- a/esphome/components/api/api_frame_helper.h +++ b/esphome/components/api/api_frame_helper.h @@ -91,6 +91,7 @@ class APIFrameHelper { bool can_write_without_blocking() { return this->state_ == State::DATA && this->tx_buf_count_ == 0; } std::string getpeername() { return socket_->getpeername(); } int getpeername(struct sockaddr *addr, socklen_t *addrlen) { return socket_->getpeername(addr, addrlen); } + size_t getpeername_to(std::span buf) { return socket_->getpeername_to(buf); } APIError close() { state_ = State::CLOSED; int err = this->socket_->close(); diff --git a/esphome/components/api/api_frame_helper_noise.cpp b/esphome/components/api/api_frame_helper_noise.cpp index 1d6f32ee9d..698b0d2128 100644 --- a/esphome/components/api/api_frame_helper_noise.cpp +++ b/esphome/components/api/api_frame_helper_noise.cpp @@ -24,8 +24,16 @@ static const char *const PROLOGUE_INIT = "NoiseAPIInit"; #endif static constexpr size_t PROLOGUE_INIT_LEN = 12; // strlen("NoiseAPIInit") +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERY_VERBOSE #define HELPER_LOG(msg, ...) \ - ESP_LOGVV(TAG, "%s (%s): " msg, this->client_info_->name.c_str(), this->client_info_->peername.c_str(), ##__VA_ARGS__) + do { \ + char peername__[socket::PEERNAME_MAX_LEN]; \ + this->socket_->getpeername_to(peername__); \ + ESP_LOGVV(TAG, "%s (%s): " msg, this->client_info_->name.c_str(), peername__, ##__VA_ARGS__); \ + } while (0) +#else +#define HELPER_LOG(msg, ...) ((void) 0) +#endif #ifdef HELPER_LOG_PACKETS #define LOG_PACKET_RECEIVED(buffer) ESP_LOGVV(TAG, "Received frame: %s", format_hex_pretty(buffer).c_str()) diff --git a/esphome/components/api/api_frame_helper_plaintext.cpp b/esphome/components/api/api_frame_helper_plaintext.cpp index b5d90b2429..21fa2f5ef6 100644 --- a/esphome/components/api/api_frame_helper_plaintext.cpp +++ b/esphome/components/api/api_frame_helper_plaintext.cpp @@ -18,8 +18,16 @@ namespace esphome::api { static const char *const TAG = "api.plaintext"; +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERY_VERBOSE #define HELPER_LOG(msg, ...) \ - ESP_LOGVV(TAG, "%s (%s): " msg, this->client_info_->name.c_str(), this->client_info_->peername.c_str(), ##__VA_ARGS__) + do { \ + char peername__[socket::PEERNAME_MAX_LEN]; \ + this->socket_->getpeername_to(peername__); \ + ESP_LOGVV(TAG, "%s (%s): " msg, this->client_info_->name.c_str(), peername__, ##__VA_ARGS__); \ + } while (0) +#else +#define HELPER_LOG(msg, ...) ((void) 0) +#endif #ifdef HELPER_LOG_PACKETS #define LOG_PACKET_RECEIVED(buffer) ESP_LOGVV(TAG, "Received frame: %s", format_hex_pretty(buffer).c_str()) diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index b1a5ee5d57..0d84ab58e0 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -127,13 +127,17 @@ void APIServer::loop() { // Check if we're at the connection limit if (this->clients_.size() >= this->max_connections_) { - ESP_LOGW(TAG, "Max connections (%d), rejecting %s", this->max_connections_, sock->getpeername().c_str()); + char peername[socket::PEERNAME_MAX_LEN]; + sock->getpeername_to(peername); + ESP_LOGW(TAG, "Max connections (%d), rejecting %s", this->max_connections_, peername); // Immediately close - socket destructor will handle cleanup sock.reset(); continue; } - ESP_LOGD(TAG, "Accept %s", sock->getpeername().c_str()); + char peername[socket::PEERNAME_MAX_LEN]; + sock->getpeername_to(peername); + ESP_LOGD(TAG, "Accept %s", peername); auto *conn = new APIConnection(std::move(sock), this); this->clients_.emplace_back(conn); @@ -166,8 +170,7 @@ void APIServer::loop() { // Network is down - disconnect all clients for (auto &client : this->clients_) { client->on_fatal_error(); - ESP_LOGW(TAG, "%s (%s): Network down; disconnect", client->client_info_.name.c_str(), - client->client_info_.peername.c_str()); + client->log_client_(ESPHOME_LOG_LEVEL_WARN, LOG_STR("Network down; disconnect")); } // Continue to process and clean up the clients below } @@ -185,7 +188,8 @@ void APIServer::loop() { // Rare case: handle disconnection #ifdef USE_API_CLIENT_DISCONNECTED_TRIGGER - this->client_disconnected_trigger_->trigger(client->client_info_.name, client->client_info_.peername); + // Trigger expects std::string, get fresh peername from socket + this->client_disconnected_trigger_->trigger(client->client_info_.name, client->get_peername()); #endif #ifdef USE_API_USER_DEFINED_ACTION_RESPONSES this->unregister_active_action_calls_for_connection(client.get()); diff --git a/esphome/components/socket/bsd_sockets_impl.cpp b/esphome/components/socket/bsd_sockets_impl.cpp index 09cd81752a..d3a44f573d 100644 --- a/esphome/components/socket/bsd_sockets_impl.cpp +++ b/esphome/components/socket/bsd_sockets_impl.cpp @@ -14,27 +14,34 @@ namespace esphome::socket { -std::string format_sockaddr(const struct sockaddr_storage &storage) { +// Format sockaddr into caller-provided buffer, returns length written (excluding null) +size_t format_sockaddr_to(const struct sockaddr_storage &storage, std::span buf) { if (storage.ss_family == AF_INET) { const struct sockaddr_in *addr = reinterpret_cast(&storage); - char buf[INET_ADDRSTRLEN]; - if (inet_ntop(AF_INET, &addr->sin_addr, buf, sizeof(buf)) != nullptr) - return std::string{buf}; + if (inet_ntop(AF_INET, &addr->sin_addr, buf.data(), buf.size()) != nullptr) + return strlen(buf.data()); } #if LWIP_IPV6 else if (storage.ss_family == AF_INET6) { const struct sockaddr_in6 *addr = reinterpret_cast(&storage); - char buf[INET6_ADDRSTRLEN]; // Format IPv4-mapped IPv6 addresses as regular IPv4 addresses if (addr->sin6_addr.un.u32_addr[0] == 0 && addr->sin6_addr.un.u32_addr[1] == 0 && addr->sin6_addr.un.u32_addr[2] == htonl(0xFFFF) && - inet_ntop(AF_INET, &addr->sin6_addr.un.u32_addr[3], buf, sizeof(buf)) != nullptr) { - return std::string{buf}; + inet_ntop(AF_INET, &addr->sin6_addr.un.u32_addr[3], buf.data(), buf.size()) != nullptr) { + return strlen(buf.data()); } - if (inet_ntop(AF_INET6, &addr->sin6_addr, buf, sizeof(buf)) != nullptr) - return std::string{buf}; + if (inet_ntop(AF_INET6, &addr->sin6_addr, buf.data(), buf.size()) != nullptr) + return strlen(buf.data()); } #endif + buf[0] = '\0'; + return 0; +} + +std::string format_sockaddr(const struct sockaddr_storage &storage) { + char buf[PEERNAME_MAX_LEN]; + if (format_sockaddr_to(storage, buf) > 0) + return std::string{buf}; return {}; } @@ -100,6 +107,15 @@ class BSDSocketImpl : public Socket { return {}; return format_sockaddr(storage); } + size_t getpeername_to(std::span buf) override { + struct sockaddr_storage storage; + socklen_t len = sizeof(storage); + if (::getpeername(this->fd_, (struct sockaddr *) &storage, &len) != 0) { + buf[0] = '\0'; + return 0; + } + return format_sockaddr_to(storage, buf); + } int getsockname(struct sockaddr *addr, socklen_t *addrlen) override { return ::getsockname(this->fd_, addr, addrlen); } diff --git a/esphome/components/socket/lwip_raw_tcp_impl.cpp b/esphome/components/socket/lwip_raw_tcp_impl.cpp index cb5d17d5af..671d5c94bb 100644 --- a/esphome/components/socket/lwip_raw_tcp_impl.cpp +++ b/esphome/components/socket/lwip_raw_tcp_impl.cpp @@ -196,6 +196,14 @@ class LWIPRawImpl : public Socket { } return this->format_ip_address_(pcb_->remote_ip); } + size_t getpeername_to(std::span buf) override { + if (pcb_ == nullptr) { + errno = ECONNRESET; + buf[0] = '\0'; + return 0; + } + return this->format_ip_address_to_(pcb_->remote_ip, buf); + } int getsockname(struct sockaddr *name, socklen_t *addrlen) override { if (pcb_ == nullptr) { errno = ECONNRESET; @@ -517,17 +525,27 @@ class LWIPRawImpl : public Socket { } protected: - std::string format_ip_address_(const ip_addr_t &ip) { - char buffer[50] = {}; + // Format IP address into caller-provided buffer, returns length written (excluding null) + size_t format_ip_address_to_(const ip_addr_t &ip, std::span buf) { if (IP_IS_V4_VAL(ip)) { - inet_ntoa_r(ip, buffer, sizeof(buffer)); + inet_ntoa_r(ip, buf.data(), buf.size()); + return strlen(buf.data()); } #if LWIP_IPV6 else if (IP_IS_V6_VAL(ip)) { - inet6_ntoa_r(ip, buffer, sizeof(buffer)); + inet6_ntoa_r(ip, buf.data(), buf.size()); + return strlen(buf.data()); } #endif - return std::string(buffer); + buf[0] = '\0'; + return 0; + } + + std::string format_ip_address_(const ip_addr_t &ip) { + char buffer[PEERNAME_MAX_LEN]; + if (format_ip_address_to_(ip, buffer) > 0) + return std::string(buffer); + return {}; } int ip2sockaddr_(ip_addr_t *ip, uint16_t port, struct sockaddr *name, socklen_t *addrlen) { diff --git a/esphome/components/socket/lwip_sockets_impl.cpp b/esphome/components/socket/lwip_sockets_impl.cpp index 23fb1a7f6f..8a694d26b0 100644 --- a/esphome/components/socket/lwip_sockets_impl.cpp +++ b/esphome/components/socket/lwip_sockets_impl.cpp @@ -9,25 +9,36 @@ namespace esphome::socket { -std::string format_sockaddr(const struct sockaddr_storage &storage) { +// Format sockaddr into caller-provided buffer, returns length written (excluding null) +size_t format_sockaddr_to(const struct sockaddr_storage &storage, std::span buf) { if (storage.ss_family == AF_INET) { const struct sockaddr_in *addr = reinterpret_cast(&storage); - char buf[INET_ADDRSTRLEN]; - const char *ret = lwip_inet_ntop(AF_INET, &addr->sin_addr, buf, sizeof(buf)); - if (ret == nullptr) - return {}; - return std::string{buf}; + const char *ret = lwip_inet_ntop(AF_INET, &addr->sin_addr, buf.data(), buf.size()); + if (ret == nullptr) { + buf[0] = '\0'; + return 0; + } + return strlen(buf.data()); } #if LWIP_IPV6 else if (storage.ss_family == AF_INET6) { const struct sockaddr_in6 *addr = reinterpret_cast(&storage); - char buf[INET6_ADDRSTRLEN]; - const char *ret = lwip_inet_ntop(AF_INET6, &addr->sin6_addr, buf, sizeof(buf)); - if (ret == nullptr) - return {}; - return std::string{buf}; + const char *ret = lwip_inet_ntop(AF_INET6, &addr->sin6_addr, buf.data(), buf.size()); + if (ret == nullptr) { + buf[0] = '\0'; + return 0; + } + return strlen(buf.data()); } #endif + buf[0] = '\0'; + return 0; +} + +std::string format_sockaddr(const struct sockaddr_storage &storage) { + char buf[PEERNAME_MAX_LEN]; + if (format_sockaddr_to(storage, buf) > 0) + return std::string{buf}; return {}; } @@ -95,6 +106,15 @@ class LwIPSocketImpl : public Socket { return {}; return format_sockaddr(storage); } + size_t getpeername_to(std::span buf) override { + struct sockaddr_storage storage; + socklen_t len = sizeof(storage); + if (lwip_getpeername(this->fd_, (struct sockaddr *) &storage, &len) != 0) { + buf[0] = '\0'; + return 0; + } + return format_sockaddr_to(storage, buf); + } int getsockname(struct sockaddr *addr, socklen_t *addrlen) override { return lwip_getsockname(this->fd_, addr, addrlen); } diff --git a/esphome/components/socket/socket.h b/esphome/components/socket/socket.h index 75eb07de4a..8fa2cf328d 100644 --- a/esphome/components/socket/socket.h +++ b/esphome/components/socket/socket.h @@ -1,5 +1,6 @@ #pragma once #include +#include #include #include "esphome/core/optional.h" @@ -8,6 +9,15 @@ #if defined(USE_SOCKET_IMPL_LWIP_TCP) || defined(USE_SOCKET_IMPL_LWIP_SOCKETS) || defined(USE_SOCKET_IMPL_BSD_SOCKETS) namespace esphome::socket { +// Maximum length for peer name string (IP address without port) +// IPv4: "255.255.255.255" = 15 chars + null = 16 +// IPv6: full address = 45 chars + null = 46 +#if LWIP_IPV6 +static constexpr size_t PEERNAME_MAX_LEN = 46; // INET6_ADDRSTRLEN +#else +static constexpr size_t PEERNAME_MAX_LEN = 16; // INET_ADDRSTRLEN +#endif + class Socket { public: Socket() = default; @@ -32,6 +42,9 @@ class Socket { virtual int getpeername(struct sockaddr *addr, socklen_t *addrlen) = 0; virtual std::string getpeername() = 0; + /// Format peer address into a fixed-size buffer (no heap allocation) + /// Returns number of characters written (excluding null terminator), or 0 on error + virtual size_t getpeername_to(std::span buf) = 0; virtual int getsockname(struct sockaddr *addr, socklen_t *addrlen) = 0; virtual std::string getsockname() = 0; virtual int getsockopt(int level, int optname, void *optval, socklen_t *optlen) = 0;