From 4882bb507404e05b3c080e58d11cef6dbcce42bd Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Mon, 11 Jun 2018 13:12:51 -0400 Subject: [PATCH] http: removing whitespace from XFF appended headers (#3587) Risk Level: Low Testing: updated unit tests Docs Changes: n/a Release Notes: inline Signed-off-by: Alyssa Wilk --- docs/root/intro/version_history.rst | 1 + source/common/http/utility.cc | 7 +------ test/common/http/conn_manager_utility_test.cc | 2 +- test/common/http/utility_test.cc | 2 +- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 0b5fd99026fd..a7b815dae549 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -65,6 +65,7 @@ Version history to elide *x-forwarded-for* header modifications. * http: fixing a bug in inline headers where addCopy and addViaMove didn't add header values when encountering inline headers with multiple instances. +* http: no longer adding whitespace when appending X-Forwarded-For headers. * listeners: added :ref:`tcp_fast_open_queue_length ` option. * listeners: added the ability to match :ref:`FilterChain ` using :ref:`application_protocols ` diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 29674ae38551..889866e53452 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -30,14 +30,9 @@ void Utility::appendXff(HeaderMap& headers, const Network::Address::Instance& re return; } - // TODO(alyssawilk) move over to the append utility. HeaderString& header = headers.insertForwardedFor().value(); - if (!header.empty()) { - header.append(", ", 2); - } - const std::string& address_as_string = remote_address.ip()->addressAsString(); - header.append(address_as_string.c_str(), address_as_string.size()); + HeaderMapImpl::appendToHeader(header, address_as_string.c_str()); } void Utility::appendVia(HeaderMap& headers, const std::string& via) { diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 1629752d779e..35682c9aead1 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -497,7 +497,7 @@ TEST_F(ConnectionManagerUtilityTest, AppendInternalAddressXffNotInternalRequest) EXPECT_EQ((MutateRequestRet{"10.0.0.1:0", false}), callMutateRequestHeaders(headers, Protocol::Http2)); - EXPECT_EQ("10.0.0.2, 10.0.0.1", headers.get_("x-forwarded-for")); + EXPECT_EQ("10.0.0.2,10.0.0.1", headers.get_("x-forwarded-for")); } // A request that is from an internal address and uses remote address should be an internal request. diff --git a/test/common/http/utility_test.cc b/test/common/http/utility_test.cc index 7d583ed044f4..6572e7788990 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -67,7 +67,7 @@ TEST(HttpUtility, appendXff) { TestHeaderMapImpl headers{{"x-forwarded-for", "10.0.0.1"}}; Network::Address::Ipv4Instance address("127.0.0.1"); Utility::appendXff(headers, address); - EXPECT_EQ("10.0.0.1, 127.0.0.1", headers.get_("x-forwarded-for")); + EXPECT_EQ("10.0.0.1,127.0.0.1", headers.get_("x-forwarded-for")); } {