Fix: Lost username when setting new URL with a path.

Closes https://github.com/espressif/esp-idf/pull/3250

diff --git a/components/esp_http_client/esp_http_client.c b/components/esp_http_client/esp_http_client.c
index 1b1f3f4824eb7dffec8ecfbfb3e69a089ff9ebdf..b2a58f725b691ed31037344f6ad6c6d81bd69239 100644 (file)
--- 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 18b68b883346ff32d069d1d84830f471252a8c51..cce15407cbe7010b442df818029c4cf01f170f6d 100644 (file)
--- 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 64c6ad3d019bca70d2ef9ca5f53e08899626a3db..495ef09aafd0023b4ffb75ada2771adf7b0d855a 100644 (file)
--- 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);
+}