Fix socket connection closed not detected (#2587)

This commit is contained in:
Otto Winter 2021-10-22 10:46:44 +02:00 committed by Otto winter
parent bbcd523967
commit f93e7d4e3a
No known key found for this signature in database
GPG key ID: 48ED2DDB96D7682C
5 changed files with 53 additions and 15 deletions

View file

@ -78,6 +78,8 @@ void APIConnection::loop() {
on_fatal_error(); on_fatal_error();
if (err == APIError::SOCKET_READ_FAILED && errno == ECONNRESET) { if (err == APIError::SOCKET_READ_FAILED && errno == ECONNRESET) {
ESP_LOGW(TAG, "%s: Connection reset", client_info_.c_str()); ESP_LOGW(TAG, "%s: Connection reset", client_info_.c_str());
} else if (err == APIError::CONNECTION_CLOSED) {
ESP_LOGW(TAG, "%s: Connection closed", client_info_.c_str());
} else { } else {
ESP_LOGW(TAG, "%s: Reading failed: %s errno=%d", client_info_.c_str(), api_error_to_str(err), errno); ESP_LOGW(TAG, "%s: Reading failed: %s errno=%d", client_info_.c_str(), api_error_to_str(err), errno);
} }

View file

@ -10,7 +10,7 @@ namespace api {
static const char *const TAG = "api.socket"; static const char *const TAG = "api.socket";
/// Is the given return value (from read/write syscalls) a wouldblock error? /// Is the given return value (from write syscalls) a wouldblock error?
bool is_would_block(ssize_t ret) { bool is_would_block(ssize_t ret) {
if (ret == -1) { if (ret == -1) {
return errno == EWOULDBLOCK || errno == EAGAIN; return errno == EWOULDBLOCK || errno == EAGAIN;
@ -64,6 +64,8 @@ const char *api_error_to_str(APIError err) {
return "HANDSHAKESTATE_SPLIT_FAILED"; return "HANDSHAKESTATE_SPLIT_FAILED";
} else if (err == APIError::BAD_HANDSHAKE_ERROR_BYTE) { } else if (err == APIError::BAD_HANDSHAKE_ERROR_BYTE) {
return "BAD_HANDSHAKE_ERROR_BYTE"; return "BAD_HANDSHAKE_ERROR_BYTE";
} else if (err == APIError::CONNECTION_CLOSED) {
return "CONNECTION_CLOSED";
} }
return "UNKNOWN"; return "UNKNOWN";
} }
@ -185,12 +187,17 @@ APIError APINoiseFrameHelper::try_read_frame_(ParsedFrame *frame) {
// no header information yet // no header information yet
size_t to_read = 3 - rx_header_buf_len_; size_t to_read = 3 - rx_header_buf_len_;
ssize_t received = socket_->read(&rx_header_buf_[rx_header_buf_len_], to_read); ssize_t received = socket_->read(&rx_header_buf_[rx_header_buf_len_], to_read);
if (is_would_block(received)) { if (received == -1) {
return APIError::WOULD_BLOCK; if (errno == EWOULDBLOCK || errno == EAGAIN) {
} else if (received == -1) { return APIError::WOULD_BLOCK;
}
state_ = State::FAILED; state_ = State::FAILED;
HELPER_LOG("Socket read failed with errno %d", errno); HELPER_LOG("Socket read failed with errno %d", errno);
return APIError::SOCKET_READ_FAILED; return APIError::SOCKET_READ_FAILED;
} else if (received == 0) {
state_ = State::FAILED;
HELPER_LOG("Connection closed");
return APIError::CONNECTION_CLOSED;
} }
rx_header_buf_len_ += received; rx_header_buf_len_ += received;
if (received != to_read) { if (received != to_read) {
@ -227,12 +234,17 @@ APIError APINoiseFrameHelper::try_read_frame_(ParsedFrame *frame) {
// more data to read // more data to read
size_t to_read = msg_size - rx_buf_len_; size_t to_read = msg_size - rx_buf_len_;
ssize_t received = socket_->read(&rx_buf_[rx_buf_len_], to_read); ssize_t received = socket_->read(&rx_buf_[rx_buf_len_], to_read);
if (is_would_block(received)) { if (received == -1) {
return APIError::WOULD_BLOCK; if (errno == EWOULDBLOCK || errno == EAGAIN) {
} else if (received == -1) { return APIError::WOULD_BLOCK;
}
state_ = State::FAILED; state_ = State::FAILED;
HELPER_LOG("Socket read failed with errno %d", errno); HELPER_LOG("Socket read failed with errno %d", errno);
return APIError::SOCKET_READ_FAILED; return APIError::SOCKET_READ_FAILED;
} else if (received == 0) {
state_ = State::FAILED;
HELPER_LOG("Connection closed");
return APIError::CONNECTION_CLOSED;
} }
rx_buf_len_ += received; rx_buf_len_ += received;
if (received != to_read) { if (received != to_read) {
@ -778,12 +790,17 @@ APIError APIPlaintextFrameHelper::try_read_frame_(ParsedFrame *frame) {
while (!rx_header_parsed_) { while (!rx_header_parsed_) {
uint8_t data; uint8_t data;
ssize_t received = socket_->read(&data, 1); ssize_t received = socket_->read(&data, 1);
if (is_would_block(received)) { if (received == -1) {
return APIError::WOULD_BLOCK; if (errno == EWOULDBLOCK || errno == EAGAIN) {
} else if (received == -1) { return APIError::WOULD_BLOCK;
}
state_ = State::FAILED; state_ = State::FAILED;
HELPER_LOG("Socket read failed with errno %d", errno); HELPER_LOG("Socket read failed with errno %d", errno);
return APIError::SOCKET_READ_FAILED; return APIError::SOCKET_READ_FAILED;
} else if (received == 0) {
state_ = State::FAILED;
HELPER_LOG("Connection closed");
return APIError::CONNECTION_CLOSED;
} }
rx_header_buf_.push_back(data); rx_header_buf_.push_back(data);
@ -824,12 +841,17 @@ APIError APIPlaintextFrameHelper::try_read_frame_(ParsedFrame *frame) {
// more data to read // more data to read
size_t to_read = rx_header_parsed_len_ - rx_buf_len_; size_t to_read = rx_header_parsed_len_ - rx_buf_len_;
ssize_t received = socket_->read(&rx_buf_[rx_buf_len_], to_read); ssize_t received = socket_->read(&rx_buf_[rx_buf_len_], to_read);
if (is_would_block(received)) { if (received == -1) {
return APIError::WOULD_BLOCK; if (errno == EWOULDBLOCK || errno == EAGAIN) {
} else if (received == -1) { return APIError::WOULD_BLOCK;
}
state_ = State::FAILED; state_ = State::FAILED;
HELPER_LOG("Socket read failed with errno %d", errno); HELPER_LOG("Socket read failed with errno %d", errno);
return APIError::SOCKET_READ_FAILED; return APIError::SOCKET_READ_FAILED;
} else if (received == 0) {
state_ = State::FAILED;
HELPER_LOG("Connection closed");
return APIError::CONNECTION_CLOSED;
} }
rx_buf_len_ += received; rx_buf_len_ += received;
if (received != to_read) { if (received != to_read) {

View file

@ -53,6 +53,7 @@ enum class APIError : int {
HANDSHAKESTATE_SETUP_FAILED = 1019, HANDSHAKESTATE_SETUP_FAILED = 1019,
HANDSHAKESTATE_SPLIT_FAILED = 1020, HANDSHAKESTATE_SPLIT_FAILED = 1020,
BAD_HANDSHAKE_ERROR_BYTE = 1021, BAD_HANDSHAKE_ERROR_BYTE = 1021,
CONNECTION_CLOSED = 1022,
}; };
const char *api_error_to_str(APIError err); const char *api_error_to_str(APIError err);

View file

@ -275,6 +275,12 @@ void OTAComponent::handle_() {
} }
ESP_LOGW(TAG, "Error receiving data for update, errno: %d", errno); ESP_LOGW(TAG, "Error receiving data for update, errno: %d", errno);
goto error; goto error;
} else if (read == 0) {
// $ man recv
// "When a stream socket peer has performed an orderly shutdown, the return value will
// be 0 (the traditional "end-of-file" return)."
ESP_LOGW(TAG, "Remote end closed connection");
goto error;
} }
error_code = backend->write(buf, read); error_code = backend->write(buf, read);
@ -362,6 +368,9 @@ bool OTAComponent::readall_(uint8_t *buf, size_t len) {
} }
ESP_LOGW(TAG, "Failed to read %d bytes of data, errno: %d", len, errno); ESP_LOGW(TAG, "Failed to read %d bytes of data, errno: %d", len, errno);
return false; return false;
} else if (read == 0) {
ESP_LOGW(TAG, "Remote closed connection");
return false;
} else { } else {
at += read; at += read;
} }

View file

@ -320,8 +320,7 @@ class LWIPRawImpl : public Socket {
return -1; return -1;
} }
if (rx_closed_ && rx_buf_ == nullptr) { if (rx_closed_ && rx_buf_ == nullptr) {
errno = ECONNRESET; return 0;
return -1;
} }
if (len == 0) { if (len == 0) {
return 0; return 0;
@ -366,6 +365,11 @@ class LWIPRawImpl : public Socket {
read += copysize; read += copysize;
} }
if (read == 0) {
errno = EWOULDBLOCK;
return -1;
}
return read; return read;
} }
ssize_t readv(const struct iovec *iov, int iovcnt) override { ssize_t readv(const struct iovec *iov, int iovcnt) override {