From b0132a11f37d14467f3e0e56d31b1265a231bd99 Mon Sep 17 00:00:00 2001 From: Yuchen Dai Date: Tue, 28 Jul 2020 21:50:18 +0000 Subject: [PATCH] More rtrim fix Signed-off-by: Yuchen Dai --- source/common/http/header_map_impl.cc | 21 +++++---------------- test/common/http/header_map_impl_test.cc | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index 4c5b11422203..45ee954bb76d 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -151,22 +151,11 @@ void HeaderString::append(const char* data, uint32_t size) { } void HeaderString::rtrim() { - switch (type_) { - case Type::Reference: { - RELEASE_ASSERT("false", "rtrim does not support header string in reference type."); - } - case Type::Dynamic: { - // rtrim only manipulate length_. Share the same code with Inline. - FALLTHRU; - } - case Type::Inline: { - absl::string_view original = getStringView(); - absl::string_view rtrimmed = StringUtil::rtrim(original); - if (original.size() != rtrimmed.size()) { - string_length_ = rtrimmed.size(); - } - break; - } + ASSERT(type() == Type::Inline || type() == Type::Dynamic); + absl::string_view original = getStringView(); + absl::string_view rtrimmed = StringUtil::rtrim(original); + if (original.size() != rtrimmed.size()) { + string_length_ = rtrimmed.size(); } } diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc index 9c52b8fc0a1c..9a1073b90a71 100644 --- a/test/common/http/header_map_impl_test.cc +++ b/test/common/http/header_map_impl_test.cc @@ -107,17 +107,35 @@ TEST(HeaderStringTest, All) { // Inline rtrim removes trailing whitespace only. { + // This header string is short enough to fit into Inline type. const std::string data_with_leading_lws = " \t\f\v data"; const std::string data_with_leading_and_trailing_lws = data_with_leading_lws + " \t\f\v"; HeaderString string; string.append(data_with_leading_and_trailing_lws.data(), data_with_leading_and_trailing_lws.size()); EXPECT_EQ(data_with_leading_and_trailing_lws, string.getStringView()); + EXPECT_EQ(string.type(), HeaderString::Type::Inline); string.rtrim(); EXPECT_NE(data_with_leading_and_trailing_lws, string.getStringView()); EXPECT_EQ(data_with_leading_lws, string.getStringView()); } + // Dynamic rtrim removes trailing whitespace only. + { + // Making this string longer than Inline can fit. + const std::string padding_data_with_leading_lws = " \t\f\v data" + std::string(128, 'a'); + const std::string data_with_leading_and_trailing_lws = + padding_data_with_leading_lws + " \t\f\v"; + HeaderString string; + string.append(data_with_leading_and_trailing_lws.data(), + data_with_leading_and_trailing_lws.size()); + EXPECT_EQ(data_with_leading_and_trailing_lws, string.getStringView()); + EXPECT_EQ(string.type(), HeaderString::Type::Dynamic); + string.rtrim(); + EXPECT_NE(data_with_leading_and_trailing_lws, string.getStringView()); + EXPECT_EQ(padding_data_with_leading_lws, string.getStringView()); + } + // Static clear() does nothing. { std::string static_string("HELLO");