web socket client issue.

[yehuda]

The web socket client example didn't work with web socket server (https://www.piesocket.com/websocket-tester) until we sent header and data at once.
Please see the the current sdk5.0 implementation and our changes below:

Code: [Select all] [Expand/Collapse]

1. //////////////////////////////////////////////
2. // current SDK code
3. //////////////////////////////////////////////
4.  
5. static int _ws_write(esp_transport_handle_t t, int opcode, int mask_flag, const char *b, int len, int timeout_ms)
6. {
7.     transport_ws_t *ws = esp_transport_get_context_data(t);
8.     char *buffer = (char *)b;
9.     char ws_header[MAX_WEBSOCKET_HEADER_SIZE];
10.     char *mask;
11.     int header_len = 0, i;
12.  
13.     int poll_write;
14.     if ((poll_write = esp_transport_poll_write(ws->parent, timeout_ms)) <= 0) {
15.         ESP_LOGE(TAG, "Error transport_poll_write");
16.         return poll_write;
17.     }
18.     ws_header[header_len++] = opcode;
19.  
20.     if (len <= 125) {
21.         ws_header[header_len++] = (uint8_t)(len | mask_flag);
22.     } else if (len < 65536) {
23.         ws_header[header_len++] = WS_SIZE16 | mask_flag;
24.         ws_header[header_len++] = (uint8_t)(len >> 8);
25.         ws_header[header_len++] = (uint8_t)(len & 0xFF);
26.     } else {
27.         ws_header[header_len++] = WS_SIZE64 | mask_flag;
28.         /* Support maximum 4 bytes length */
29.         ws_header[header_len++] = 0; //(uint8_t)((len >> 56) & 0xFF);
30.         ws_header[header_len++] = 0; //(uint8_t)((len >> 48) & 0xFF);
31.         ws_header[header_len++] = 0; //(uint8_t)((len >> 40) & 0xFF);
32.         ws_header[header_len++] = 0; //(uint8_t)((len >> 32) & 0xFF);
33.         ws_header[header_len++] = (uint8_t)((len >> 24) & 0xFF);
34.         ws_header[header_len++] = (uint8_t)((len >> 16) & 0xFF);
35.         ws_header[header_len++] = (uint8_t)((len >> 8) & 0xFF);
36.         ws_header[header_len++] = (uint8_t)((len >> 0) & 0xFF);
37.     }
38.  
39.     if (mask_flag) {
40.         mask = &ws_header[header_len];
41.         getrandom(ws_header + header_len, 4, 0);
42.         header_len += 4;
43.  
44.         for (i = 0; i < len; ++i) {
45.             buffer[i] = (buffer[i] ^ mask[i % 4]);
46.         }
47.     }
48.  
49.     if (esp_transport_write(ws->parent, ws_header, header_len, timeout_ms) != header_len) {
50.         ESP_LOGE(TAG, "Error write header");
51.         return -1;
52.     }
53.     if (len == 0) {
54.         return 0;
55.     }
56.  
57.     int ret = esp_transport_write(ws->parent, buffer, len, timeout_ms);
58.     // in case of masked transport we have to revert back to the original data, as ws layer
59.     // does not create its own copy of data to be sent
60.     if (mask_flag) {
61.         mask = &ws_header[header_len-4];
62.         for (i = 0; i < len; ++i) {
63.             buffer[i] = (buffer[i] ^ mask[i % 4]);
64.         }
65.     }
66.     return ret;
67. }
68.  
69. //////////////////////////////////////////////
70. // changed code
71. //////////////////////////////////////////////
72.  
73. static int _ws_write(esp_transport_handle_t t, int opcode, int mask_flag, const char *b, int len, int timeout_ms)
74. {
75.     transport_ws_t *ws = esp_transport_get_context_data(t);
76.     int header_len = 0, i;
77.     int poll_write;
78.     char *pData, *pMask = NULL;
79.  
80.     char* pBuffer = malloc(len+10); // allocation buffer with length of data plus maximal header length
81.     if(pBuffer == NULL) {
82.         ESP_LOGE(TAG, "Websocket transport: Failed to malloc buffer to sending...");
83.         return 0;
84.     }
85.  
86.     if ((poll_write = esp_transport_poll_write(ws->parent, timeout_ms)) <= 0) {
87.         ESP_LOGE(TAG, "Error transport_poll_write");
88.         return poll_write;
89.     }
90.     pBuffer[header_len++] = opcode;
91.  
92.     if (len <= 125) {
93.         pBuffer[header_len++] = (uint8_t)(len | mask_flag);
94.     } else if (len < 65536) {
95.         pBuffer[header_len++] = WS_SIZE16 | mask_flag;
96.         pBuffer[header_len++] = (uint8_t)(len >> 8);
97.         pBuffer[header_len++] = (uint8_t)(len & 0xFF);
98.     } else {
99.         pBuffer[header_len++] = WS_SIZE64 | mask_flag;
100.         /* Support maximum 4 bytes length */
101.         pBuffer[header_len++] = 0; //(uint8_t)((len >> 56) & 0xFF);
102.         pBuffer[header_len++] = 0; //(uint8_t)((len >> 48) & 0xFF);
103.         pBuffer[header_len++] = 0; //(uint8_t)((len >> 40) & 0xFF);
104.         pBuffer[header_len++] = 0; //(uint8_t)((len >> 32) & 0xFF);
105.         pBuffer[header_len++] = (uint8_t)((len >> 24) & 0xFF);
106.         pBuffer[header_len++] = (uint8_t)((len >> 16) & 0xFF);
107.         pBuffer[header_len++] = (uint8_t)((len >> 8) & 0xFF);
108.         pBuffer[header_len++] = (uint8_t)((len >> 0) & 0xFF);
109.     }
110.  
111.     if (mask_flag) {
112.         pMask = &pBuffer[header_len];
113.         getrandom(pMask, 4, 0);
114.         header_len += 4;
115.     }
116.  
117.     pData = &pBuffer[header_len];
118.     memcpy(pData, b, len);
119.  
120.     if (mask_flag) {
121.         for (i = 0; i < len; ++i) {
122.             pData[i] = (pData[i] ^ pMask[i % 4]);
123.         }
124.     }
125.  
126.     int ret = esp_transport_write(ws->parent, pBuffer, header_len+len, timeout_ms);
127.  
128.     free(pBuffer);
129.  
130.     return ret;
131. }

GeSHi © Codebox Plus Extension

[mi4863]

The web socket client example of the esp-idf 5.1.2 didn't work either, if concatenated messages with CONT+FIN are used. In rare cases, already Wireshark misses the FIN part of the message and the unmasked payload looks very strange - like being masked by another mask.

I use the web socket server from https://www.npmjs.com/package/ws. In those rare cases, the server closes the connection with an exception since some of the RSV1/2 bits in the header are 1.

Sending header and data at once solves this issue, however the new implementation has a memory leak if esp_transport_poll_write fails. Some lines have to be exchanged to:

Code: Select all

static int _ws_write(esp_transport_handle_t t, int opcode, int mask_flag, const char *b, int len, int timeout_ms)
{
    transport_ws_t *ws = esp_transport_get_context_data(t);
    int header_len = 0, i;
    char *pData, *pMask = NULL;
 
    int poll_write;
    if ((poll_write = esp_transport_poll_write(ws->parent, timeout_ms)) <= 0) {
        ESP_LOGE(TAG, "Error transport_poll_write");
        return poll_write;
    }

    char* pBuffer = malloc(len + MAX_WEBSOCKET_HEADER_SIZE);    // allocation buffer with length of data plus maximal header length
    if (pBuffer == NULL) {
        ESP_LOGE(TAG, "Websocket transport: Failed to malloc buffer to sending...");
        return 0;
    }
    pBuffer[header_len++] = opcode;
    
    ....
}

Also the memcpy isn't necessary if the data has to be masked, if the masking is done like this:

Code: Select all

    if (mask_flag) {
        for (i = 0; i < len; ++i) {
            pData[i] = (b[i] ^ pMask[i % 4]);
        }
    } else {
        memcpy(pData, b, len);
    }

I didn't understand why this fix helps, but it does. In Wireshark I see, that now the partial Websocket messages are divided in separate TCP frames and not stacked inside the same TCP frames. This costs perfomance.

Two questions:
Is it planned to include this fix to the released esp-idf?
Is it planned to save the needed the empty FIN message by adding a final flag directly to the last websocket message containing payload? This will reduce the perfomance loss due to this fix.

[Bryght-Richard]

I've also seen small WS frames split over multiple TCP or TLS segments on ESP-IDF v5.1.2, and created https://github.com/espressif/esp-idf/issues/14914 to track it.

I've also had some trouble with the ESP WS transport stability - we have an application-level PING/PONG heartbeat message, and sometimes the ESP's PONG response is delayed after it is split over multiple TLS segments, and the server tears down the connection. I wonder if we need to call esp_transport_poll_write() twice, once before each esp_transport_write()