This is an automated email from the ASF dual-hosted git repository.
chenBright pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new 68d18eec Fix inline redis protocol consuming HTTP/2 connection preface
(#3338)
68d18eec is described below
commit 68d18eecf7878018a171ef660ffdba25b683c01a
Author: rajvarun77 <[email protected]>
AuthorDate: Sun Jun 14 00:32:43 2026 -0400
Fix inline redis protocol consuming HTTP/2 connection preface (#3338)
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: rajvarun77 <[email protected]>
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
---
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 4532b3c1..cf1507b8 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 4dffd55e..43775d85 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
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
