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;
}
Bug in esp_http_client_fetch_headers
-
- Posts: 33
- Joined: Wed Sep 19, 2018 7:17 pm
Re: Bug in esp_http_client_fetch_headers
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 :
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}
-
- Posts: 9766
- Joined: Thu Nov 26, 2015 4:08 am
Re: Bug in esp_http_client_fetch_headers
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.
-
- Posts: 33
- Joined: Wed Sep 19, 2018 7:17 pm
Who is online
Users browsing this forum: Google [Bot] and 115 guests