Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions source/common/router/header_formatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "common/json/json_loader.h"
#include "common/request_info/utility.h"

#include "absl/strings/str_cat.h"
#include "absl/types/optional.h"

namespace Envoy {
Expand Down Expand Up @@ -143,9 +144,13 @@ RequestInfoHeaderFormatter::RequestInfoHeaderFormatter(absl::string_view field_n
}
field_extractor_ = [this, pattern](const Envoy::RequestInfo::RequestInfo& request_info) {
const auto& formatters = start_time_formatters_.at(pattern);
ASSERT(formatters.size() == 1);
Http::HeaderMapImpl empty_map;
return formatters.at(0)->format(empty_map, empty_map, empty_map, request_info);
std::string formatted;
for (const auto& formatter : formatters) {
absl::StrAppend(&formatted,
formatter->format(empty_map, empty_map, empty_map, request_info));
}
return formatted;
};
} else if (field_name.find("UPSTREAM_METADATA") == 0) {
field_extractor_ =
Expand Down
6 changes: 5 additions & 1 deletion test/common/router/header_formatter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,10 @@ match: { prefix: "/new_endpoint" }
key: "x-request-start"
value: "%START_TIME(%s.%3f)%"
append: true
- header:
key: "x-request-start"

@dio dio Aug 19, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's rename the key to something else, e.g. "x-request-start-multiple", and add some similar expectations below.

EXPECT_TRUE(headerMap.has("x-request-start-multiple"));
EXPECT_EQ("1522796769.123 2018-04-03T23:06:09.123Z 1522796769", headerMap.get_("x-request-start-multiple"));

value: "%START_TIME(%s.%3f)% %START_TIME% %START_TIME(%s)%"
append: true
- header:
key: "x-request-start-f"
value: "%START_TIME(f)%"
Expand All @@ -640,7 +644,7 @@ match: { prefix: "/new_endpoint" }

// Initialize start_time as 2018-04-03T23:06:09.123Z in microseconds.
const SystemTime start_time(std::chrono::microseconds(1522796769123456));
EXPECT_CALL(request_info, startTime()).Times(4).WillRepeatedly(Return(start_time));
EXPECT_CALL(request_info, startTime()).Times(7).WillRepeatedly(Return(start_time));

resp_header_parser->evaluateHeaders(headerMap, request_info);
EXPECT_TRUE(headerMap.has("x-client-ip"));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
headers_to_add {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anirudhmurali can you add a test case to the unit tests for header_formatter.cc? Best practice is to check in both corpus and a unit test for the new error handling.

header {
value: "%START_TIMEY()%T %START_TIME(f, %�{{{{{_�����������85request_i: 1�227 f55555_n555555555555%85nfo 5#555.55f, %1f, %85/5_inf %8,,,,,,,,,,,,,,,,,,,,,,,,,55555 start_timefo 5#5555#555.55f, %1f, %85/55ime: 15227 f %1f, %8555555555 %85/5555Fme: 15227 f-5555_inf 965L5559f)%"
}
}
request_info {
start_time: 1522796769123
}