lib-http: http_client_request_add_header() - Replace existing header

If header with the same key already exists, just replace the value.
HTTP supports having multiple headers with the same key only when they
can be rewritten into a single comma-separated header. So practically
there's no reason for lib-http to need to support adding multiple
headers. Replacing an existing value is more useful generally.

diff --git a/src/lib-http/http-client-request.c b/src/lib-http/http-client-request.c
index 2795eaaa748b186887f29b3b35cd2acefed5f775..aa2233e3851f73ab1e1a14a64c82c149953e4d73 100644 (file)
--- a/src/lib-http/http-client-request.c
+++ b/src/lib-http/http-client-request.c
@@ -420,6 +420,8 @@ http_client_request_lookup_header_pos(struct http_client_request *req,
 void http_client_request_add_header(struct http_client_request *req,
                                    const char *key, const char *value)
 {
+       size_t key_pos, value_pos, next_pos;
+
        i_assert(req->state == HTTP_REQUEST_STATE_NEW ||
                 /* allow calling for retries */
                 req->state == HTTP_REQUEST_STATE_GOT_RESPONSE ||
@@ -466,7 +468,14 @@ void http_client_request_add_header(struct http_client_request *req,
        }
        if (req->headers == NULL)
                req->headers = str_new(default_pool, 256);
-       str_printfa(req->headers, "%s: %s\r\n", key, value);
+       if (!http_client_request_lookup_header_pos(req, key, &key_pos,
+                                                  &value_pos, &next_pos))
+               str_printfa(req->headers, "%s: %s\r\n", key, value);
+       else {
+               /* don't delete CRLF */
+               size_t old_value_len = next_pos - value_pos - 2;
+               str_replace(req->headers, value_pos, old_value_len, value);
+       }
 }
 
 void http_client_request_remove_header(struct http_client_request *req,

diff --git a/src/lib-http/http-client.h b/src/lib-http/http-client.h
index bc678aa0e85457361dfa0ac7698afeee4be27d2b..b246e0e3f18a74ddf7b4f1aa279ea178fa0ad500 100644 (file)
--- a/src/lib-http/http-client.h
+++ b/src/lib-http/http-client.h
@@ -293,7 +293,8 @@ void http_client_request_set_urgent(struct http_client_request *req);
 void http_client_request_set_preserve_exact_reason(struct http_client_request *req);
 
 /* add a custom header to the request. This can override headers that are
-   otherwise created implicitly. */
+   otherwise created implicitly. If the same header key was already added,
+   the value is replaced. */
 void http_client_request_add_header(struct http_client_request *req,
                                    const char *key, const char *value);
 /* remove a header added earlier. This has no influence on implicitly created

diff --git a/src/lib-http/test-http-client-request.c b/src/lib-http/test-http-client-request.c
index 0e12c5a33a72f03939ab7980c3a7ff2f52c50070..92b1811385f20f178561d24477fba8a07569882c 100644 (file)
--- a/src/lib-http/test-http-client-request.c
+++ b/src/lib-http/test-http-client-request.c
@@ -23,24 +23,39 @@ static void test_http_client_request_headers(void)
        req = http_client_request(client, "GET", "host", "target",
                                  test_http_client_request_callback, NULL);
 
-       /* add */
-       http_client_request_add_header(req, "foo1", "value1");
-       test_assert_strcmp(str_c(req->headers), "foo1: value1\r\n");
+       /* add the first */
+       http_client_request_add_header(req, "qwe", "value1");
+       test_assert_strcmp(str_c(req->headers), "qwe: value1\r\n");
 
-       http_client_request_add_header(req, "foo1", "value2");
-       test_assert_strcmp(str_c(req->headers), "foo1: value1\r\nfoo1: value2\r\n");
+       /* replace the first with the same length */
+       http_client_request_add_header(req, "qwe", "234567");
+       test_assert_strcmp(str_c(req->headers), "qwe: 234567\r\n");
 
-       http_client_request_add_header(req, "foo2", "value3");
-       test_assert_strcmp(str_c(req->headers), "foo1: value1\r\nfoo1: value2\r\nfoo2: value3\r\n");
+       /* replace the first with smaller length */
+       http_client_request_add_header(req, "qwe", "xyz");
+       test_assert_strcmp(str_c(req->headers), "qwe: xyz\r\n");
 
-       /* remove */
-       http_client_request_remove_header(req, "foo1");
-       test_assert_strcmp(str_c(req->headers), "foo1: value2\r\nfoo2: value3\r\n");
+       /* replace the first with longer length */
+       http_client_request_add_header(req, "qwe", "abcdefg");
+       test_assert_strcmp(str_c(req->headers), "qwe: abcdefg\r\n");
 
-       http_client_request_remove_header(req, "foo2");
-       test_assert_strcmp(str_c(req->headers), "foo1: value2\r\n");
+       /* add the second */
+       http_client_request_add_header(req, "xyz", "1234");
+       test_assert_strcmp(str_c(req->headers), "qwe: abcdefg\r\nxyz: 1234\r\n");
 
-       http_client_request_remove_header(req, "foo1");
+       /* replace second */
+       http_client_request_add_header(req, "xyz", "yuiop");
+       test_assert_strcmp(str_c(req->headers), "qwe: abcdefg\r\nxyz: yuiop\r\n");
+
+       /* replace the first again */
+       http_client_request_add_header(req, "qwe", "1234");
+       test_assert_strcmp(str_c(req->headers), "qwe: 1234\r\nxyz: yuiop\r\n");
+
+       /* remove the headers */
+       http_client_request_remove_header(req, "qwe");
+       test_assert_strcmp(str_c(req->headers), "xyz: yuiop\r\n");
+
+       http_client_request_remove_header(req, "xyz");
        test_assert_strcmp(str_c(req->headers), "");
 
        http_client_request_abort(&req);