Ensure the commands are lower case when deciding whether to mirror th…

…e request. (envoyproxy#8530) Signed-off-by: Henry Yang <[email protected]>
KNWBIN · Oct 8, 2019 · 99fcced · 99fcced
1 parent 48c7c20
commit 99fcced
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 3 deletions.
diff --git a/source/extensions/filters/network/redis_proxy/router_impl.cc b/source/extensions/filters/network/redis_proxy/router_impl.cc
@@ -21,7 +21,10 @@ bool MirrorPolicyImpl::shouldMirror(const std::string& command) const {
     return false;
   }
 
-  if (exclude_read_commands_ && Common::Redis::SupportedCommands::isReadCommand(command)) {
+  std::string to_lower_string(command);
+  to_lower_table_.toLowerCase(to_lower_string);
+
+  if (exclude_read_commands_ && Common::Redis::SupportedCommands::isReadCommand(to_lower_string)) {
     return false;
   }
 

diff --git a/source/extensions/filters/network/redis_proxy/router_impl.h b/source/extensions/filters/network/redis_proxy/router_impl.h
@@ -41,6 +41,7 @@ class MirrorPolicyImpl : public MirrorPolicy {
   const bool exclude_read_commands_;
   ConnPool::InstanceSharedPtr upstream_;
   Runtime::Loader& runtime_;
+  const ToLowerTable to_lower_table_;
 };
 
 class Prefix : public Route {

diff --git a/test/extensions/filters/network/redis_proxy/router_impl_test.cc b/test/extensions/filters/network/redis_proxy/router_impl_test.cc
@@ -199,6 +199,8 @@ TEST(MirrorPolicyImplTest, ShouldMirrorDefault) {
 
   EXPECT_EQ(true, policy.shouldMirror("get"));
   EXPECT_EQ(true, policy.shouldMirror("set"));
+  EXPECT_EQ(true, policy.shouldMirror("GET"));
+  EXPECT_EQ(true, policy.shouldMirror("SET"));
 }
 
 TEST(MirrorPolicyImplTest, MissingUpstream) {
@@ -210,6 +212,8 @@ TEST(MirrorPolicyImplTest, MissingUpstream) {
 
   EXPECT_EQ(false, policy.shouldMirror("get"));
   EXPECT_EQ(false, policy.shouldMirror("set"));
+  EXPECT_EQ(false, policy.shouldMirror("GET"));
+  EXPECT_EQ(false, policy.shouldMirror("SET"));
 }
 
 TEST(MirrorPolicyImplTest, ExcludeReadCommands) {
@@ -223,6 +227,8 @@ TEST(MirrorPolicyImplTest, ExcludeReadCommands) {
 
   EXPECT_EQ(false, policy.shouldMirror("get"));
   EXPECT_EQ(true, policy.shouldMirror("set"));
+  EXPECT_EQ(false, policy.shouldMirror("GET"));
+  EXPECT_EQ(true, policy.shouldMirror("SET"));
 }
 
 TEST(MirrorPolicyImplTest, DefaultValueZero) {
@@ -257,18 +263,22 @@ TEST(MirrorPolicyImplTest, DeterminedByRuntimeFraction) {
   EXPECT_CALL(
       runtime.snapshot_,
       featureEnabled("runtime_key", Matcher<const envoy::type::FractionalPercent&>(Percent(50))))
-      .Times(2)
+      .Times(4)
       .WillRepeatedly(Return(true));
   EXPECT_EQ(true, policy.shouldMirror("get"));
   EXPECT_EQ(true, policy.shouldMirror("set"));
+  EXPECT_EQ(true, policy.shouldMirror("GET"));
+  EXPECT_EQ(true, policy.shouldMirror("SET"));
 
   EXPECT_CALL(
       runtime.snapshot_,
       featureEnabled("runtime_key", Matcher<const envoy::type::FractionalPercent&>(Percent(50))))
-      .Times(2)
+      .Times(4)
       .WillRepeatedly(Return(false));
   EXPECT_EQ(false, policy.shouldMirror("get"));
   EXPECT_EQ(false, policy.shouldMirror("set"));
+  EXPECT_EQ(false, policy.shouldMirror("GET"));
+  EXPECT_EQ(false, policy.shouldMirror("SET"));
 }
 
 } // namespace RedisProxy