From c640a56e575773a602baccfed1cb644ec8eff201 Mon Sep 17 00:00:00 2001
From: rajvarun77 <287367605+rajvarun77@users.noreply.github.com>
Date: Sat, 13 Jun 2026 00:23:03 -0400
Subject: [PATCH] Fix inline redis protocol consuming HTTP/2 connection preface
 (#3109)

Inline redis protocol support (#3024) accepts any alpha-leading line as
an inline command. The HTTP/2 client connection preface
("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n") is alpha-leading, so it was parsed
as an inline command instead of returning PARSE_ERROR_TRY_OTHERS. This
prevented protocol auto-detection from falling through to HTTP/2, and
gRPC clients calling a server with redis_service enabled failed with
"connection closed before server preface received".

Detect the HTTP/2 preface (fully, or as a not-yet-complete prefix) in
the inline branch of RedisCommandParser::ConsumeImpl and defer to other
protocols. A command-name blacklist is not viable because some HTTP
methods are also valid redis commands (e.g. GET), whereas the preface
is a fixed magic string that no redis command begins with, making the
check unambiguous.

Add RedisTest.inline_does_not_eat_h2_preface covering the full preface,
a partial prefix, and a genuine inline command sharing the leading byte.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/brpc/redis_command.cpp   | 17 +++++++++++++++++
 test/brpc_redis_unittest.cpp | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/src/brpc/redis_command.cpp b/src/brpc/redis_command.cpp
index 4532b3c197..cf1507b80f 100644
--- a/src/brpc/redis_command.cpp
+++ b/src/brpc/redis_command.cpp
@@ -408,6 +408,23 @@ RedisCommandConsumeState RedisCommandParser::ConsumeImpl(butil::IOBuf& buf,
             *err = PARSE_ERROR_TRY_OTHERS;
             return CONSUME_STATE_ERROR;
         }
+        // The HTTP/2 connection preface ("PRI * HTTP/2.0\r\n...") is alpha-leading
+        // and would otherwise be consumed as an inline redis command (first token
+        // "PRI"), preventing protocol auto-detection from falling through to
+        // HTTP/2 (e.g. gRPC clients). Defer to other protocols when the input
+        // matches the preface, either fully or as a not-yet-complete prefix. No
+        // valid redis command begins with these bytes, so this is unambiguous.
+        // See issue #3109.
+        static const char h2_preface[] = "PRI * HTTP/2.0\r\n";
+        const size_t h2_preface_len = sizeof(h2_preface) - 1;
+        if (*pfc == h2_preface[0]) {
+            char head[h2_preface_len];
+            const size_t n = buf.copy_to(head, h2_preface_len);
+            if (memcmp(head, h2_preface, n) == 0) {
+                *err = PARSE_ERROR_TRY_OTHERS;
+                return CONSUME_STATE_ERROR;
+            }
+        }
         const size_t buf_size = buf.size();
         const auto copy_str = static_cast<char *>(arena->allocate(buf_size + 1));
         // arena->allocate() may return NULL on allocation failure
diff --git a/test/brpc_redis_unittest.cpp b/test/brpc_redis_unittest.cpp
index 4dffd55e9f..43775d8583 100644
--- a/test/brpc_redis_unittest.cpp
+++ b/test/brpc_redis_unittest.cpp
@@ -658,6 +658,42 @@ TEST_F(RedisTest, command_parser) {
     }
 }
 
+// Regression test for issue #3109: the inline redis protocol must not consume
+// the HTTP/2 connection preface as a command, otherwise protocol auto-detection
+// never falls through to HTTP/2 and gRPC clients fail with "connection closed
+// before server preface received".
+TEST_F(RedisTest, inline_does_not_eat_h2_preface) {
+    brpc::RedisCommandParser parser;
+    butil::IOBuf buf;
+    std::vector<butil::StringPiece> command_out;
+    butil::Arena arena;
+    {
+        // Full HTTP/2 client connection preface: must defer to other protocols.
+        buf.append("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n");
+        ASSERT_EQ(brpc::PARSE_ERROR_TRY_OTHERS,
+                  parser.Consume(buf, &command_out, &arena));
+        buf.clear();
+        parser.Reset();
+    }
+    {
+        // A not-yet-complete prefix of the preface must also defer, leaving the
+        // bytes intact for the HTTP/2 parser instead of being misparsed.
+        buf.append("PRI * HT");
+        ASSERT_EQ(brpc::PARSE_ERROR_TRY_OTHERS,
+                  parser.Consume(buf, &command_out, &arena));
+        buf.clear();
+        parser.Reset();
+    }
+    {
+        // A genuine inline command sharing the leading 'P' must still parse.
+        buf.append("PING\r\n");
+        ASSERT_EQ(brpc::PARSE_OK, parser.Consume(buf, &command_out, &arena));
+        ASSERT_EQ("ping", GetCompleteCommand(command_out));
+        buf.clear();
+        parser.Reset();
+    }
+}
+
 TEST_F(RedisTest, redis_reply_codec) {
     butil::Arena arena;
     // status