SpanNetty icon indicating copy to clipboard operation
SpanNetty copied to clipboard

Fix empty buffer handling in TlsHandler

Open maksimkim opened this issue 4 years ago • 2 comments

Problem: promise associated with writing empty buffer to TlsHandler never completes because EmptyByteBuffer doesn't write anything on .ReadBytes call to passed stream: https://github.com/cuteant/SpanNetty/blob/9f31f09ca110c6a3994966f860ee54b6a1836d75/src/DotNetty.Handlers/Tls/TlsHandler.Writer.cs#L171

Minimal repro: https://github.com/maksimkim/DotNetty/blob/2a35ea6df4b57f33997cb00702a1c712d6a17c6b/examples/HttpClient/Program.cs#L60

await channel.WriteAndFlushAsync(EmptyLastHttpContent.Default) never completes.

Solution: special case handling for empty buffer

maksimkim avatar Sep 20 '21 22:09 maksimkim

@cuteant , the change breaks existing TlsHandler tests when checking EmbeddedChannel.Finish(): https://github.com/cuteant/SpanNetty/blob/9f31f09ca110c6a3994966f860ee54b6a1836d75/test/DotNetty.Handlers.Tests/TlsHandlerTest.cs#L126

It expects channel not to have any outgoing messages but the Finish() itself causes TlsHandler to write another Unpooled.Empty to the EmbeddedChannel: https://github.com/cuteant/SpanNetty/blob/9f31f09ca110c6a3994966f860ee54b6a1836d75/src/DotNetty.Handlers/Tls/TlsHandler.Writer.cs#L105 Hence this validation always fails. What's going to be your recommendation on proper fix for the tests?

maksimkim avatar Sep 20 '21 22:09 maksimkim

@cuteant , also the regression is introduced in this PR: https://github.com/cuteant/SpanNetty/pull/65 Prior to that the promise for empty buffer write was completed in place without writing to channel: https://github.com/cuteant/SpanNetty/blob/34a24e358bf03178075cd90c7d7a319660232481/src/DotNetty.Handlers/Tls/TlsHandler.Writer.cs#L161

I wonder what is the original netty behavior in this situation?

maksimkim avatar Sep 20 '21 22:09 maksimkim