-
Notifications
You must be signed in to change notification settings - Fork 999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Invalid bulk string terminator #4424
Comments
thank you for reporting the problem. To fix it we need to know exactly the response that causes the problem. luckily I found this helper tool in .Net client library will it be possible to record the response stream so we could reproduce the problem locally? |
Basic Information:
We've encountered an consistent error with |
We also experienced this issue last year when we upgraded to Dragonfly v1.22.2. Upgrading StackExchange.Redis version had no impact. Only solution we found at the time was to rollback to v1.21.4 which seemed to be the last version to not experience the issue. For us the error was inconsistent - we didn't see it in testing but appeared sporadically during a production rollout. Even then the error did not appear always even repeating the same application steps with the same dataset, so it has been difficult to capture. The error was most likely to appear when we used ZRANGEBYSCORE (via StackExchange.Redis SortedSetRangeByScoreAsync method) on a Sorted Set with a larger limit (200+) of JSON encoded value objects. If we are able to capture more details, I'll add them here. |
We switched to a new code that renders RESP replies in more efficient way. I am confident the bug is there. With upto 1.25.6 it is possible to workaround the issue using |
Hey @romange, I've created a reproducible public repository to address the "Invalid bulk string terminator" issue encountered in the "LRANGE" ListRangeAsync implementation of StackExchange.Redis. You can find the repo here: Invalid-bulk-string-terminator. If you need any additional information or assistance to help fix the problem, please feel free to reach out! |
@MaksTenne thanks! Currently I am stuck at causing the webapp to run correctly.
I at this point I know that the app is running because if I do not run dragonfly on 6379, it exits with an error. So far so good.
or any other url - results in error 400:
|
Instead of using URLs:
I accidentally configured the Pull the repo again and it should be changed. |
thanks, I succeeded to run both handlers and both succeed for me. |
Ah, I see - the second request fails. |
…strings Due to a corner-case bug, reply builder could add \0\0 to the end of bulk strings, instead of \r\n. The bug slipped our tests because redis-py parser most probably does not validate the ending as long as everything else is consistent. This PR: 1. Adds a test that catches the bug 2. Adds a debug check that verifies the destination pointer is consistent with the iovec being used. 3. Fixes the bug. Fixes #4424
…dings. Due to a corner-case bug, reply builder could add \0\0 to the end of bulk strings, instead of \r\n. The bug slipped our tests because redis-py parser most probably does not validate the ending as long as everything else is consistent. This PR: 1. Adds a test that catches the bug 2. Adds a debug check that verifies the destination pointer is consistent with the iovec being used. 3. Fixes the bug. Fixes #4424
Due to a corner-case bug, reply builder could add \0\0 to the end of bulk strings, instead of \r\n. The bug slipped our tests because redis-py parser most probably does not validate the ending as long as everything else is consistent. This PR: 1. Adds a test that catches the bug 2. Adds a debug check that verifies the destination pointer is consistent with the iovec being used. 3. Fixes the bug. Fixes #4424
It was a bit tricky to catch by using our python tests as as the redis-py client does not validate the endings of bulk strings. Thank you to everyone and especially @MaksTenne for helping to identify the issue 🙏🏼 |
…4441) Due to a corner-case bug, reply builder could add \0\0 to the end of bulk strings, instead of \r\n. The bug slipped our tests because redis-py parser most probably does not validate the ending as long as everything else is consistent. This PR: 1. Adds a test that catches the bug 2. Adds a debug check that verifies the destination pointer is consistent with the iovec being used. 3. Fixes the bug. Fixes #4424
Thank you for your quick response and solution! It was fun to assist you :). |
The bug is fixed in v1.26.1 |
Thanks for the support and help @romange and @MaksTenne. I was unable to provide feedback on the response stream as I was waiting for my company's development team to become available. |
Describe the bug
After upgrading our dragonfly instances from version v1.20.1 to version v1.25.5 we noticed the appearance of this exception (using the .net driver):
"StackExchange.Redis.RedisConnectionException: Invalid bulk string terminator"
We performed tests with versions v1.24.0 and v1.26.0 which also presented the same problem.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
We expect the application to run normally, as it did in version v1.20.1.
Screenshots
If applicable, add screenshots to help explain your problem.
Environment (please complete the following information):
Additional context
The text was updated successfully, but these errors were encountered: