From 7d28c02fd549a530fb50408df2472d3bd639d740 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20H=E1=BB=93ng=20Qu=C3=A2n?= Date: Mon, 8 Apr 2019 23:15:04 +0700 Subject: [PATCH 1/2] Fix: Lost username when setting new URL with a path. Closes https://github.com/espressif/esp-idf/pull/3250 --- components/esp_http_client/esp_http_client.c | 30 ++++++- .../esp_http_client/include/esp_http_client.h | 31 ++++++- .../esp_http_client/test/test_http_client.c | 86 ++++++++++++++++++- 3 files changed, 141 insertions(+), 6 deletions(-) diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c index 1b1f3f482..b2a58f725 100644 --- a/components/esp_http_client/esp_http_client.c +++ b/components/esp_http_client/esp_http_client.c @@ -297,6 +297,26 @@ esp_err_t esp_http_client_delete_header(esp_http_client_handle_t client, const c return http_header_delete(client->request->headers, key); } +esp_err_t esp_http_client_get_username(esp_http_client_handle_t client, char **value) +{ + if (client == NULL || value == NULL) { + ESP_LOGE(TAG, "client or value must not be NULL"); + return ESP_ERR_INVALID_ARG; + } + *value = client->connection_info.username; + return ESP_OK; +} + +esp_err_t esp_http_client_get_password(esp_http_client_handle_t client, char **value) +{ + if (client == NULL || value == NULL) { + ESP_LOGE(TAG, "client or value must not be NULL"); + return ESP_ERR_INVALID_ARG; + } + *value = client->connection_info.password; + return ESP_OK; +} + static esp_err_t _set_config(esp_http_client_handle_t client, const esp_http_client_config_t *config) { client->connection_info.method = config->method; @@ -677,7 +697,10 @@ esp_err_t esp_http_client_set_url(esp_http_client_handle_t client, const char *u } old_port = client->connection_info.port; - if (purl.field_data[UF_HOST].len) { + // Whether the passed url is absolute or is just a path + bool is_absolute_url = (bool) purl.field_data[UF_HOST].len; + + if (is_absolute_url) { http_utils_assign_string(&client->connection_info.host, url + purl.field_data[UF_HOST].off, purl.field_data[UF_HOST].len); HTTP_MEM_CHECK(TAG, client->connection_info.host, return ESP_ERR_NO_MEM); } @@ -734,7 +757,8 @@ esp_err_t esp_http_client_set_url(esp_http_client_handle_t client, const char *u } else { return ESP_ERR_NO_MEM; } - } else { + } else if (is_absolute_url) { + // Only reset authentication info if the passed URL is full free(client->connection_info.username); free(client->connection_info.password); client->connection_info.username = NULL; @@ -1136,7 +1160,7 @@ esp_err_t esp_http_client_open(esp_http_client_handle_t client, int write_len) return err; } if ((err = esp_http_client_request_send(client, write_len)) != ESP_OK) { - return err; + return err; } return ESP_OK; } diff --git a/components/esp_http_client/include/esp_http_client.h b/components/esp_http_client/include/esp_http_client.h index 18b68b883..cce15407c 100644 --- a/components/esp_http_client/include/esp_http_client.h +++ b/components/esp_http_client/include/esp_http_client.h @@ -235,13 +235,42 @@ esp_err_t esp_http_client_set_header(esp_http_client_handle_t client, const char */ esp_err_t esp_http_client_get_header(esp_http_client_handle_t client, const char *key, char **value); +/** + * @brief Get http request username. + * The address of username buffer will be assigned to value parameter. + * This function must be called after `esp_http_client_init`. + * + * @param[in] client The esp_http_client handle + * @param[out] value The username value + * + * @return + * - ESP_OK + */ +esp_err_t esp_http_client_get_username(esp_http_client_handle_t client, char **value); + +/** + * @brief Get http request password. + * The address of password buffer will be assigned to value parameter. + * This function must be called after `esp_http_client_init`. + * + * @param[in] client The esp_http_client handle + * @param[out] value The password value + * + * @return + * - ESP_OK + * - ESP_ERR_INVALID_ARG + */ +esp_err_t esp_http_client_get_password(esp_http_client_handle_t client, char **value); + /** * @brief Set http request method * * @param[in] client The esp_http_client handle * @param[in] method The method * - * @return ESP_OK + * @return + * - ESP_OK + * - ESP_ERR_INVALID_ARG */ esp_err_t esp_http_client_set_method(esp_http_client_handle_t client, esp_http_client_method_t method); diff --git a/components/esp_http_client/test/test_http_client.c b/components/esp_http_client/test/test_http_client.c index 64c6ad3d0..495ef09aa 100644 --- a/components/esp_http_client/test/test_http_client.c +++ b/components/esp_http_client/test/test_http_client.c @@ -20,7 +20,11 @@ #include "unity.h" #include "test_utils.h" -TEST_CASE("Input Param Tests", "[ESP HTTP CLIENT]") +#define HOST "httpbin.org" +#define USERNAME "user" +#define PASSWORD "challenge" + +TEST_CASE("most_common_use", "Test in common case: Only URL and hostname are specified.") { esp_http_client_config_t config_incorrect = {0}; @@ -38,10 +42,88 @@ TEST_CASE("Input Param Tests", "[ESP HTTP CLIENT]") esp_http_client_config_t config_with_hostname_path = { - .host = "httpbin.org", + .host = HOST, .path = "/get", }; client = esp_http_client_init(&config_with_hostname_path); TEST_ASSERT(client != NULL); TEST_ASSERT(esp_http_client_cleanup(client) == ESP_OK); } + +TEST_CASE("get_username_password", "Get username and password after initialization.") +{ + esp_http_client_config_t config_with_auth = { + .host = HOST, + .path = "/", + .username = USERNAME, + .password = PASSWORD + }; + char *value = NULL; + esp_http_client_handle_t client = esp_http_client_init(&config_with_auth); + TEST_ASSERT_NOT_NULL(client); + // Test with username + esp_err_t r = esp_http_client_get_username(client, &value); + TEST_ASSERT_EQUAL(ESP_OK, r); + TEST_ASSERT_NOT_NULL(value); + TEST_ASSERT_EQUAL_STRING(USERNAME, value); + // Test with password + value = NULL; + r = esp_http_client_get_password(client, &value); + TEST_ASSERT_EQUAL(ESP_OK, r); + TEST_ASSERT_NOT_NULL(value); + TEST_ASSERT_EQUAL_STRING(PASSWORD, value); + esp_http_client_cleanup(client); +} + +/** + * Test case to test that, the esp_http_client_set_url won't drop username and password + * when pass a path "/abc" for url. + **/ +TEST_CASE("username_not_lost", "Username is unmodified when we change to new path") +{ + esp_http_client_config_t config_with_auth = { + .host = HOST, + .path = "/", + .username = USERNAME, + .password = PASSWORD + }; + char *value = NULL; + esp_http_client_handle_t client = esp_http_client_init(&config_with_auth); + TEST_ASSERT_NOT_NULL(client); + esp_err_t r = esp_http_client_get_username(client, &value); + TEST_ASSERT_EQUAL(ESP_OK, r); + TEST_ASSERT_NOT_NULL(value); + TEST_ASSERT_EQUAL_STRING(USERNAME, value); + esp_http_client_set_url(client, "/something-else/"); + r = esp_http_client_get_username(client, &value); + TEST_ASSERT_EQUAL(ESP_OK, r); + TEST_ASSERT_NOT_NULL(value); + TEST_ASSERT_EQUAL_STRING(USERNAME, value); + esp_http_client_cleanup(client); +} + +/** + * Test case to test that, the esp_http_client_set_url will reset username and password + * when passing a full URL with username & password missing. + **/ +TEST_CASE("username_is_reset", "Username is reset if new absolute URL doesnot specify username.") +{ + esp_http_client_config_t config_with_auth = { + .host = HOST, + .path = "/", + .username = USERNAME, + .password = PASSWORD + }; + char *value = NULL; + esp_http_client_handle_t client = esp_http_client_init(&config_with_auth); + TEST_ASSERT_NOT_NULL(client); + esp_err_t r = esp_http_client_get_username(client, &value); + TEST_ASSERT_EQUAL(ESP_OK, r); + TEST_ASSERT_NOT_NULL(value); + TEST_ASSERT_EQUAL_STRING(USERNAME, value); + esp_http_client_set_url(client, "http://" HOST "/get"); + r = esp_http_client_get_username(client, &value); + TEST_ASSERT_EQUAL(ESP_OK, r); + TEST_ASSERT_NULL(value); + esp_http_client_cleanup(client); +} From cc3ba7186f3915df15e3f44440aef5c1e525213a Mon Sep 17 00:00:00 2001 From: Roland Dobai Date: Wed, 28 Aug 2019 16:17:28 +0530 Subject: [PATCH 2/2] esp_http_client: fix CI issues & return value --- .gitlab-ci.yml | 12 ++++++++++++ components/esp_http_client/include/esp_http_client.h | 1 + components/esp_http_client/test/test_http_client.c | 8 ++++---- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 55e8ef113..d6a78184d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1181,6 +1181,18 @@ UT_029: - UT_T2_1 - 8Mpsram +UT_030: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_1 + +UT_031: + <<: *unit_test_template + tags: + - ESP32_IDF + - UT_T1_1 + IT_001: <<: *test_template parallel: 3 diff --git a/components/esp_http_client/include/esp_http_client.h b/components/esp_http_client/include/esp_http_client.h index cce15407c..102bf9e3e 100644 --- a/components/esp_http_client/include/esp_http_client.h +++ b/components/esp_http_client/include/esp_http_client.h @@ -245,6 +245,7 @@ esp_err_t esp_http_client_get_header(esp_http_client_handle_t client, const char * * @return * - ESP_OK + * - ESP_ERR_INVALID_ARG */ esp_err_t esp_http_client_get_username(esp_http_client_handle_t client, char **value); diff --git a/components/esp_http_client/test/test_http_client.c b/components/esp_http_client/test/test_http_client.c index 495ef09aa..f85e6970f 100644 --- a/components/esp_http_client/test/test_http_client.c +++ b/components/esp_http_client/test/test_http_client.c @@ -24,7 +24,7 @@ #define USERNAME "user" #define PASSWORD "challenge" -TEST_CASE("most_common_use", "Test in common case: Only URL and hostname are specified.") +TEST_CASE("Test in common case: Only URL and hostname are specified.", "[ESP HTTP CLIENT]") { esp_http_client_config_t config_incorrect = {0}; @@ -50,7 +50,7 @@ TEST_CASE("most_common_use", "Test in common case: Only URL and hostname are spe TEST_ASSERT(esp_http_client_cleanup(client) == ESP_OK); } -TEST_CASE("get_username_password", "Get username and password after initialization.") +TEST_CASE("Get username and password after initialization.", "[ESP HTTP CLIENT]") { esp_http_client_config_t config_with_auth = { .host = HOST, @@ -79,7 +79,7 @@ TEST_CASE("get_username_password", "Get username and password after initializati * Test case to test that, the esp_http_client_set_url won't drop username and password * when pass a path "/abc" for url. **/ -TEST_CASE("username_not_lost", "Username is unmodified when we change to new path") +TEST_CASE("Username is unmodified when we change to new path", "[ESP HTTP CLIENT]") { esp_http_client_config_t config_with_auth = { .host = HOST, @@ -106,7 +106,7 @@ TEST_CASE("username_not_lost", "Username is unmodified when we change to new pat * Test case to test that, the esp_http_client_set_url will reset username and password * when passing a full URL with username & password missing. **/ -TEST_CASE("username_is_reset", "Username is reset if new absolute URL doesnot specify username.") +TEST_CASE("Username is reset if new absolute URL doesnot specify username.", "[ESP HTTP CLIENT]") { esp_http_client_config_t config_with_auth = { .host = HOST,