From b762ffa1884918d81c922322654660a88dc2bd59 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Mon, 27 May 2019 17:47:16 +0300 Subject: [PATCH] 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. --- src/lib-http/http-client-request.c | 11 ++++++- src/lib-http/http-client.h | 3 +- src/lib-http/test-http-client-request.c | 41 +++++++++++++++++-------- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/lib-http/http-client-request.c b/src/lib-http/http-client-request.c index 2795eaaa74..aa2233e385 100644 --- 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 bc678aa0e8..b246e0e3f1 100644 --- 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 0e12c5a33a..92b1811385 100644 --- 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); -- 2.47.3