-
Notifications
You must be signed in to change notification settings - Fork 157
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
Fixed off by one for writeBlocksRaw #590
Fixed off by one for writeBlocksRaw #590
Conversation
Is it possible to add a regression test? |
It may be possible, but I am unsure how to. This is a case of the last character in the buffer not being used because the bounds check was copy pasted from the above write* functions that have the possibility of writing two characters CRLF. |
Would anything in the current test suite break if we put |
All tests pass with |
I don't know how to test for buffer overflow unless we can add a test buffer type that checks for that. |
We might not catch this in regular builds but we can test this by inserting a debug assertion (enabled by |
@Lysxia Good idea. I have added the bounds assertion to a wrapped writeCharBuf |
f0337f2
to
cd295ec
Compare
@Bodigrim Anything else that needs to be done? |
cd295ec
to
75246e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks! |
closes #587