Bug in esp_http_client_fetch_headers

[DannyBackx]

Hey,
esp_http_client_fetch_headers has a bug. It calls http_parser_execute with incomplete headers, and when it does that, the latter will call the on_header_value functions with erroneous data.

To make this concrete :
- I'm requesting stuff from the letsencrypt.org
- on some requests, the set of headers exceeds 512 bytes
- it regularly cuts the line with the nonce in half (so my code picks up a short nonce, and further communication is not accepted)

Workaround is possible by setting a larger value than by default :
httpc.buffer_size = 1024; // HACK to ensure the nonce header doesn't get cut in two
but that's arbitrary : the same could still happen.

I propose a fix like below.
Comments ?

Danny

#if 0
// Original code calls http_parser_execute with a buffer that may end on an incompletely read line
while (client->state < HTTP_STATE_RES_COMPLETE_HEADER) {
buffer->len = esp_transport_read(client->transport, buffer->data, client->buffer_size, client->timeout_ms);
if (buffer->len <= 0) {
return ESP_FAIL;
}
http_parser_execute(client->parser, client->parser_settings, buffer->data, buffer->len);
}
#else
// Original code calls http_parser_execute with a buffer that may end on an incompletely read line
// In this version, detect incomplete header, allocate more memory, only call http_parser when that's ok.
int off = 0;
buffer->len = esp_transport_read(client->transport, buffer->data, client->buffer_size, client->timeout_ms);
int eoh;

while ((eoh = _header_end(buffer->data, buffer->len)) < 0) {
off = buffer->len;
buffer->data = realloc(buffer->data, buffer->len + 200);

int r = esp_transport_read(client->transport, buffer->data+off, 200, client->timeout_ms);
if (r < 0) {
return ESP_FAIL;
}
buffer->len += r;
}

http_parser_execute(client->parser, client->parser_settings, buffer->data, buffer->len);
#endif

to go with that, you also need :
int _header_end(const char *ptr, const int len) {
for (int i=0; i<len-3; i++)
if (ptr == 0x0D && ptr[i+1] == 0x0A && ptr[i+2] == 0x0D && ptr[i+3] == 0x0A)
return i+3;
return -1;
}

[DannyBackx]

Over three weeks and not a single reply

The same bug is present in esp-idf-4.2.1 .
Here's a diff for esp-idf-v4.2.1/components/esp_http_client/esp_http_client.c :

Code: Select all

hp: {16} diff esp_http_client.c.orig esp_http_client.c
1005a1006,1012
> int _header_end(const char *ptr, const int len) {
>   for (int i=0; i<len-3; i++)
>     if (ptr[i] == 0x0D && ptr[i+1] == 0x0A && ptr[i+2] == 0x0D && ptr[i+3] == 0x0A)
>       return i+3;
>   return -1;
> }
> 
1015a1023,1024
> #if 0
>     // Original code calls http_parser_execute with a buffer that may end on an incompletely read line
1022a1032,1051
> #else
>     // Original code calls http_parser_execute with a buffer that may end on an incompletely read line
>     // In this version, detect incomplete header, allocate more memory, only call http_parser when that's ok.
>     int off = 0;
>     buffer->len = esp_transport_read(client->transport, buffer->data, client->buffer_size_rx, client->timeout_ms);
>     int eoh;
> 
>     while ((eoh = _header_end(buffer->data, buffer->len)) < 0) {
>       off = buffer->len;
>       buffer->data = realloc(buffer->data, buffer->len + 200);
> 
>       int r = esp_transport_read(client->transport, buffer->data+off, 200, client->timeout_ms);
>       if (r < 0) {
>       return ESP_FAIL;
>       }
>       buffer->len += r;
>     }
> 
>     http_parser_execute(client->parser, client->parser_settings, buffer->data, buffer->len);
> #endif
hp: {17}

[ESP_Sprite]

Thanks for reporting this, but you may be better off opening an issue on Github, as more devs look there and we can track the status of this.

[DannyBackx]

Done, https://github.com/espressif/esp-idf/issues/7157