diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 33552fc0e7c07..380cb3bb74a4b 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -5,6 +5,9 @@ Version history =============== * admin: added support for displaying subject alternate names in :ref:`certs` end point. * fault: removed integer percentage support. +* http: no longer adding whitespace when appending X-Forwarded-For headers. **Warning**: this is not + compatible with 1.7.0 builds prior to `9d3a4eb4ac44be9f0651fcc7f87ad98c538b01ee `_. + See `#3611 `_ for details. * router: added ability to configure arbitrary :ref:`retriable status codes. ` * router: added ability to set attempt count in upstream requests, see :ref:`virtual host's include request attempt count flag `. diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 58e2c3c05ac3f..61ce7ef594863 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 80fd2ff08b0f7..57d1fd706e5f0 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -532,7 +532,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 0e72e83261a3f..7d378857fd7fe 100644 --- a/test/common/http/utility_test.cc +++ b/test/common/http/utility_test.cc @@ -173,7 +173,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")); } {