diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c index 22b93f9a907..cf8a11924ec 100644 --- a/components/esp_http_client/esp_http_client.c +++ b/components/esp_http_client/esp_http_client.c @@ -339,6 +339,24 @@ static int http_on_header_value(http_parser *parser, const char *at, size_t leng return 0; } +/* A response never carries a body on the wire when it answers a HEAD request, + * or when its status is 204 No Content or 304 Not Modified. The server can + * still send a Content-Length header in these cases: RFC 9110, section 8.6 + * allows it on a 304, where the value describes the selected representation. + * The client must report such a response as complete, or it waits for bytes + * that never arrive. */ +static bool http_status_is_bodyless(esp_http_client_handle_t client) +{ + return client->response->status_code == HttpStatus_NoContent || + client->response->status_code == HttpStatus_NotModified; +} + +static bool http_response_is_bodyless(esp_http_client_handle_t client) +{ + return client->connection_info.method == HTTP_METHOD_HEAD || + http_status_is_bodyless(client); +} + static int http_on_headers_complete(http_parser *parser) { esp_http_client_handle_t client = parser->data; @@ -351,11 +369,11 @@ static int http_on_headers_complete(http_parser *parser) client->state = HTTP_STATE_RES_COMPLETE_HEADER; http_dispatch_event(client, HTTP_EVENT_ON_HEADERS_COMPLETE, NULL, 0); http_dispatch_event_to_event_loop(HTTP_EVENT_ON_HEADERS_COMPLETE, &client, sizeof(esp_http_client_handle_t)); - if (client->connection_info.method == HTTP_METHOD_HEAD) { + if (http_response_is_bodyless(client)) { /* In a HTTP_RESPONSE parser returning '1' from on_headers_complete will tell the parser that it should not expect a body. This is used when receiving a response - to a HEAD request which may contain 'Content-Length' or 'Transfer-Encoding: chunked' - headers that indicate the presence of a body.*/ + that may contain 'Content-Length' or 'Transfer-Encoding: chunked' headers + although no body follows.*/ return 1; } return 0; @@ -1403,7 +1421,7 @@ static int esp_http_client_get_data(esp_http_client_handle_t client) return ESP_FAIL; } - if (client->connection_info.method == HTTP_METHOD_HEAD) { + if (http_response_is_bodyless(client)) { return 0; } @@ -1430,6 +1448,13 @@ static int esp_http_client_get_data(esp_http_client_handle_t client) bool esp_http_client_is_complete_data_received(esp_http_client_handle_t client) { + /* No body follows the headers, so a declared Content-Length must not + * count as outstanding data. esp_http_client_flush_response() loops on + * this result, and esp_http_client_get_data() returns 0 here, so a false + * result would never change. */ + if (http_response_is_bodyless(client)) { + return true; + } if (client->response->is_chunked) { if (!client->is_chunk_complete) { ESP_LOGD(TAG, "Chunks were not completely read"); @@ -1465,7 +1490,9 @@ int esp_http_client_read(esp_http_client_handle_t client, char *buffer, int len) int need_read = len - ridx; bool is_data_remain = true; while (need_read > 0 && is_data_remain) { - if (client->response->is_chunked) { + if (http_response_is_bodyless(client)) { + is_data_remain = false; + } else if (client->response->is_chunked) { is_data_remain = !client->is_chunk_complete; } else { is_data_remain = client->response->data_process < client->response->content_length; @@ -1617,7 +1644,7 @@ esp_err_t esp_http_client_perform(esp_http_client_handle_t client) if (client->is_async && errno == EAGAIN) { return ESP_ERR_HTTP_EAGAIN; } - if (client->connection_info.method != HTTP_METHOD_HEAD && !client->is_chunk_complete) { + if (!http_response_is_bodyless(client) && !client->is_chunk_complete) { ESP_LOGE(TAG, "Incomplete chunked data received %d", ret); // `ret` is the raw, untranslated value from esp_http_client_get_data() @@ -1645,7 +1672,7 @@ esp_err_t esp_http_client_perform(esp_http_client_handle_t client) if (client->is_async && errno == EAGAIN) { return ESP_ERR_HTTP_EAGAIN; } - if (client->connection_info.method != HTTP_METHOD_HEAD && client->response->data_process < client->response->content_length) { + if (!http_response_is_bodyless(client) && client->response->data_process < client->response->content_length) { ESP_LOGE(TAG, "Incomlete data received, ret=%d, %"PRId64"/%"PRId64" bytes", ret, client->response->data_process, client->response->content_length); if (ret == ERR_TCP_TRANSPORT_CONNECTION_TIMEOUT) { @@ -1713,6 +1740,13 @@ int64_t esp_http_client_fetch_headers(esp_http_client_handle_t client) } client->state = HTTP_STATE_RES_ON_DATA_START; ESP_LOGD(TAG, "content_length = %"PRId64, client->response->content_length); + /* A 204 or 304 has nothing to read, whatever its Content-Length says. + * Return before the check below, which would mark it as chunked. A reply + * to HEAD keeps the declared length for every status, 304 included: + * callers use it as the resource size. */ + if (client->connection_info.method != HTTP_METHOD_HEAD && http_status_is_bodyless(client)) { + return 0; + } if (client->response->content_length <= 0) { client->response->is_chunked = true; return 0; diff --git a/components/esp_http_client/include/esp_http_client.h b/components/esp_http_client/include/esp_http_client.h index 10931796abc..62f3a7591a5 100644 --- a/components/esp_http_client/include/esp_http_client.h +++ b/components/esp_http_client/include/esp_http_client.h @@ -270,6 +270,7 @@ typedef struct { typedef enum { /* 2xx - Success */ HttpStatus_Ok = 200, + HttpStatus_NoContent = 204, HttpStatus_PartialContent = 206, /* 3xx - Redirection */ @@ -747,9 +748,14 @@ int esp_http_client_chunk_write_end(esp_http_client_handle_t client, bool last_c * * @return * - (0) if stream doesn't contain content-length header, or chunked encoding (checked by `esp_http_client_is_chunked_response`) + * - (0) if the status is 204 No Content or 304 Not Modified and the request is not HEAD, + * because no body follows. `esp_http_client_get_content_length()` still reports the + * Content-Length header value. * - (-1: ESP_FAIL) if any errors * - (-ESP_ERR_HTTP_EAGAIN = -0x7007) if call is timed-out before any data was ready - * - Download data length defined by content-length header + * - Download data length defined by content-length header. For a response to a HEAD + * request, with any status, this is the length that the server declares for the + * resource. No body follows, and `esp_http_client_read()` returns 0. */ int64_t esp_http_client_fetch_headers(esp_http_client_handle_t client); @@ -792,6 +798,12 @@ int esp_http_client_get_status_code(esp_http_client_handle_t client); * @brief Get http response content length (from header Content-Length) * the valid value if this function invoke after `esp_http_client_perform` * + * @note A response to a HEAD request, a 204 No Content and a 304 Not Modified + * carry no body, although the server can still send a Content-Length + * header. This function reports the declared value, but + * `esp_http_client_read()` returns 0 at once for such a response. Check + * the status code and the request method before you read that many bytes. + * * @param[in] client The esp_http_client handle * * @return @@ -927,6 +939,10 @@ esp_err_t esp_http_client_add_auth(esp_http_client_handle_t client); /** * @brief Checks if entire data in the response has been read without any error. * + * @note A response to a HEAD request, a 204 No Content and a 304 Not Modified + * carry no body, so this function returns true for them after the + * headers are read, even if the server sent a Content-Length header. + * * @param[in] client The esp_http_client handle * * @return diff --git a/components/esp_http_client/test_apps/main/test_http_client.c b/components/esp_http_client/test_apps/main/test_http_client.c index 8dfaf962fed..01c6c16efeb 100644 --- a/components/esp_http_client/test_apps/main/test_http_client.c +++ b/components/esp_http_client/test_apps/main/test_http_client.c @@ -803,6 +803,100 @@ TEST_CASE("HTTP client survives multiple error/success cycles", "[esp_http_clien esp_http_client_cleanup(client); mock_http_transport_destroy(mock_transport); } +/* + * Bodyless responses. A 204 No Content and a 304 Not Modified never carry a + * body on the wire, but a server can still send a Content-Length header. + * RFC 9110, section 15.4.5 allows this on a 304, where the value describes the + * selected representation. The client must not wait for that body. + */ +TEST_CASE("esp_http_client_perform() treats a 304 with Content-Length as bodyless", "[ESP HTTP CLIENT]") +{ + /* + * The mock serves headers only. A client that enters the body loop reads + * again, gets 0 bytes, and reports ESP_ERR_HTTP_READ_TIMEOUT. A client that + * treats the response as bodyless returns ESP_OK after a single read and + * keeps the connection for the next request. + */ + static const char resp_304[] = + "HTTP/1.1 304 Not Modified\r\nContent-Length: 10\r\nConnection: keep-alive\r\n\r\n"; + static const char resp_full[] = + "HTTP/1.1 200 OK\r\nContent-Length: 4\r\nConnection: keep-alive\r\n\r\nABCD"; + + mock_http_transport_config_t mock_config = MOCK_HTTP_TRANSPORT_DEFAULT_CONFIG(); + mock_config.response_data = resp_304; + mock_config.response_len = sizeof(resp_304) - 1; + esp_transport_handle_t mock = mock_http_transport_create(&mock_config); + TEST_ASSERT_NOT_NULL(mock); + TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_queue_response(mock, resp_full, sizeof(resp_full) - 1)); + + esp_http_client_config_t config = { .url = "http://example.com/", .transport = mock }; + esp_http_client_handle_t client = esp_http_client_init(&config); + TEST_ASSERT_NOT_NULL(client); + + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_perform(client)); + TEST_ASSERT_EQUAL(304, esp_http_client_get_status_code(client)); + /* The header value stays visible to the caller. */ + TEST_ASSERT_EQUAL(10, esp_http_client_get_content_length(client)); + + /* No body was expected, so the connection is kept and reused. */ + TEST_ASSERT_EQUAL(HTTP_STATE_CONNECTED, esp_http_client_get_state(client)); + + mock_http_transport_stats_t stats; + TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_stats(mock, &stats)); + TEST_ASSERT_EQUAL(0, stats.close_calls); + /* One read for the headers, and no second read for a body. */ + TEST_ASSERT_EQUAL(1, stats.read_calls); + + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_perform(client)); + TEST_ASSERT_EQUAL(200, esp_http_client_get_status_code(client)); + + TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_stats(mock, &stats)); + TEST_ASSERT_EQUAL(1, stats.connect_calls); + + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_cleanup(client)); + mock_http_transport_destroy(mock); +} + +TEST_CASE("esp_http_client_read() returns 0 for a 204 with Content-Length", "[ESP HTTP CLIENT]") +{ + /* + * The streaming path must agree with perform(). A read on a 204 No Content + * returns 0 at once instead of a wait on the socket. + */ + static const char resp_204[] = + "HTTP/1.1 204 No Content\r\nContent-Length: 7\r\nConnection: keep-alive\r\n\r\n"; + + mock_http_transport_config_t mock_config = MOCK_HTTP_TRANSPORT_DEFAULT_CONFIG(); + mock_config.response_data = resp_204; + mock_config.response_len = sizeof(resp_204) - 1; + esp_transport_handle_t mock = mock_http_transport_create(&mock_config); + TEST_ASSERT_NOT_NULL(mock); + + esp_http_client_config_t config = { .url = "http://example.com/", .transport = mock }; + esp_http_client_handle_t client = esp_http_client_init(&config); + TEST_ASSERT_NOT_NULL(client); + + char buf[16]; + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_open(client, 0)); + /* No bytes to read, but the declared length stays available. */ + TEST_ASSERT_EQUAL(0, esp_http_client_fetch_headers(client)); + TEST_ASSERT_EQUAL(204, esp_http_client_get_status_code(client)); + TEST_ASSERT_EQUAL(7, esp_http_client_get_content_length(client)); + TEST_ASSERT_FALSE(esp_http_client_is_chunked_response(client)); + + TEST_ASSERT_EQUAL(0, esp_http_client_read(client, buf, sizeof(buf))); + /* A second read must also return 0 and must not touch the socket. */ + mock_http_transport_stats_t stats; + TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_stats(mock, &stats)); + int reads_after_first = stats.read_calls; + TEST_ASSERT_EQUAL(0, esp_http_client_read(client, buf, sizeof(buf))); + TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_stats(mock, &stats)); + TEST_ASSERT_EQUAL(reads_after_first, stats.read_calls); + + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_cleanup(client)); + mock_http_transport_destroy(mock); +} + #endif // CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT void app_main(void) diff --git a/examples/protocols/esp_http_client/main/esp_http_client_example.c b/examples/protocols/esp_http_client/main/esp_http_client_example.c index d76f1e56e59..591937f62fe 100644 --- a/examples/protocols/esp_http_client/main/esp_http_client_example.c +++ b/examples/protocols/esp_http_client/main/esp_http_client_example.c @@ -653,7 +653,11 @@ static void http_perform_as_stream_reader(void) if (read_len <= 0) { ESP_LOGE(TAG, "Error read data"); } - buffer[read_len] = 0; + /* A negative read_len is an error code, not a length: using it as an + * index would write before the start of the buffer. */ + if (read_len >= 0) { + buffer[read_len] = 0; + } ESP_LOGD(TAG, "read_len = %d", read_len); } ESP_LOGI(TAG, "HTTP Stream reader Status = %d, content_length = %"PRId64,