Merge branch 'bugfix/pr_3250' into 'master'

Fix: Lost username when setting new URL with a path. Closes IDFGH-904 See merge request idf/esp-idf!4755
espressif · Apr 17, 2019 · 8edc995 · 8edc995
2 parents 327963e + 74dd9b4
commit 8edc995
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 6 deletions.
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
@@ -1873,6 +1873,18 @@ UT_017_05:
     - UT_T2_1
     - 8Mpsram
 
+UT_017_06:
+  <<: *unit_test_template
+  tags:
+    - ESP32_IDF
+    - UT_T1_1
+
+UT_017_07:
+  <<: *unit_test_template
+  tags:
+    - ESP32_IDF
+    - UT_T1_1
+
 UT_601_01:
   <<: *unit_test_template
   tags:

diff --git 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
@@ -235,13 +235,43 @@ 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_INVALID_ARG
+ */
+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
@@ -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("Test in common case: Only URL and hostname are specified.", "[ESP HTTP CLIENT]")
 {
     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 and password after initialization.", "[ESP HTTP CLIENT]")
+{
+    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 is unmodified when we change to new path", "[ESP HTTP CLIENT]")
+{
+    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 if new absolute URL doesnot specify username.", "[ESP HTTP CLIENT]")
+{
+    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);
+}