Rewrite HTTPClient chunked transfer-encoding #3034

Merged
earlephilhower merged 5 commits from chunkymonkey into master 2025-07-11 22:12:46 +03:00
earlephilhower commented 2025-07-11 03:45:38 +03:00 (Migrated from github.com)

Fixes #3029

The chunked decoder seems to have had some race conditions resulting in lost data.

The HTTPClient::getStreamPtr returned the raw WiFiClient which included all of the chunked block size markers, requiring the user to manually parse the chunked encoding.

Remove the existing chunked handling, add a HTTPStream as a WiFiClient subclass. The HTTPClient will return this HTTPStream which will always properly cut out chunk markers when present (and pass things raw for non-encoded streams). Use this new HTTPStream class to replace the existing handling in HTTPClient

Update the fingerprint in the StreamHTTPSClient example because their cert was updated.

Fixes #3029 The chunked decoder seems to have had some race conditions resulting in lost data. The HTTPClient::getStreamPtr returned the raw WiFiClient which included all of the chunked block size markers, requiring the user to manually parse the chunked encoding. Remove the existing chunked handling, add a HTTPStream as a WiFiClient subclass. The HTTPClient will return this HTTPStream which will always properly cut out chunk markers when present (and pass things raw for non-encoded streams). Use this new HTTPStream class to replace the existing handling in HTTPClient Update the fingerprint in the StreamHTTPSClient example because their cert was updated.
KB1RD commented 2025-07-11 20:07:48 +03:00 (Migrated from github.com)

It would be very helpful to add a isEof() function or similar to detect the end of a chunked/normal HTTP stream, if its not too much trouble. I don't see a way to tell the difference between an EOF and no data, unless I'm mistaken.

It would be very helpful to add a `isEof()` function or similar to detect the end of a chunked/normal HTTP stream, if its not too much trouble. I don't see a way to tell the difference between an EOF and no data, unless I'm mistaken.
KB1RD commented 2025-07-11 20:28:32 +03:00 (Migrated from github.com)

I'm noticing one small issue: The write function does not appear to behave in the same way as the one before the changes. My outbound websocket connection now seems unable to write any data. Will continue investigating.

EDIT: This is the code writing to the websocket:

bool WebsocketConnection::sendString(const char* str) {
  size_t len = strlen(str);
  // I'm only supporting sending short packets rn
  if (len >= 126) { return false; }

  // Write header
  client->write(0x81);
  client->write((uint8_t)len); // I'm also not masking because I'm really lazy, and I know I've configured my reverse proxy properly

  // Write body
  client->write((const uint8_t*)str, len);

  return true;
}

The 0x81 and the 0x1b gets written to the websocket (first two writes), but the larger write from a buffer never ends up getting sent.

EDIT 2: The solution is to add:

    size_t write(const uint8_t *buf, size_t size) {
        return _conn->write(buf, size);
    }

to the HTTPStream. I'm surprised the WiFi client doesn't rely on the interface single-char write method to do this, though.

I'm noticing one small issue: The write function does not appear to behave in the same way as the one before the changes. My outbound websocket connection now seems unable to write any data. Will continue investigating. EDIT: This is the code writing to the websocket: ```cpp bool WebsocketConnection::sendString(const char* str) { size_t len = strlen(str); // I'm only supporting sending short packets rn if (len >= 126) { return false; } // Write header client->write(0x81); client->write((uint8_t)len); // I'm also not masking because I'm really lazy, and I know I've configured my reverse proxy properly // Write body client->write((const uint8_t*)str, len); return true; } ``` The 0x81 and the 0x1b gets written to the websocket (first two writes), but the larger write from a buffer never ends up getting sent. EDIT 2: The solution is to add: ```cpp size_t write(const uint8_t *buf, size_t size) { return _conn->write(buf, size); } ``` to the HTTPStream. I'm surprised the WiFi client doesn't rely on the interface single-char `write` method to do this, though.
earlephilhower commented 2025-07-11 21:31:31 +03:00 (Migrated from github.com)

Thanks for the update and good catch. It's a pain this is a WiFiClient* and not a Stream * and complicates things.

At a minimum I should pass through all "reasonable" virtual calls. Let me look at what might make sense...

Thanks for the update and good catch. It's a pain this is a `WiFiClient*` and not a `Stream *` and complicates things. At a minimum I should pass through all "reasonable" virtual calls. Let me look at what might make sense...
Sign in to join this conversation.