Fix inline protocol don't allow LF only EOL (#808)

apache · Sep 3, 2022 · 2cb5190 · 2cb5190
1 parent b3506ad
commit 2cb5190
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 4 deletions.
diff --git 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
@@ -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"}} {