Ensure free buffer space when reading TLS messages #83480
Conversation
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsFixes #83455.
|
| // there should be space left to read into | ||
| Debug.Assert(_buffer.AvailableLength > 0, "_buffer.AvailableBytes > 0"); | ||
| // make sure we have space to read into | ||
| _buffer.EnsureAvailableSpace(Math.Min(frameSize, _buffer.Capacity) - _buffer.EncryptedLength); |
There was a problem hiding this comment.
Is there risk of growing too much if framers are keep coming. I know this is not normal case but as far as I understand the old code, we would make space for a frame, maybe more if needed.
We had EnsureAvailableSpace on line 246 so I'm wondering why that was not sufficient.
There was a problem hiding this comment.
yes, there is, I should've waited with the fix till the morning :D
We had EnsureAvailableSpace on line 246 so I'm wondering why that was not sufficient.
Not sure which one you are talking about, the closes one is in 214 and 287 and both are outside of the while loop which is receiving TLS frames
|
I assume test would be difficult? |
yes, the exact condition for this regression is that there are just enough leftover bytes at the end of the buffer that we don't know the size of the next frame (when debugging it was 3 or 4 bytes). With more bytes, the frame size would be known and the check on 720 would fix that. I am not even sure we can make a test for this, since we can't control the size of TLS frames in handshake in order to hit the condition above. |
The initial size is not enough to cover later TLS frames
|
/backport to release/7.0 |
|
Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/4445535094 |
Fixes #83455.