esp_http_server : Bugfix in parsing of empty header values

This MR is intended to fix incorrect parsing of HTTP requests when empty header values are present.

The issue is was due to asymmetric behavior of `http_parser` library, which in case of:

    non-empty header values : invokes callbacks with the pointer to the start of a value
    empty header values : invokes callbacks with pointer to the start of next header or section

Since HTTP server relies on this pointer (along with length of the value) to locate the end of a value, and replace the line terminators (CRLFs) with null characters, the second case needed to be handled correctly.

Closes IDFGH-1539

Closes https://github.com/espressif/esp-idf/issues/3803

components/esp_http_server/src/httpd_parse.c

@@ -267,6 +267,23 @@ static esp_err_t cb_header_value(http_parser *parser, const char *at, size_t len
         parser_data->last.at     = at;
         parser_data->last.length = 0;
         parser_data->status      = PARSING_HDR_VALUE;
+
+        if (length == 0) {
+            /* As per behavior of http_parser, when length > 0,
+             * `at` points to the start of CRLF. But, in the
+             * case when header value is empty (zero length),
+             * then `at` points to the position right after
+             * the CRLF. Since for our purpose we need `last.at`
+             * to point to exactly where the CRLF starts, it
+             * needs to be adjusted by the right offset */
+            char *at_adj = (char *)parser_data->last.at;
+            /* Find the end of header field string */
+            while (*(--at_adj) != ':');
+            /* Now skip leading spaces' */
+            while (*(++at_adj) == ' ');
+            /* Now we are at the right position */
+            parser_data->last.at = at_adj;
+        }
     } else if (parser_data->status != PARSING_HDR_VALUE) {
         ESP_LOGE(TAG, LOG_FMT("unexpected state transition"));
         parser_data->error = HTTPD_500_INTERNAL_SERVER_ERROR;

examples/protocols/http_server/advanced_tests/http_server_advanced_test.py

@@ -133,6 +133,8 @@ def test_examples_protocol_http_server_advanced(env, extra_data):
         failed = True
     if not client.get_false_uri(got_ip, got_port):
         failed = True
+    if not client.get_test_headers(got_ip, got_port):
+        failed = True
 
     Utility.console_log("Error code tests...")
     if not client.code_500_server_error_test(got_ip, got_port):

examples/protocols/http_server/advanced_tests/main/tests.c

@@ -27,6 +27,96 @@ static esp_err_t hello_get_handler(httpd_req_t *req)
 #undef STR
 }
 
+/* This handler is intended to check what happens in case of empty values of headers. 
+ * Here `Header2` is an empty header and `Header1` and `Header3` will have `Value1` 
+ * and `Value3` in them. */
+static esp_err_t test_header_get_handler(httpd_req_t *req)
+{
+    httpd_resp_set_type(req, HTTPD_TYPE_TEXT);
+    int buf_len;
+    char *buf;
+
+    buf_len = httpd_req_get_hdr_value_len(req, "Header1");
+    if (buf_len > 0) {
+        buf = malloc(++buf_len);
+        if (!buf) {
+            ESP_LOGE(TAG, "Failed to allocate memory of %d bytes!", buf_len);
+            httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "Memory allocation failed");
+            return ESP_ERR_NO_MEM;
+        }
+        /* Copy null terminated value string into buffer */
+        if (httpd_req_get_hdr_value_str(req, "Header1", buf, buf_len) == ESP_OK) {
+            ESP_LOGI(TAG, "Header1 content: %s", buf);
+            if (strcmp("Value1", buf) != 0) {
+                httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Wrong value of Header1 received");
+                free(buf);
+                return ESP_ERR_INVALID_ARG;
+            } else {
+                ESP_LOGI(TAG, "Expected value and received value matched for Header1");
+            }
+        } else {
+            ESP_LOGE(TAG, "Error in getting value of Header1");
+            httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Error in getting value of Header1");
+            free(buf);
+            return ESP_FAIL;
+        }
+        free(buf);
+    } else {
+        ESP_LOGE(TAG, "Header1 not found");
+        httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Header1 not found");
+        return ESP_ERR_NOT_FOUND;
+    }
+
+    buf_len = httpd_req_get_hdr_value_len(req, "Header3");
+    if (buf_len > 0) {
+        buf = malloc(++buf_len);
+        if (!buf) {
+            ESP_LOGE(TAG, "Failed to allocate memory of %d bytes!", buf_len);
+            httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "Memory allocation failed");
+            return ESP_ERR_NO_MEM;
+        }
+        /* Copy null terminated value string into buffer */
+        if (httpd_req_get_hdr_value_str(req, "Header3", buf, buf_len) == ESP_OK) {
+            ESP_LOGI(TAG, "Header3 content: %s", buf);
+            if (strcmp("Value3", buf) != 0) {
+                httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Wrong value of Header3 received");
+                free(buf);
+                return ESP_ERR_INVALID_ARG;
+            } else {
+                ESP_LOGI(TAG, "Expected value and received value matched for Header3");
+            }
+        } else {
+            ESP_LOGE(TAG, "Error in getting value of Header3");
+            httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Error in getting value of Header3");
+            free(buf);
+            return ESP_FAIL;
+        }
+        free(buf);
+    } else {
+        ESP_LOGE(TAG, "Header3 not found");
+        httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Header3 not found");
+        return ESP_ERR_NOT_FOUND;
+    }
+
+    buf_len = httpd_req_get_hdr_value_len(req, "Header2");
+    buf = malloc(++buf_len);
+    if (!buf) {
+        ESP_LOGE(TAG, "Failed to allocate memory of %d bytes!", buf_len);
+        httpd_resp_send_err(req, HTTPD_500_INTERNAL_SERVER_ERROR, "Memory allocation failed");
+        return ESP_ERR_NO_MEM;
+    }
+    if (httpd_req_get_hdr_value_str(req, "Header2", buf, buf_len) == ESP_OK) {
+        ESP_LOGI(TAG, "Header2 content: %s", buf);
+        httpd_resp_send(req, buf, strlen(buf));
+    } else {
+        ESP_LOGE(TAG, "Header2 not found");
+        httpd_resp_send_err(req, HTTPD_400_BAD_REQUEST, "Header2 not found");
+        return ESP_FAIL;
+    }
+
+    return ESP_OK;
+}
+
 static esp_err_t hello_type_get_handler(httpd_req_t *req)
 {
 #define STR "Hello World!"
@@ -217,6 +307,11 @@ static const httpd_uri_t basic_handlers[] = {
       .handler  = hello_type_get_handler,
       .user_ctx = NULL,
     },
+    { .uri      = "/test_header",
+      .method   = HTTP_GET,
+      .handler  = test_header_get_handler,
+      .user_ctx = NULL,
+    },
     { .uri      = "/hello",
       .method   = HTTP_GET,
       .handler  = hello_get_handler,
@@ -275,6 +370,8 @@ static httpd_handle_t test_httpd_start()
     pre_start_mem = esp_get_free_heap_size();
     httpd_handle_t hd;
     httpd_config_t config = HTTPD_DEFAULT_CONFIG();
+    /* Modify this setting to match the number of test URI handlers */
+    config.max_uri_handlers  = 9;
     config.server_port = 1234;
 
     /* This check should be a part of http_server */

examples/protocols/http_server/advanced_tests/scripts/test.py

@@ -142,7 +142,20 @@ import http.client
 import sys
 import string
 import random
-import Utility
+
+
+try:
+    import Utility
+except ImportError:
+    import os
+
+    # This environment variable is expected on the host machine
+    # > export TEST_FW_PATH=~/esp/esp-idf/tools/tiny-test-fw
+    test_fw_path = os.getenv("TEST_FW_PATH")
+    if test_fw_path and test_fw_path not in sys.path:
+        sys.path.insert(0, test_fw_path)
+
+    import Utility
 
 _verbose_ = False
 
@@ -427,6 +440,34 @@ def get_echo(dut, port):
     return True
 
 
+def get_test_headers(dut, port):
+    # GET /test_header returns data of Header2'
+    Utility.console_log("[test] GET /test_header =>", end=' ')
+    conn = http.client.HTTPConnection(dut, int(port), timeout=15)
+    custom_header = {"Header1": "Value1", "Header3": "Value3"}
+    header2_values = ["", "  ", "Value2", "   Value2", "Value2  ", "  Value2  "]
+    for val in header2_values:
+        custom_header["Header2"] = val
+        conn.request("GET", "/test_header", headers=custom_header)
+        resp = conn.getresponse()
+        if not test_val("status_code", 200, resp.status):
+            conn.close()
+            return False
+        hdr_val_start_idx = val.find("Value2")
+        if hdr_val_start_idx == -1:
+            if not test_val("header: Header2", "", resp.read().decode()):
+                conn.close()
+                return False
+        else:
+            if not test_val("header: Header2", val[hdr_val_start_idx:], resp.read().decode()):
+                conn.close()
+                return False
+        resp.read()
+    Utility.console_log("Success")
+    conn.close()
+    return True
+
+
 def get_hello_type(dut, port):
     # GET /hello/type_html returns text/html as Content-Type'
     Utility.console_log("[test] GET /hello/type_html has Content-Type of text/html =>", end=' ')
@@ -966,6 +1007,7 @@ if __name__ == '__main__':
     get_hello_type(dut, port)
     get_hello_status(dut, port)
     get_false_uri(dut, port)
+    get_test_headers(dut, port)
 
     Utility.console_log("### Error code tests")
     code_500_server_error_test(dut, port)