Extract a reusable connect poll helper and enforce bounds at the write

This commit is contained in:
J. Nick Koston
2026-09-02 11:58:51 +02:00
parent ae549b25bb
commit 1171fbf892
6 changed files with 62 additions and 23 deletions
@@ -88,6 +88,9 @@ void OutgoingConnectionManager::try_dial_(APIServer *server, uint32_t now) {
// (covers an IPv6 literal left by an earlier enable_ipv6 build too)
this->saved_ = {};
this->host_persisted_ = this->target_pref_.save(&this->saved_) && global_preferences->sync();
if (!this->host_persisted_) {
ESP_LOGW(TAG, "Failed to clear target");
}
#endif
this->schedule_retry_(now);
return;
@@ -125,12 +128,26 @@ void OutgoingConnectionManager::poll_connect_(APIServer *server, uint32_t now) {
return;
}
this->last_poll_ = now;
int fd = this->dial_socket_->get_fd();
int err = 0;
switch (poll_connect(*this->dial_socket_, err)) {
case ConnectPollResult::CONNECT_POLL_PENDING:
break;
case ConnectPollResult::CONNECT_POLL_CONNECTED:
this->handoff_(server, now);
break;
case ConnectPollResult::CONNECT_POLL_ERROR:
ESP_LOGW(TAG, "Connect failed: %d", err);
this->schedule_retry_(now);
break;
}
}
ConnectPollResult poll_connect(socket::Socket &sock, int &err_out) {
int fd = sock.get_fd();
if (fd < 0 || fd >= FD_SETSIZE) {
// FD_SET on either is undefined behavior
ESP_LOGW(TAG, "fd %d unusable for select", fd);
this->schedule_retry_(now);
return;
err_out = EBADF;
return ConnectPollResult::CONNECT_POLL_ERROR;
}
// Connect completion is a write event; the main loop only selects on reads
fd_set writefds;
@@ -145,26 +162,23 @@ void OutgoingConnectionManager::poll_connect_(APIServer *server, uint32_t now) {
int ret = ::select(fd + 1, nullptr, &writefds, nullptr, &tv);
#endif
if (ret < 0) {
ESP_LOGW(TAG, "Connect poll failed: errno %d", errno);
this->schedule_retry_(now);
return;
err_out = errno;
return ConnectPollResult::CONNECT_POLL_ERROR;
}
if (ret == 0 || !FD_ISSET(fd, &writefds)) {
return; // still in progress
return ConnectPollResult::CONNECT_POLL_PENDING;
}
int error = 0;
socklen_t len = sizeof(error);
if (this->dial_socket_->getsockopt(SOL_SOCKET, SO_ERROR, &error, &len) != 0) {
ESP_LOGW(TAG, "Connect status check failed: errno %d", errno);
this->schedule_retry_(now);
return;
if (sock.getsockopt(SOL_SOCKET, SO_ERROR, &error, &len) != 0) {
err_out = errno;
return ConnectPollResult::CONNECT_POLL_ERROR;
}
if (error != 0) {
ESP_LOGW(TAG, "Connect failed: %d", error);
this->schedule_retry_(now);
return;
err_out = error;
return ConnectPollResult::CONNECT_POLL_ERROR;
}
this->handoff_(server, now);
return ConnectPollResult::CONNECT_POLL_CONNECTED;
}
void OutgoingConnectionManager::handoff_(APIServer *server, uint32_t now) {
@@ -25,6 +25,18 @@ class APIConnection;
// target is simply relearned
static constexpr size_t SAVED_TARGET_HOST_LEN = socket::SOCKADDR_STR_LEN;
enum class ConnectPollResult : uint8_t {
CONNECT_POLL_PENDING,
CONNECT_POLL_CONNECTED,
CONNECT_POLL_ERROR,
};
/// Check a non-blocking connect() for completion without blocking. On
/// CONNECT_POLL_ERROR, err_out holds the socket's SO_ERROR, or errno when the
/// poll itself failed. Lives here for now; a candidate for the socket
/// component (async_tcp has a near-duplicate poll).
ConnectPollResult poll_connect(socket::Socket &sock, int &err_out);
struct SavedOutgoingTarget {
// IP as text so the socket component's v4-mapped-IPv6 normalization is
// reused on both ends; empty = none remembered
+10 -2
View File
@@ -282,7 +282,12 @@ void __attribute__((flatten)) APIServer::accept_new_connections_() {
}
}
void APIServer::add_client_(APIConnection *conn) {
bool APIServer::add_client_(APIConnection *conn) {
if (this->at_client_limit_()) {
// Callers check first; enforce the array bound where the write happens
delete conn;
return false;
}
this->clients_[this->api_connection_count_++].reset(conn);
conn->start();
@@ -292,6 +297,7 @@ void APIServer::add_client_(APIConnection *conn) {
if (this->api_connection_count_ == 1 && this->reboot_timeout_ != 0 && !this->provisioning_pending_()) {
this->status_clear_warning();
}
return true;
}
#ifdef USE_API_OUTGOING_CONNECTION
@@ -304,7 +310,9 @@ APIConnection *APIServer::add_outgoing_client_(std::unique_ptr<socket::Socket> s
return nullptr;
}
auto *conn = new APIConnection(std::move(sock), this);
this->add_client_(conn);
if (!this->add_client_(conn)) {
return nullptr;
}
// After start(): sends our server hello first so the peer can pick the key
conn->mark_outgoing();
return conn;
+2 -1
View File
@@ -264,7 +264,8 @@ class APIServer final : public Component,
// Accept incoming socket connections. Only called when socket has pending connections.
void __attribute__((noinline)) accept_new_connections_();
// Insert a constructed connection into the client slots and start it.
void add_client_(APIConnection *conn);
// Takes ownership; deletes the connection and returns false at the limit
bool add_client_(APIConnection *conn);
bool at_client_limit_() const { return this->api_connection_count_ >= MAX_API_CONNECTIONS; }
#ifdef USE_API_OUTGOING_CONNECTION
// Returns the new connection, or nullptr (socket dropped) when at the limit
@@ -59,13 +59,15 @@ int BSDSocketImpl::close() {
int BSDSocketImpl::setblocking(bool blocking) {
int fl = ::fcntl(this->fd_, F_GETFL, 0);
if (fl < 0) {
return fl;
}
if (blocking) {
fl &= ~O_NONBLOCK;
} else {
fl |= O_NONBLOCK;
}
::fcntl(this->fd_, F_SETFL, fl);
return 0;
return ::fcntl(this->fd_, F_SETFL, fl);
}
size_t BSDSocketImpl::getpeername_to(std::span<char, SOCKADDR_STR_LEN> buf) {
@@ -49,13 +49,15 @@ int LwIPSocketImpl::close() {
int LwIPSocketImpl::setblocking(bool blocking) {
int fl = lwip_fcntl(this->fd_, F_GETFL, 0);
if (fl < 0) {
return fl;
}
if (blocking) {
fl &= ~O_NONBLOCK;
} else {
fl |= O_NONBLOCK;
}
lwip_fcntl(this->fd_, F_SETFL, fl);
return 0;
return lwip_fcntl(this->fd_, F_SETFL, fl);
}
size_t LwIPSocketImpl::getpeername_to(std::span<char, SOCKADDR_STR_LEN> buf) {