Discard trailing data after a deflated message #8610
Conversation
We can recover from this and it seems like the least worst of our options. Closes: #8551
| if (inflater.bytesRead < totalBytesToRead) { | ||
| deflatedBytes.clear() | ||
| inflaterSource.close() | ||
| this.inflaterSource = null |
There was a problem hiding this comment.
I’m gonna claim it’s really weird if you have a stream-terminating message followed by another message and noContextTakeover = false. But perhaps I should write a test for this? I’m not exactly sure what reasonable behavior is.
There was a problem hiding this comment.
Also - I think the server in the case we’re addressing here is probably buggy. We might be better to follow @Fischiii’s lead and just reject the bogus data.
There was a problem hiding this comment.
I think that was my change to make his PR pass
There was a problem hiding this comment.
@swankjesse @yschimke in case it helps. The server used is a Kestrel (.net version 7) with the WebsocketSharp.Standard2 Framework for the WebwSocket connection. Data transferred is binary form Protobuffer.
| @@ -136,6 +139,41 @@ internal class MessageDeflaterInflaterTest { | |||
| assertThat(buffer.readUtf8()).isEqualTo("Hello inflation!") | |||
| } | |||
|
|
|||
| /** | |||
| * It's possible a self-terminating deflated message will contain trailing data that won't be | |||
There was a problem hiding this comment.
Is it worth tying this back to the padding bytes in the spec?
We can recover from this and it seems like the least worst of our options.
Closes: #8551