From 093ea0045958428654f962cf57ce0985ee918bc5 Mon Sep 17 00:00:00 2001 From: Richard Allen Date: Thu, 10 Oct 2024 17:31:21 -0500 Subject: [PATCH 1/2] fix(ws_transport): correct split header bytes When the underlying transport returns header, length, or mask bytes early, again call the underlying transport. This solves the WS parser getting offset when the server sends a burst of frames where the last WS header is split across packet boundaries, so fewer than the needed bytes may be available. --- components/tcp_transport/transport_ws.c | 36 ++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index 0211347fba03..1e100a626935 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -133,6 +133,34 @@ static int esp_transport_read_internal(transport_ws_t *ws, char *buffer, int len return to_read; } +static int esp_transport_read_exact_size(transport_ws_t *ws, char *buffer, int requested_len, int timeout_ms) +{ + int total_read = 0; + int len = requested_len; + + while (len > 0) { + int bytes_read = esp_transport_read_internal(ws, buffer, len, timeout_ms); + + if (bytes_read < 0) { + return bytes_read; // Return error from the underlying read + } + + if (bytes_read == 0) { + // If we read 0 bytes, we return an error, since reading exact number of bytes resulted in a timeout operation + ESP_LOGW(TAG, "Requested to read %d, actually read %d bytes", requested_len, total_read); + return -1; + } + + // Update buffer and remaining length + buffer += bytes_read; + len -= bytes_read; + total_read += bytes_read; + + ESP_LOGV(TAG, "Read fragment of %d bytes", bytes_read); + } + return total_read; +} + static char *trimwhitespace(char *str) { char *end; @@ -486,7 +514,7 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t // Receive and process header first (based on header size) int header = 2; int mask_len = 4; - if ((rlen = esp_transport_read_internal(ws, data_ptr, header, timeout_ms)) <= 0) { + if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { ESP_LOGE(TAG, "Error read data"); return rlen; } @@ -500,7 +528,7 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t ESP_LOGD(TAG, "Opcode: %d, mask: %d, len: %d", ws->frame_state.opcode, mask, payload_len); if (payload_len == 126) { // headerLen += 2; - if ((rlen = esp_transport_read_internal(ws, data_ptr, header, timeout_ms)) <= 0) { + if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { ESP_LOGE(TAG, "Error read data"); return rlen; } @@ -508,7 +536,7 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t } else if (payload_len == 127) { // headerLen += 8; header = 8; - if ((rlen = esp_transport_read_internal(ws, data_ptr, header, timeout_ms)) <= 0) { + if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { ESP_LOGE(TAG, "Error read data"); return rlen; } @@ -523,7 +551,7 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t if (mask) { // Read and store mask - if (payload_len != 0 && (rlen = esp_transport_read_internal(ws, buffer, mask_len, timeout_ms)) <= 0) { + if (payload_len != 0 && (rlen = esp_transport_read_exact_size(ws, buffer, mask_len, timeout_ms)) <= 0) { ESP_LOGE(TAG, "Error read data"); return rlen; } From 175a42c0eb358d8610ed81b8fd924b9df816c79d Mon Sep 17 00:00:00 2001 From: Richard Allen Date: Wed, 30 Oct 2024 08:49:38 -0500 Subject: [PATCH 2/2] feat(ws_transport): include error reason in failure log When the underlying transport returns a failure, add the failure result to the log. --- components/tcp_transport/transport_ws.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index 1e100a626935..6ccfe369f9c3 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -385,7 +385,7 @@ static int _ws_write(esp_transport_handle_t t, int opcode, int mask_flag, const int poll_write; if ((poll_write = esp_transport_poll_write(ws->parent, timeout_ms)) <= 0) { - ESP_LOGE(TAG, "Error transport_poll_write"); + ESP_LOGE(TAG, "Error transport_poll_write(%i)", poll_write); return poll_write; } ws_header[header_len++] = opcode; @@ -484,7 +484,7 @@ static int ws_read_payload(esp_transport_handle_t t, char *buffer, int len, int // Receive and process payload if (bytes_to_read != 0 && (rlen = esp_transport_read_internal(ws, buffer, bytes_to_read, timeout_ms)) <= 0) { - ESP_LOGE(TAG, "Error read data"); + ESP_LOGE(TAG, "Error read data(%i)", rlen); return rlen; } ws->frame_state.bytes_remaining -= rlen; @@ -515,7 +515,7 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t int header = 2; int mask_len = 4; if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { - ESP_LOGE(TAG, "Error read data"); + ESP_LOGE(TAG, "Error read data(%i)", rlen); return rlen; } ws->frame_state.header_received = true; @@ -529,7 +529,7 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t if (payload_len == 126) { // headerLen += 2; if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { - ESP_LOGE(TAG, "Error read data"); + ESP_LOGE(TAG, "Error read data(%i)", rlen); return rlen; } payload_len = (uint8_t)data_ptr[0] << 8 | (uint8_t)data_ptr[1]; @@ -537,7 +537,7 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t // headerLen += 8; header = 8; if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { - ESP_LOGE(TAG, "Error read data"); + ESP_LOGE(TAG, "Error read data(%i)", rlen); return rlen; } @@ -552,7 +552,7 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t if (mask) { // Read and store mask if (payload_len != 0 && (rlen = esp_transport_read_exact_size(ws, buffer, mask_len, timeout_ms)) <= 0) { - ESP_LOGE(TAG, "Error read data"); + ESP_LOGE(TAG, "Error read data(%i)", rlen); return rlen; } memcpy(ws->frame_state.mask_key, buffer, mask_len); @@ -647,7 +647,7 @@ static int ws_read(esp_transport_handle_t t, char *buffer, int len, int timeout_ if (ws->frame_state.payload_len) { if ( (rlen = ws_read_payload(t, buffer, len, timeout_ms)) <= 0) { - ESP_LOGE(TAG, "Error reading payload data"); + ESP_LOGE(TAG, "Error reading payload data(%i)", rlen); ws->frame_state.bytes_remaining = 0; return rlen; }