From 4a6c5032f4a84a73e0878669201509cc2de3f68a 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] 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 faf0c20fd5..ce93eae4be 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; @@ -666,7 +686,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); } @@ -723,7 +746,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; @@ -1125,7 +1149,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 160d16fa59..a85df28c9a 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 64c6ad3d01..495ef09aaf 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); +}