[incubator-kvrocks] branch unstable updated: Fix inline protocol don't allow LF only EOL (#808)

[ti...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tison pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new 2cb5190  Fix inline protocol don't allow LF only EOL (#808)
2cb5190 is described below

commit 2cb51903532e26db231f040e57c6e1e9a74edc13
Author: hulk <hu...@gmail.com>
AuthorDate: Sat Sep 3 15:46:15 2022 +0800

    Fix inline protocol don't allow LF only EOL (#808)
---
 src/redis_request.cc              | 18 ++++++++++++++----
 tests/tcl/tests/unit/protocol.tcl | 23 +++++++++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/src/redis_request.cc b/src/redis_request.cc
index 158f531..88a18c4 100644
--- a/src/redis_request.cc
+++ b/src/redis_request.cc
@@ -43,7 +43,16 @@ Status Request::Tokenize(evbuffer *input) {
   while (true) {
     switch (state_) {
       case ArrayLen: {
-        UniqueEvbufReadln line(input, EVBUFFER_EOL_CRLF_STRICT);
+        bool isOnlyLF = true;
+        // We don't use the `EVBUFFER_EOL_CRLF_STRICT` here since only LF is allowed in INLINE protocol.
+        // So we need to search LF EOL and figure out current line has CR or not.
+        UniqueEvbufReadln line(input, EVBUFFER_EOL_LF);
+        if (line && line.length > 0 && line[line.length-1] == '\r') {
+          // remove `\r` if exists
+          --line.length;
+          isOnlyLF = false;
+        }
+
         if (!line || line.length <= 0) {
           if (pipeline_size > 128) {
             LOG(INFO) << "Large pipeline detected: " << pipeline_size;
@@ -53,6 +62,7 @@ Status Request::Tokenize(evbuffer *input) {
           }
           return Status::OK();
         }
+
         pipeline_size++;
         svr_->stats_.IncrInbondBytes(line.length);
         if (line[0] == '*') {
@@ -61,13 +71,13 @@ Status Request::Tokenize(evbuffer *input) {
           } catch (std::exception &e) {
             return Status(Status::NotOK, "Protocol error: invalid multibulk length");
           }
+          if (isOnlyLF || multi_bulk_len_ > (int64_t)PROTO_MULTI_MAX_SIZE) {
+            return Status(Status::NotOK, "Protocol error: invalid multibulk length");
+          }
           if (multi_bulk_len_ <= 0) {
               multi_bulk_len_ = 0;
               continue;
           }
-          if (multi_bulk_len_ > (int64_t)PROTO_MULTI_MAX_SIZE) {
-            return Status(Status::NotOK, "Protocol error: invalid multibulk length");
-          }
           state_ = BulkLen;
         } else {
           if (line.length > PROTO_INLINE_MAX_SIZE) {
diff --git a/tests/tcl/tests/unit/protocol.tcl b/tests/tcl/tests/unit/protocol.tcl
index c6ed5cc..bb8b915 100644
--- a/tests/tcl/tests/unit/protocol.tcl
+++ b/tests/tcl/tests/unit/protocol.tcl
@@ -82,6 +82,29 @@ start_server {tags {"protocol network"}} {
         r flush
         assert_equal "OK" [r read]
     }
+
+    test "Allow only LF protocol separator" {
+        reconnect
+        r write "set foo 123\n"
+        r flush
+        assert_equal "OK" [r read]
+    }
+
+    test "Mix LF/CRLF protocol separator" {
+        reconnect
+        r write "*-1\r\nset foo 123\nget foo\r\n*3\r\n\$3\r\nset\r\n\$3\r\nkey\r\n\$3\r\nval\r\n"
+        r flush
+        assert_equal "OK" [r read]
+        assert_equal "123" [r read]
+        assert_equal "OK" [r read]
+    }
+
+    test "invalid LF in multi bulk protocol" {
+        reconnect
+        r write "*3\n\$3\r\nset\r\n\$3\r\nkey\r\n\$3\r\nval\r\n"
+        r flush
+        assert_error "*invalid multibulk length*" {r read}
+    }
 }
 
 start_server {tags {"regression"}} {