espTlsTransportSend possible bug (CA-261)

[DimaSes99]

Here is the implementation of the espTlsTransportSend function.

int32_t espTlsTransportSend(NetworkContext_t* pxNetworkContext,
    const void* pvData, size_t uxDataLen)
{
    if (pvData == NULL || uxDataLen == 0)
    {
        return -1;
    }

    int32_t lBytesSent = 0;

    if(pxNetworkContext != NULL && pxNetworkContext->pxTls != NULL)
    {
        xSemaphoreTake(pxNetworkContext->xTlsContextSemaphore, portMAX_DELAY);
        lBytesSent = esp_tls_conn_write(pxNetworkContext->pxTls, pvData, uxDataLen);
        xSemaphoreGive(pxNetworkContext->xTlsContextSemaphore);
    }
    else
    {
        lBytesSent = -1;
    }

    return lBytesSent;
}

Sometimes the esp_tls_conn_write returns ESP_TLS_ERR_SSL_WANT_READ or ESP_TLS_ERR_SSL_WANT_WRITE. The description of the esp_tls_conn_write says that if one of this values is returned the function must be called one more time. But in the current implementation the upper laying code interprets these values as an error. Isn't it correct to handle these two values as it is done in the espTlsTransportRecv? Return 0 to initiate call one more time.

[txf-]

In the receive function this is defined:

    if (lBytesRead == ESP_TLS_ERR_SSL_WANT_WRITE  || lBytesRead == ESP_TLS_ERR_SSL_WANT_READ) {
        return 0;
    }

In core_mqtt.c the result of those flags are treated as no data in the receive case. It doesn't do any special handling for this.

static MQTTStatus_t receiveSingleIteration( MQTTContext_t * pContext,
                                            bool manageKeepAlive )
{
    MQTTStatus_t status = MQTTSuccess;
    MQTTPacketInfo_t incomingPacket = { 0 };
    int32_t recvBytes;
    size_t totalMQTTPacketLength = 0;

    assert( pContext != NULL );
    assert( pContext->networkBuffer.pBuffer != NULL );

    uint32_t looptimeStart = GetTimeMs();
    /* Read as many bytes as possible into the network buffer. */
    recvBytes = pContext->transportInterface.recv( pContext->transportInterface.pNetworkContext,
                                                   &( pContext->networkBuffer.pBuffer[ pContext->index ] ),
                                                   pContext->networkBuffer.size - pContext->index );

    if( recvBytes < 0 )
    {
        /* The receive function has failed. Bubble up the error up to the user. */
        status = MQTTRecvFailed;
    }
    else if( ( recvBytes == 0 ) && ( pContext->index == 0U ) )
    {
        /* No more bytes available since the last read and neither is anything in
         * the buffer. */
        status = MQTTNoDataAvailable;
    }

Would it even make a difference?