Add another test and fix for v2 followed by v1 signature bytes

Signed-off-by: Kevin Dorosh <kevin.dorosh@solo.io>

Author: Kevin Dorosh

source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc

@@ -458,6 +458,18 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer)
     for (; search_index_ < raw_slice.len_; search_index_++) {
       if (buf[search_index_] == '\n' && buf[search_index_ - 1] == '\r') {
         if (search_index_ == 1) {
+          if (config_.get()->allowRequestsWithoutProxyProtocol()) {
+            // we need to check if what we have already could be v2 proxy protocol;
+            // if it cannot be, then we might as well forward now
+            auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE,
+                                   std::min<size_t>(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_));
+            if (!matchv2) {
+              // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can
+              // safely short-circuit
+              ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is");
+              return ReadOrParseState::SkipFilter;
+            }
+          }
           // There is not enough data to determine if it contains the v2 protocol signature, so wait
           // for more data.
           break;
@@ -469,13 +481,6 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer)
       } else if (config_.get()->allowRequestsWithoutProxyProtocol()) {
         if (search_index_ < PROXY_PROTO_V1_SIGNATURE_LEN &&
             buf[search_index_] != PROXY_PROTO_V1_SIGNATURE[search_index_]) {
-          possibly_v1_ = false;
-        }
-        if (search_index_ < PROXY_PROTO_V2_SIGNATURE_LEN &&
-            buf[search_index_] != PROXY_PROTO_V2_SIGNATURE[search_index_]) {
-          possibly_v2_ = false;
-        }
-        if (!possibly_v1_ && !possibly_v2_) {
           // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely
           // short-circuit
           ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is");

source/extensions/filters/listener/proxy_protocol/proxy_protocol.h

@@ -127,9 +127,6 @@ class Filter : public Network::ListenerFilter, Logger::Loggable<Logger::Id::filt
   // The index in buf_ where the search for '\r\n' should continue from
   size_t search_index_{1};
 
-  bool possibly_v1_{true};
-  bool possibly_v2_{true};
-
   ProxyProtocolVersion header_version_{Unknown};
 
   ConfigSharedPtr config_;

test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc

@@ -251,7 +251,7 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) {
   disconnect();
 }
 
-TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatches) {
+TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV1First) {
   // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use
   // proxy protocol v1/v2 (but it does match parts of both signatures)
   envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
@@ -272,6 +272,27 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatches) {
   disconnect();
 }
 
+TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV2First) {
+  // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use
+  // proxy protocol v1/v2 (but it does match parts of both signatures)
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(true, &proto_config);
+
+  // first two bytes proxy protocol v2, second two bytes proxy protocol v1.
+  // this ensures our byte by byte parsing (search_index_) has persistence built-in to
+  // remember whether the previous bytes were also valid for the signature
+  std::string msg = "\r\nOX";
+  EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length());
+  EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length());
+
+  write(msg);
+
+  expectData(msg);
+
+  disconnect();
+}
+
 TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) {
   // allows a large request (more bytes than v1/v2 signature) through even though it doesn't use
   // proxy protocol