diff --git a/components/esp_http_client/test_apps/main/test_http_client_async.c b/components/esp_http_client/test_apps/main/test_http_client_async.c index 9abb10b61b4..f00625a28bd 100644 --- a/components/esp_http_client/test_apps/main/test_http_client_async.c +++ b/components/esp_http_client/test_apps/main/test_http_client_async.c @@ -137,7 +137,6 @@ TEST_CASE("async write would-block on header send aborts the request", "[esp_htt esp_http_client_set_post_field(client, "k=v", 3); esp_err_t err = esp_http_client_perform(client); - // characterization: master behavior, see refactor spec TEST_ASSERT(err == ESP_ERR_HTTP_EAGAIN || err == ESP_ERR_HTTP_WRITE_DATA); mock_http_transport_stats_t stats; diff --git a/components/esp_http_client/test_apps/main/test_http_client_auth.c b/components/esp_http_client/test_apps/main/test_http_client_auth.c new file mode 100644 index 00000000000..3d042506a91 --- /dev/null +++ b/components/esp_http_client/test_apps/main/test_http_client_auth.c @@ -0,0 +1,163 @@ +/* + * SPDX-FileCopyrightText: 2026 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @file test_http_client_auth.c + * @brief P0 Critical Tests: 401 Basic auth retry within one perform() + * + * This test app's sdkconfig.ci.default sets + * CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH=y (upstream Kconfig default is + * "n" - Basic auth is unencrypted). That option is required for + * esp_http_client_add_auth() to recognize a "WWW-Authenticate: Basic ..." + * header at all: with it off, the "#if CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH" + * branch that sets auth_type and process_again is compiled out entirely, and + * a 401 falls into add_auth()'s unguarded "not supported" else-branch + * instead - perform() returns ESP_ERR_NOT_SUPPORTED and the 401 is never + * retried. That compiled-out shape is a static #ifdef fact (it cannot + * silently regress) and is documented in the commit history for this file + * rather than pinned as a permanent runtime test here. Stage 0 pins the + * TRUE retry path instead, since the refactor's auth-retry handling and + * counter-split fix build on it. + * + * This file characterizes: + * - A 401 response carrying a WWW-Authenticate: Basic header is answered + * automatically with a retried request that carries an Authorization: + * Basic header, within a single esp_http_client_perform() call, when the + * URL embeds credentials, auth_type is set to HTTP_AUTH_TYPE_BASIC, and + * CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH is enabled. + * - Separately: a caller who preconfigures auth_type = HTTP_AUTH_TYPE_BASIC + * with credentials in the URL gets an Authorization: Basic header on the + * very FIRST request, before any 401 is ever seen - this path + * (esp_http_client_prepare_basic_auth(), called from + * esp_http_client_prepare() whenever auth_type == BASIC and a username is + * set) has no Kconfig guard at all, unlike the WWW-Authenticate-driven + * detection in add_auth(). Preconfigured auth and auto-detected auth are + * gated independently in master. + */ + +#include +#include "esp_http_client.h" +#include "unity.h" +#include "sdkconfig.h" +#include "test_http_client_mock_transport.h" + +/* + * Every case in this file drives the client through a mock transport injected + * via esp_http_client_config_t::transport. Without custom transport support + * the clients would fall back to a real transport aimed at test-server.local, + * which does not exist, so the whole file compiles out. + */ +#if CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT + +static const char *resp_401 = + "HTTP/1.1 401 Unauthorized\r\n" + "WWW-Authenticate: Basic realm=\"Test\"\r\n" + "Content-Length: 0\r\n" + "\r\n"; + +static const char *resp_200 = + "HTTP/1.1 200 OK\r\n" + "Content-Length: 2\r\n" + "\r\n" + "ok"; + +TEST_CASE("401 with credentials retries with Authorization header", "[esp_http_client][auth][p0]") +{ + mock_http_transport_config_t mc = MOCK_HTTP_TRANSPORT_DEFAULT_CONFIG(); + mc.response_data = resp_401; + esp_transport_handle_t mock = mock_http_transport_create(&mc); + TEST_ASSERT_NOT_NULL(mock); + mock_http_transport_queue_response(mock, resp_200, 0); + + esp_http_client_config_t cfg = { + .url = "http://user:pass@test-server.local/secure", + .auth_type = HTTP_AUTH_TYPE_BASIC, + .transport = mock, + }; + esp_http_client_handle_t client = esp_http_client_init(&cfg); + TEST_ASSERT_NOT_NULL(client); + + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_perform(client)); + TEST_ASSERT_EQUAL(200, esp_http_client_get_status_code(client)); + + /* characterization: master behavior, see refactor spec + * With CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH enabled, + * esp_http_check_response() matches status 401 and calls + * esp_http_client_add_auth() (esp_http_client.c ~L1239-1240), which + * finds a non-NULL auth_header populated from the WWW-Authenticate + * response header, recognizes the "Basic" scheme, sets process_again=1, + * and the client transparently retries within this same perform() call. + * Confirmed as a checked fact (not just inferred from the two asserts + * above): exactly 2 writes reach the transport, one per request. This + * assumes one mock_write() call per request's header block, which held + * for every GET-with-no-body case observed in this suite so far + * (single-write requests); a refactor that splits header writes across + * multiple esp_transport_write() calls would need to update this count + * without necessarily changing behavior, so treat it as a coupled-to- + * buffering assertion, not a load-bearing behavior pin by itself. */ + mock_http_transport_stats_t stats; + TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_stats(mock, &stats)); + TEST_ASSERT_EQUAL(2, stats.write_calls); + + char req[2048]; + TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_last_request(mock, req, sizeof(req), NULL)); + /* characterization: master behavior, see refactor spec + * resp_401 has Content-Length: 0 and no "Connection: close", so + * http_should_keep_alive() keeps the connection open across the retry - + * esp_http_client.c never calls esp_http_client_close() between the two + * requests, the same keep-alive shape as Task 7's redirect flow. + * Because the connection never closes, the mock's request-capture buffer + * is never reset via mock_connect(); mock_write()'s FIFO-advance-on- + * boundary logic pops resp_200 into the active buffer on the second + * write, and the capture buffer resets at that same boundary. So this + * assert observes only the SECOND (retried) request. Note this alone + * cannot distinguish "the retry added Authorization" from "it was + * already there" - see the companion test case below, which pins the + * preconfigured-auth path in isolation with no 401 involved at all. */ + TEST_ASSERT_NOT_NULL(strstr(req, "Authorization: Basic ")); + + esp_http_client_cleanup(client); + mock_http_transport_destroy(mock); +} + +TEST_CASE("preconfigured Basic auth is sent before any 401 is seen", "[esp_http_client][auth][p0]") +{ + /* Companion to the retry test above: isolates the preconfigured-auth + * path by never sending a 401 at all - only a single 200 response. */ + mock_http_transport_config_t mc = MOCK_HTTP_TRANSPORT_DEFAULT_CONFIG(); + mc.response_data = resp_200; + esp_transport_handle_t mock = mock_http_transport_create(&mc); + TEST_ASSERT_NOT_NULL(mock); + + esp_http_client_config_t cfg = { + .url = "http://user:pass@test-server.local/secure", + .auth_type = HTTP_AUTH_TYPE_BASIC, + .transport = mock, + }; + esp_http_client_handle_t client = esp_http_client_init(&cfg); + TEST_ASSERT_NOT_NULL(client); + + TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_perform(client)); + TEST_ASSERT_EQUAL(200, esp_http_client_get_status_code(client)); + + char req[2048]; + TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_last_request(mock, req, sizeof(req), NULL)); + /* characterization: master behavior, see refactor spec + * esp_http_client_prepare() (esp_http_client.c ~L800-803) calls + * esp_http_client_prepare_basic_auth() whenever + * connection_info.auth_type == HTTP_AUTH_TYPE_BASIC and a username is + * set - both true here purely from cfg.auth_type and the URL's embedded + * credentials - before the first request is ever sent, and with no + * CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH guard around that call. So + * Authorization: Basic is present on the only request this case sends, + * even though no 401/WWW-Authenticate exchange happened. */ + TEST_ASSERT_NOT_NULL(strstr(req, "Authorization: Basic ")); + + esp_http_client_cleanup(client); + mock_http_transport_destroy(mock); +} + +#endif // CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT diff --git a/components/esp_http_client/test_apps/main/test_http_client_basic.c b/components/esp_http_client/test_apps/main/test_http_client_basic.c index d5f36c05276..b3e8dc8df03 100644 --- a/components/esp_http_client/test_apps/main/test_http_client_basic.c +++ b/components/esp_http_client/test_apps/main/test_http_client_basic.c @@ -418,7 +418,29 @@ TEST_CASE("Client handles 400 Bad Request error", "[esp_http_client][basic][p0][ /** * Test: Client handles 401 Unauthorized * - * Negative scenario: Authentication required but not provided + * Negative scenario: Authentication required, no credentials configured. + * + * characterization: master behavior, see refactor spec + * This test app's sdkconfig.ci.default sets + * CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH=y, so esp_http_client_add_auth() + * (esp_http_client.c ~L2132-2198) can recognize the "Basic" scheme in this + * response's WWW-Authenticate header. That function sets + * client->process_again = 1 purely from successfully parsing that header - + * it never checks whether any credentials are actually configured. The + * credential check happens later and separately, in + * esp_http_client_prepare() (~L800-803): it only gates whether an + * Authorization header gets *attached* to the retried request, not whether + * a retry is *attempted*. So this client - no auth_type, no + * username/password, no URL-embedded credentials - still retries on a 401: + * it resends a byte-identical, credential-less request. This is the same + * shared redirect_counter / max_authorization_retries mechanism the FSM + * refactor's auth-retry handling and counter-split fix are meant to + * address. To pin that credential-less-retry behavior deterministically + * (rather than depend on how many canned responses happen to be queued), + * this test caps the retry at 1 and queues a second identical 401, so the + * client hits esp_http_client_add_auth()'s own + * "redirect_counter >= max_authorization_retries" guard (~L2140-2143) + * on the second 401 and terminates with a real, mock-independent outcome. */ TEST_CASE("Client handles 401 Unauthorized error", "[esp_http_client][basic][p0][negative]") { @@ -431,10 +453,15 @@ TEST_CASE("Client handles 401 Unauthorized error", "[esp_http_client][basic][p0] esp_transport_handle_t mock_transport = mock_http_transport_create(&mock_config); TEST_ASSERT_NOT_NULL(mock_transport); + /* Second 401 so the credential-less retry lands on the deterministic + * max_authorization_retries cap below, instead of exhausting the + * mock's queue and timing out. */ + mock_http_transport_queue_response(mock_transport, response_401_unauthorized, 0); esp_http_client_config_t config = { .url = "http://test-server.local/api/protected", .event_handler = basic_event_handler, + .max_authorization_retries = 1, .transport = mock_transport, }; @@ -442,13 +469,34 @@ TEST_CASE("Client handles 401 Unauthorized error", "[esp_http_client][basic][p0] TEST_ASSERT_NOT_NULL(client); esp_err_t err = esp_http_client_perform(client); - TEST_ASSERT_EQUAL(ESP_ERR_NOT_SUPPORTED, err); + /* characterization: master behavior, see refactor spec + * The first 401 is answered by a credential-less retry (see the file + * comment above). The second 401 then trips + * esp_http_client_add_auth()'s "redirect_counter(1) >= + * max_authorization_retries(1)" guard, which logs "reached + * max_authorization_retries" and returns ESP_FAIL directly; + * esp_http_client_perform() propagates that ESP_FAIL to the caller + * without any further retry. */ + TEST_ASSERT_EQUAL(ESP_FAIL, err); + /* characterization: master behavior, see refactor spec + * status_code is a direct field read of the last response actually + * parsed (the second 401) - untouched by the retry-cap error path. */ TEST_ASSERT_EQUAL(401, esp_http_client_get_status_code(client)); - // Verify WWW-Authenticate header could be read - // (In real scenarios, this would trigger authentication retry) + /* characterization: master behavior, see refactor spec + * Confirms the retry actually happened (2 writes: the original + * request and the one credential-less retry) rather than the client + * simply giving up on the first 401. As in test_http_client_auth.c, + * this assumes one mock_write() call per request's header block, + * which held for every GET-with-no-body case observed in this suite; + * a refactor that splits header writes across multiple + * esp_transport_write() calls would need to update this count + * without necessarily changing behavior. */ + mock_http_transport_stats_t stats; + TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_stats(mock_transport, &stats)); + TEST_ASSERT_EQUAL(2, stats.write_calls); - ESP_LOGI(TAG, "OK: 401 error handled, authentication required"); + ESP_LOGI(TAG, "OK: 401 error handled, credential-less retry capped and reported"); esp_http_client_cleanup(client); mock_http_transport_destroy(mock_transport); diff --git a/components/esp_http_client/test_apps/pytest_stage0_qemu.py b/components/esp_http_client/test_apps/pytest_stage0_qemu.py index daf6c8cdb71..c8d6a015809 100644 --- a/components/esp_http_client/test_apps/pytest_stage0_qemu.py +++ b/components/esp_http_client/test_apps/pytest_stage0_qemu.py @@ -8,4 +8,4 @@ from pytest_embedded_idf.utils import idf_parametrize @pytest.mark.qemu @idf_parametrize('target', ['esp32c3'], indirect=['target']) def test_http_client_mock(dut: Dut) -> None: - dut.run_all_single_board_cases(group=['basic', 'async', 'lifecycle', 'chunked', 'redirect'], timeout=120) + dut.run_all_single_board_cases(group=['basic', 'async', 'lifecycle', 'chunked', 'redirect', 'auth'], timeout=120) diff --git a/components/esp_http_client/test_apps/sdkconfig.ci.default b/components/esp_http_client/test_apps/sdkconfig.ci.default index d79c0487a98..9248be8bca7 100644 --- a/components/esp_http_client/test_apps/sdkconfig.ci.default +++ b/components/esp_http_client/test_apps/sdkconfig.ci.default @@ -10,3 +10,14 @@ CONFIG_ESP_TASK_WDT_EN=n # Enable custom transport for mock testing CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT=y + +# Enable HTTP Basic Authentication so the 401-retry path (auth_type +# auto-detected from WWW-Authenticate and process_again set) is compiled +# in and can be pinned by test_http_client_auth.c. Off by default upstream +# (unencrypted credentials without TLS); this is a test-only config, not a +# client-code change - the mock transport never touches the network. +CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH=y + +# Some of the test apps allocate 2048 size buffers on the stack. +# Increase the default stack size to accommodate for that +CONFIG_ESP_MAIN_TASK_STACK_SIZE=8192 diff --git a/components/esp_http_client/test_apps/sdkconfig.ci.strict_header b/components/esp_http_client/test_apps/sdkconfig.ci.strict_header index fc86f5414d9..9c7f2715bee 100644 --- a/components/esp_http_client/test_apps/sdkconfig.ci.strict_header +++ b/components/esp_http_client/test_apps/sdkconfig.ci.strict_header @@ -1,2 +1,14 @@ CONFIG_ESP_HTTP_CLIENT_STRICT_HEADER_BUFFER=y CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT=y + +# Enable HTTP Basic Authentication so the 401-retry path (auth_type +# auto-detected from WWW-Authenticate and process_again set) is compiled +# in and can be pinned by test_http_client_auth.c and the adapted 401 +# case in test_http_client_basic.c. Off by default upstream (unencrypted +# credentials without TLS); this is a test-only config, not a client-code +# change - the mock transport never touches the network. Must be kept in +# sync with sdkconfig.ci.default: pytest_esp_http_client_ut.py runs the +# full unfiltered suite against both the 'default' and 'strict_header' +# configs (see docs/en/contribute/esp-idf-tests-with-pytest.rst), so any +# sdkconfig.ci.* file that suite parametrizes over must carry this option. +CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH=y