diff --git a/components/tcp_transport/host_test/main/test_websocket_transport.cpp b/components/tcp_transport/host_test/main/test_websocket_transport.cpp index d31a07f4b13..7c7998cb0af 100644 --- a/components/tcp_transport/host_test/main/test_websocket_transport.cpp +++ b/components/tcp_transport/host_test/main/test_websocket_transport.cpp @@ -316,6 +316,36 @@ TEST_CASE("WebSocket Transport Connection", "[success]") // Verify the marker after the buffer wasn't overwritten REQUIRE(response_header_buffer[ws_config.response_headers_len] == marker); } + + SECTION("Poll read with buffered data") { + // Set the callback function for mock_read + mock_read_Stub(mock_valid_read_callback); + + static int parent_poll_calls = 0; + parent_poll_calls = 0; + + // Verify poll_read is not called when buffer has data + mock_poll_read_Stub([](esp_transport_handle_t t, int timeout_ms, int num_call){ + parent_poll_calls++; + return 0; + }); + + REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == 0); + + // buffer should contain "Test" (4 bytes) + // ws_poll_read should return 1 because buffer is not empty + REQUIRE(esp_transport_poll_read(websocket_transport.get(), timeout) == 1); + REQUIRE(parent_poll_calls == 0); + + // Read the data to empty the buffer + char buffer[10]; + int read_len = esp_transport_read(websocket_transport.get(), buffer, sizeof(buffer), timeout); + REQUIRE(read_len == 4); + + // Now buffer is empty, ws_poll_read should call parent poll + esp_transport_poll_read(websocket_transport.get(), timeout); + REQUIRE(parent_poll_calls == 1); + } } TEST_CASE("WebSocket Transport Connection", "[failure]") @@ -399,4 +429,30 @@ TEST_CASE("WebSocket Transport Connection", "[failure]") // Verify the response header is empty REQUIRE(std::string(response_header_buffer.data()) == ""); } + + SECTION("ws connect fails (buffer full, no delimiter)") { + // Mock read to fill buffer with non-delimiter data + mock_read_Stub([](esp_transport_handle_t h, char *buf, int len, int tout, int n) { + if (len > 0) { + memset(buf, 'A', len); + } + return len; + }); + mock_poll_read_Stub(mock_poll_read_callback); + + REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == -1); + } + + SECTION("ws connect succeeds (response header buffer too small for handshake verification)") { + // Set a very small response header buffer + ws_config.response_headers_len = 50; + REQUIRE(esp_transport_ws_set_config(websocket_transport.get(), &ws_config) == ESP_OK); + + // Set the callback function for mock_read + mock_read_Stub(mock_valid_read_callback); + mock_poll_read_Stub(mock_poll_read_callback); + + // Connect should now succeed even with small user buffer + REQUIRE(esp_transport_connect(websocket_transport.get(), host, port, timeout) == 0); + } } diff --git a/components/tcp_transport/transport_ws.c b/components/tcp_transport/transport_ws.c index 35dfad98d7b..328737d393f 100644 --- a/components/tcp_transport/transport_ws.c +++ b/components/tcp_transport/transport_ws.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -271,22 +271,27 @@ static int ws_connect(esp_transport_handle_t t, const char *host, int port, int ESP_LOGD(TAG, "Read header chunk %d, current header size: %d", len, header_len); } while (NULL == strstr(ws->buffer, delimiter) && header_len < WS_BUFFER_SIZE - 1); - if (header_len >= WS_BUFFER_SIZE - 1) { + if (header_len > WS_BUFFER_SIZE - 1) { ESP_LOGE(TAG, "Header size exceeded buffer size (need=%d, max=%d)", header_len + 1, WS_BUFFER_SIZE); return -1; } if(ws->response_header) { + int copy_len = header_len; if(ws->response_header_len - 1 < header_len) { ESP_LOGW(TAG, "Received header length exceedes the allocated buffer size (need=%d, allocated=%d), truncating to allocated size", header_len, ws->response_header_len); - header_len = ws->response_header_len; + copy_len = ws->response_header_len - 1; } // Copy response header to the static array - strncpy(ws->response_header, ws->buffer, header_len); - ws->response_header[ws->response_header_len - 1] = '\0'; + strncpy(ws->response_header, ws->buffer, copy_len); + ws->response_header[copy_len] = '\0'; } char* delim_ptr = strstr(ws->buffer, delimiter); + if (!delim_ptr) { + ESP_LOGE(TAG, "Header size exceeded buffer size or delimiter not found"); + return -1; + } ws->http_status_code = get_http_status_code(ws->buffer); if (ws->http_status_code == -1) { @@ -510,7 +515,9 @@ 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(%d)", rlen); + if (rlen < 0) { + ESP_LOGE(TAG, "Error read data(%d)", rlen); + } return rlen; } ws->frame_state.bytes_remaining -= rlen; @@ -559,10 +566,12 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t char ws_header[MAX_WEBSOCKET_HEADER_SIZE]; char *data_ptr = ws_header, mask; int rlen; - int poll_read; ws->frame_state.header_received = false; - if ((poll_read = esp_transport_poll_read(ws->parent, timeout_ms)) <= 0) { - return poll_read; + if (ws->buffer_len == 0) { + int poll_read = esp_transport_poll_read(ws->parent, timeout_ms); + if (poll_read <= 0) { + return poll_read; + } } // Receive and process header first (based on header size) @@ -575,11 +584,33 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t ws->frame_state.header_received = true; ws->frame_state.fin = (*data_ptr & 0x80) != 0; ws->frame_state.opcode = (*data_ptr & 0x0F); + uint8_t rsv = (*data_ptr & 0x70); data_ptr ++; mask = ((*data_ptr >> 7) & 0x01); payload_len = (*data_ptr & 0x7F); data_ptr++; - ESP_LOGD(TAG, "Opcode: %d, mask: %d, len: %d", ws->frame_state.opcode, mask, payload_len); + ESP_LOGD(TAG, "Opcode: %d, mask: %d, len: %d, rsv: 0x%02X", ws->frame_state.opcode, mask, payload_len, rsv); + + // RFC 6455 Section 5.2: RSV bits MUST be 0 unless an extension is negotiated + if (rsv != 0) { + ESP_LOGE(TAG, "Non-zero RSV bits detected (rsv=0x%02X) - protocol violation, no extensions negotiated", rsv); + return -1; + } + + // RFC 6455 Section 5.2: Validate opcode (only 0x0-0x2 for data, 0x8-0xA for control are defined) + if ((ws->frame_state.opcode >= 0x3 && ws->frame_state.opcode <= 0x7) || + (ws->frame_state.opcode >= 0xB && ws->frame_state.opcode <= 0xF)) { + ESP_LOGE(TAG, "Reserved opcode detected (opcode=0x%02X) - protocol violation", ws->frame_state.opcode); + return -1; + } + + // RFC 6455 Section 5.5: Control frames MUST NOT be fragmented + if ((ws->frame_state.opcode & WS_OPCODE_CONTROL_FRAME) && !ws->frame_state.fin) { + ESP_LOGE(TAG, "Fragmented control frame detected (opcode=0x%02X, fin=%d) - protocol violation", + ws->frame_state.opcode, ws->frame_state.fin); + return -1; + } + if (payload_len == 126) { // headerLen += 2; if ((rlen = esp_transport_read_exact_size(ws, data_ptr, header, timeout_ms)) <= 0) { @@ -602,7 +633,12 @@ static int ws_read_header(esp_transport_handle_t t, char *buffer, int len, int t payload_len = (uint8_t)data_ptr[4] << 24 | (uint8_t)data_ptr[5] << 16 | (uint8_t)data_ptr[6] << 8 | data_ptr[7]; } } - + // RFC 6455 Section 5.5: Control frames MUST have payload length of 125 bytes or less + if ((ws->frame_state.opcode & WS_OPCODE_CONTROL_FRAME) && payload_len > 125) { + ESP_LOGE(TAG, "Control frame with excessive payload detected (opcode=0x%02X, payload_len=%d) - protocol violation", + ws->frame_state.opcode, payload_len); + return -1; + } if (mask) { // Read and store mask if (payload_len != 0 && (rlen = esp_transport_read_exact_size(ws, buffer, mask_len, timeout_ms)) <= 0) { @@ -701,8 +737,10 @@ 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(%d)", rlen); - ws->frame_state.bytes_remaining = 0; + if (rlen < 0) { + ESP_LOGE(TAG, "Error reading payload data(%d)", rlen); + ws->frame_state.bytes_remaining = 0; + } return rlen; } } @@ -714,6 +752,10 @@ static int ws_read(esp_transport_handle_t t, char *buffer, int len, int timeout_ static int ws_poll_read(esp_transport_handle_t t, int timeout_ms) { transport_ws_t *ws = esp_transport_get_context_data(t); + if (ws->buffer_len > 0) { + ESP_LOGD(TAG, "ws_poll_read: buffered data available (%zu bytes)", ws->buffer_len); + return 1; + } return esp_transport_poll_read(ws->parent, timeout_ms); }