Skip to content

Commit

Permalink
regex -> prefix
Browse files Browse the repository at this point in the history
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
alyssawilk committed Apr 24, 2018
1 parent d9dd886 commit 219472d
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 22 deletions.
9 changes: 3 additions & 6 deletions include/envoy/http/header_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include <algorithm>
#include <cstdint>
#include <memory>
#include <regex>
#include <string>

#include "envoy/common/pure.h"
Expand Down Expand Up @@ -467,12 +466,10 @@ class HeaderMap {
virtual void remove(const LowerCaseString& key) PURE;

/**
* Remove all instances of headers with key matching the supplied regex.
* @param key_matcher supplies the regex to match header keys against. Note
* that this will be matching against LowerCaseString, so should be using
* lowercase characters.
* Remove all instances of headers where the key begins with the supplied prefix.
* @param prefix supplies the prefix to match header keys against.
*/
virtual void remove(const std::regex& key_matcher) PURE;
virtual void removePrefix(const LowerCaseString& prefix) PURE;

/**
* @return the number of headers in the map.
Expand Down
10 changes: 6 additions & 4 deletions source/common/http/header_map_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "common/common/utility.h"
#include "common/singleton/const_singleton.h"

#include "absl/strings/match.h"

namespace Envoy {
namespace Http {

Expand Down Expand Up @@ -464,14 +466,14 @@ void HeaderMapImpl::remove(const LowerCaseString& key) {
}
}

void HeaderMapImpl::remove(const std::regex& regex) {
void HeaderMapImpl::removePrefix(const LowerCaseString& prefix) {

This comment has been minimized.

Copy link
@jmarantz

jmarantz Apr 24, 2018

Contributor

any particular reason to take this as a LowerCaseString as opposed to an absl::string_view lower_case_prefix?

This comment has been minimized.

Copy link
@alyssawilk

alyssawilk Apr 24, 2018

Author Contributor

I think it's more consistent with existing Envoy code to do the explicit lower casing. I'm fairly neutral but I guarantee in-house the code that calls this is X-prefixed not x-prefixed, so requiring the LowerCaseString seems a little safer.

doing absl::string_view as "the cheap way we do things going foward" and an ASSERT that it's lower would be fine by me if you prefer that.

headers_.remove_if([&](const HeaderEntryImpl& entry) {
absl::string_view key = entry.key().getStringView();
bool to_remove = std::regex_search(key.begin(), key.end(), regex);
bool to_remove = absl::StartsWith(entry.key().getStringView(), prefix.get());
if (to_remove) {
// If this header should be removed, make sure any references in the
// static lookup table are cleared as well.
StaticLookupEntry::EntryCb cb = ConstSingleton<StaticLookupTable>::get().find(key.data());
StaticLookupEntry::EntryCb cb =
ConstSingleton<StaticLookupTable>::get().find(entry.key().c_str());
if (cb) {
StaticLookupResponse ref_lookup_response = cb(*this);
if (ref_lookup_response.entry_) {
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/header_map_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class HeaderMapImpl : public HeaderMap {
void iterateReverse(ConstIterateCb cb, void* context) const override;
Lookup lookup(const LowerCaseString& key, const HeaderEntry** entry) const override;
void remove(const LowerCaseString& key) override;
void remove(const std::regex& regex) override;
void removePrefix(const LowerCaseString& key) override;
size_t size() const override { return headers_.size(); }

protected:
Expand Down
14 changes: 3 additions & 11 deletions test/common/http/header_map_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -365,32 +365,24 @@ TEST(HeaderMapImplTest, RemoveRegex) {
headers.addReference(key4, "value");
headers.addReference(key5, "value");

// Trying to remove upper case strings from LowerCaseStrings will not work.
headers.remove(std::regex("^X-Prefix-"));
EXPECT_NE(nullptr, headers.get(key1));
EXPECT_NE(nullptr, headers.get(key2));
EXPECT_NE(nullptr, headers.get(key3));
EXPECT_NE(nullptr, headers.get(key4));
EXPECT_NE(nullptr, headers.get(key5));

// Test removing the first header, middle headers, and the end header.
headers.remove(std::regex("^x-prefix-"));
headers.removePrefix(LowerCaseString("x-prefix-"));
EXPECT_EQ(nullptr, headers.get(key1));
EXPECT_NE(nullptr, headers.get(key2));
EXPECT_EQ(nullptr, headers.get(key3));
EXPECT_NE(nullptr, headers.get(key4));
EXPECT_EQ(nullptr, headers.get(key5));

// Remove all headers.
headers.remove(std::regex(".*"));
headers.removePrefix(LowerCaseString(""));
EXPECT_EQ(nullptr, headers.get(key2));
EXPECT_EQ(nullptr, headers.get(key4));

// Add inline and remove by regex
headers.insertContentLength().value(5);
EXPECT_STREQ("5", headers.ContentLength()->value().c_str());
EXPECT_EQ(1UL, headers.size());
headers.remove(std::regex("content"));
headers.removePrefix(LowerCaseString("content"));
EXPECT_EQ(nullptr, headers.ContentLength());
}

Expand Down

0 comments on commit 219472d

Please sign in to comment.