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: Untitled.c Select all

//////////////////////////////////////////////

// current SDK code

//////////////////////////////////////////////



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);

    char *buffer = (char *)b;

    char ws_header[MAX_WEBSOCKET_HEADER_SIZE];

    char *mask;

    int header_len = 0, i;



    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;

    }

    ws_header[header_len++] = opcode;



    if (len <= 125) {

        ws_header[header_len++] = (uint8_t)(len | mask_flag);

    } else if (len < 65536) {

        ws_header[header_len++] = WS_SIZE16 | mask_flag;

        ws_header[header_len++] = (uint8_t)(len >> 8);

        ws_header[header_len++] = (uint8_t)(len & 0xFF);

    } else {

        ws_header[header_len++] = WS_SIZE64 | mask_flag;

        /* Support maximum 4 bytes length */

        ws_header[header_len++] = 0; //(uint8_t)((len >> 56) & 0xFF);

        ws_header[header_len++] = 0; //(uint8_t)((len >> 48) & 0xFF);

        ws_header[header_len++] = 0; //(uint8_t)((len >> 40) & 0xFF);

        ws_header[header_len++] = 0; //(uint8_t)((len >> 32) & 0xFF);

        ws_header[header_len++] = (uint8_t)((len >> 24) & 0xFF);

        ws_header[header_len++] = (uint8_t)((len >> 16) & 0xFF);

        ws_header[header_len++] = (uint8_t)((len >> 8) & 0xFF);

        ws_header[header_len++] = (uint8_t)((len >> 0) & 0xFF);

    }



    if (mask_flag) {

        mask = &ws_header[header_len];

        getrandom(ws_header + header_len, 4, 0);

        header_len += 4;



        for (i = 0; i < len; ++i) {

            buffer[i] = (buffer[i] ^ mask[i % 4]);

        }

    }



    if (esp_transport_write(ws->parent, ws_header, header_len, timeout_ms) != header_len) {

        ESP_LOGE(TAG, "Error write header");

        return -1;

    }

    if (len == 0) {

        return 0;

    }



    int ret = esp_transport_write(ws->parent, buffer, len, timeout_ms);

    // in case of masked transport we have to revert back to the original data, as ws layer

    // does not create its own copy of data to be sent

    if (mask_flag) {

        mask = &ws_header[header_len-4];

        for (i = 0; i < len; ++i) {

            buffer[i] = (buffer[i] ^ mask[i % 4]);

        }

    }

    return ret;

}



//////////////////////////////////////////////

// changed code

//////////////////////////////////////////////



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;

    int poll_write;

    char *pData, *pMask = NULL;



    char* pBuffer = malloc(len+10); // 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;

    }



    if ((poll_write = esp_transport_poll_write(ws->parent, timeout_ms)) <= 0) {

        ESP_LOGE(TAG, "Error transport_poll_write");

        return poll_write;

    }

    pBuffer[header_len++] = opcode;



    if (len <= 125) {

        pBuffer[header_len++] = (uint8_t)(len | mask_flag);

    } else if (len < 65536) {

        pBuffer[header_len++] = WS_SIZE16 | mask_flag;

        pBuffer[header_len++] = (uint8_t)(len >> 8);

        pBuffer[header_len++] = (uint8_t)(len & 0xFF);

    } else {

        pBuffer[header_len++] = WS_SIZE64 | mask_flag;

        /* Support maximum 4 bytes length */

        pBuffer[header_len++] = 0; //(uint8_t)((len >> 56) & 0xFF);

        pBuffer[header_len++] = 0; //(uint8_t)((len >> 48) & 0xFF);

        pBuffer[header_len++] = 0; //(uint8_t)((len >> 40) & 0xFF);

        pBuffer[header_len++] = 0; //(uint8_t)((len >> 32) & 0xFF);

        pBuffer[header_len++] = (uint8_t)((len >> 24) & 0xFF);

        pBuffer[header_len++] = (uint8_t)((len >> 16) & 0xFF);

        pBuffer[header_len++] = (uint8_t)((len >> 8) & 0xFF);

        pBuffer[header_len++] = (uint8_t)((len >> 0) & 0xFF);

    }



    if (mask_flag) {

        pMask = &pBuffer[header_len];

        getrandom(pMask, 4, 0);

        header_len += 4;

    }



    pData = &pBuffer[header_len];

    memcpy(pData, b, len);



    if (mask_flag) {

        for (i = 0; i < len; ++i) {

            pData[i] = (pData[i] ^ pMask[i % 4]);

        }

    }



    int ret = esp_transport_write(ws->parent, pBuffer, header_len+len, timeout_ms);



    free(pBuffer);



    return ret;

}

[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()

[Bryght-Richard]

After integrating your fix, it helps a lot, but rarely when sending multiple WS frames close together in time, one will still get stuck in MbedTLS and doesn't reach the TCP send buffer for some times(~10 seconds). Are you using MbedTLS, or WolfSSL, or plaintext(no SSL/TLS)?

Cross-linking related issues:
https://github.com/espressif/esp-idf/issues/14918
https://forums.mbed.com/t/how-can-i-flu ... ayer/24548