From 7a6ee9858fa1b10dd345002a57ddcf0e3f526191 Mon Sep 17 00:00:00 2001 From: Takuya ASADA Date: Mon, 25 Jan 2021 09:54:17 +0900 Subject: [PATCH 1/2] redis: fix large message handling If the message is larger than current buffer size, we need to consume more data until we reach to tail of the message. To do so, we need to return nullptr when it's not on the tail. Fixes #7273 --- redis/protocol_parser.rl | 5 +++++ test/redis/test_strings.py | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/redis/protocol_parser.rl b/redis/protocol_parser.rl index f1348747af..d60338e25f 100644 --- a/redis/protocol_parser.rl +++ b/redis/protocol_parser.rl @@ -120,6 +120,11 @@ public: }; %% write exec; + // does not reach to the tail of the message, continue reading + if (_size_left || _req._args_count - _req._args.size()) { + return nullptr; + } + if (_req._state != request_state::error) { return p; } diff --git a/test/redis/test_strings.py b/test/redis/test_strings.py index 2963b08bab..6abcd0c5a9 100644 --- a/test/redis/test_strings.py +++ b/test/redis/test_strings.py @@ -150,6 +150,16 @@ def test_exists_multiple_existent_key(redis_host, redis_port): assert r.get(key4) == None assert r.exists(key1, key2, key3, key4) == 3 +def test_exists_lots_of_keys(redis_host, redis_port): + r = connect(redis_host, redis_port) + keys = [] + for i in range(0, 30): + k = random_string(11) + v = random_string(10) + r.set(k, v) + keys.append(k) + assert r.exists(*keys) == len(keys) + def test_setex_ttl(redis_host, redis_port): r = connect(redis_host, redis_port) key = random_string(10) From 229940aaff1c9ae9f33de1fce87b4890b1afa7e1 Mon Sep 17 00:00:00 2001 From: Takuya ASADA Date: Mon, 25 Jan 2021 10:07:44 +0900 Subject: [PATCH 2/2] redis: rename _args_size/_size_left There are two types of numerical parameter in redis protocol: - *[0-9]+ defined array size - $[0-9]+ defined string size Currently, array size is stored to args_count, and string size is stored to _arg_size / _size_left. It's bit hard to understand since both uses same word "arg(s)", let's rename string size variables to _bytes_count / _bytes_left. --- redis/protocol_parser.rl | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/redis/protocol_parser.rl b/redis/protocol_parser.rl index d60338e25f..cfcf9551b3 100644 --- a/redis/protocol_parser.rl +++ b/redis/protocol_parser.rl @@ -41,19 +41,19 @@ action mark { action start_blob { g.mark_start(p); - _size_left = _arg_size; + _bytes_left = _bytes_count; } action start_command { g.mark_start(p); - _size_left = _arg_size; + _bytes_left = _bytes_count; } action advance_blob { - auto len = std::min(static_cast(pe - p), _size_left); - _size_left -= len; + auto len = std::min(static_cast(pe - p), _bytes_left); + _bytes_left -= len; p += len; - if (_size_left == 0) { + if (_bytes_left == 0) { _req._args.push_back(str()); p--; fret; @@ -62,10 +62,10 @@ action advance_blob { } action advance_command { - auto len = std::min(static_cast(pe - p), _size_left); - _size_left -= len; + auto len = std::min(static_cast(pe - p), _bytes_left); + _bytes_left -= len; p += len; - if (_size_left == 0) { + if (_bytes_left == 0) { _req._command = str(); p--; fret; @@ -79,7 +79,7 @@ u32 = digit+ >{ _u32 = 0;} ${ _u32 *= 10; _u32 += fc - '0';}; args_count = '*' u32 crlf ${_req._args_count = _u32 - 1;}; blob := any+ >start_blob $advance_blob; command := any+ >start_command $advance_command; -arg = '$' u32 crlf ${ _arg_size = _u32;}; +arg = '$' u32 crlf ${ _bytes_count = _u32;}; main := (args_count (arg @{fcall command; } crlf) (arg @{fcall blob; } crlf)+) ${_req._state = request_state::ok;} >eof{_req._state = request_state::eof;}; @@ -98,16 +98,16 @@ class redis_protocol_parser : public ragel_parser_base { public: redis::request _req; uint32_t _u32; - uint32_t _arg_size; - uint32_t _size_left; + uint32_t _bytes_count; + uint32_t _bytes_left; public: virtual void init() { init_base(); _req._state = request_state::error; _req._args.clear(); _req._args_count = 0; - _size_left = 0; - _arg_size = 0; + _bytes_left = 0; + _bytes_count = 0; %% write init; } @@ -121,7 +121,7 @@ public: %% write exec; // does not reach to the tail of the message, continue reading - if (_size_left || _req._args_count - _req._args.size()) { + if (_bytes_left || _req._args_count - _req._args.size()) { return nullptr; }