Skip to content
This repository was archived by the owner on Jan 20, 2025. It is now read-only.

AsyncEventSourceMessage::send() fails when message size > TCP_SND_BUF #888

Closed
ul-gh opened this issue Nov 18, 2020 · 3 comments
Closed

AsyncEventSourceMessage::send() fails when message size > TCP_SND_BUF #888

ul-gh opened this issue Nov 18, 2020 · 3 comments
Labels

Comments

@ul-gh
Copy link

ul-gh commented Nov 18, 2020

Hi,

In file AsyncEventSource.cpp: When the SSE message size is greater than the size reported by AsyncClient::space(), which is the LWIP TCP send buffer space (CONFIG_LWIP_TCP_SND_BUF_DEFAULT, set to 5744 bytes), the event message is not sent correctly.

In function AsyncEventSourceMessage::send(), the call to "client->add()" is made using the "_data" message buffer.
While "client->add()" has a maximum message size limit of TCP_SND_BUF size, this is only added to the "_sent" bytes data member, but that property is never evaluated. There is no offset added to the _data buffer index, nor is the _data buffer ever consumed. (Until deletion, that is).

From the code:

  const size_t len = _len - _sent;
  if(client->space() < len){
    return 0;
  }
  size_t sent = client->add((const char *)_data, len);  <====== No offset, always starts from beginning of buffer
  if(client->canSend())
    client->send();
  _sent += sent;
  return sent; 
}

This simply does not work.

The following code does, this was just now tested with SSE messages of 16 kB size:

// This could also return void as the return value is not used.
// Leaving as-is for compatibility...
size_t AsyncEventSourceMessage::send(AsyncClient *client) {
    if (_sent >= _len) {
      return 0;
    }
    const size_t len_to_send = _len - _sent;
    auto position = reinterpret_cast<const char*>(_data + _sent * sizeof(*_data));
    const size_t sent_now = client->write(position, len_to_send);
    _sent += sent_now;
    return sent_now;
}

PR follows; please let me know if there are any issues.

ul-gh pushed a commit to ul-gh/ESPAsyncWebServer that referenced this issue Nov 18, 2020
Tested with 16 kB message size: Respect number of already sent bytes
in successive calls to AsyncEventSourceMessage::send()
@ul-gh
Copy link
Author

ul-gh commented Nov 18, 2020

Please find a test bench header file containing plain C strings of 1...16 kB size for testing:
dummy_text.h

@stale
Copy link

stale bot commented Jan 18, 2021

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 18, 2021
@stale
Copy link

stale bot commented Feb 2, 2021

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.

@stale stale bot closed this as completed Feb 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant