fix: protocol parsing errors with ws compression and pongs

[rsafonseca]

Fixes: #1783

This PR addresses a problem that happens when using WS compression and receiving PONGs from the server along with other compressed messages.

The byte slices added to websocketReader.pending are memory references to the original buffer, so when the buffer is re-written in websocketReader.drainPending([]byte) the value for the pending PONG message takes a value that is part of the re-written bytes in the buffer.

This only happens with compression, because the size of the messages in the buffer changes, and only happens when receiving PONG messages, because the value for the regular messages is a new []byte generated after decompression, and not a memory reference to a sub-slice of the original buffer

[coveralls]

coverage: 84.882% (-0.03%) from 84.911%
when pulling 66d84c6 on rsafonseca:fix_pong_bug
into f7dfee9 on nats-io:main.

[piotrpio]

Amazing find @rsafonseca, thank you. My only concern is the added allocations - maybe we could only copy the slice if it's necessary? What I mean is - if we already create a new byte slice when decompressing, this one does not have to be cloned again when appending to r.pending. As you said, we're only really concerned with frames coming from the server which are not compressed (when using compression). WDYT?

[rsafonseca]

Yeah, i considered that, but when i ran the benchmark i didn't see any noticeable difference and just wanted to keep it simple for getting the issue fixed. The memory allocation should be quickly released though as soon as drainPendingMessages runs.
Right now there's no flag in the websocketReader to assert whether compression was negotiated with the server or not, only on the websocketWriter, the reader only checks this on a per-message basis, though that could be added for the purpose of this check, or we could find another way to implement it if you feel strongly about it

[rsafonseca]

I've pushed a simple change for this, WDYT @piotrpio ?

[piotrpio]

Looks nice and simple enough, I see no reason not to avoid those allocations 🙂

LGTM, but it would be good if @wallyqs could take a look as well.

Thanks for the contribution!

[wallyqs]

LGTM

[rsafonseca]

qq, when do you expect a new tag version that includes it? :)

[piotrpio]

@rsafonseca most probably second week of February