Skip to content

Commit

Permalink
Revert "http: removing whitespace from XFF appended headers (#3587)" (#…
Browse files Browse the repository at this point in the history
…3609)

This reverts commit 4882bb5.

Signed-off-by: Daniel Hochman <danielhochman@users.noreply.github.com>
  • Loading branch information
danielhochman authored and mattklein123 committed Jun 12, 2018
1 parent 7beeea0 commit 75ca181
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 4 deletions.
1 change: 0 additions & 1 deletion docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ 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 <envoy_api_field_Listener.tcp_fast_open_queue_length>` option.
* listeners: added the ability to match :ref:`FilterChain <envoy_api_msg_listener.FilterChain>` using
:ref:`application_protocols <envoy_api_field_listener.FilterChainMatch.application_protocols>`
Expand Down
7 changes: 6 additions & 1 deletion source/common/http/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,14 @@ 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();
HeaderMapImpl::appendToHeader(header, address_as_string.c_str());
header.append(address_as_string.c_str(), address_as_string.size());
}

void Utility::appendVia(HeaderMap& headers, const std::string& via) {
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion test/common/http/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}

{
Expand Down

0 comments on commit 75ca181

Please sign in to comment.