Add protection against concurrent Readers

[oslook]

During my use of the library(server and client both v1.8.7), the server occasionally receives the following prompt,

failed to get reader: received header with unexpected rsv bits set: false:true:true

which seems to be related to this part of the code.

write.go L300:

	c.writeHeader.rsv1 = false
	if flate && (opcode == opText || opcode == opBinary) {
		c.writeHeader.rsv1 = true
	}

Could you please confirm if it is necessary to add the following code here?

	c.writeHeader.rsv1 = false
	c.writeHeader.rsv2 = false
	c.writeHeader.rsv3 = false

[vendion]

I'm getting something slightly similar using the same version: failed to get reader: received header with unexpected rsv bits set: true:true:true

[soypat]

Me too- I've checked with wireshark and the websocket frame is OK, it's just being parsed wrong on nhooyr/websocket's side.

[nhooyr]

That's quite concerning that all three of you guys are seeing that... I'd suggest disabling compression though it'll be disabled by default in the next release.

I'll go through the code again to see if I can find the bug. The autobahn compression tests pass fine under the race detector so I'm surprised to see a bug like this.

[nhooyr]

@soypat If possible, can you share the frames you checked with wireshark? Maybe I can replay them with the library and thus reproduce. That would help immensely in debugging.

[nhooyr]

There's been enough changes for v1.8.8 that whatever this was is likely fixed. Compression in particular is disabled by default now and I recommend keeping it that way. Let me know if you see it again once v1.8.8 is released.

[nhooyr]

Nvm I was able to reproduce, see 249edb2

[nhooyr]

Were you guys using the library through a proxy?

[nhooyr]

Nvm, that bug was from my proxy not working correctly. Fixed now.

[nhooyr]

Actually nvm, this is probably caused by #355 so I'll close after fixing that.

[nhooyr]

Done so closing.

[maggie44]

I still see this error on occasions. Seen it today on the latest release: failed to get reader: received header with unexpected rsv bits set: true:true:false"

Whenever I see it, it so far has always been the result of me having two readers open at the same time. It states in the library that Only one Reader may be open at a time, but sometimes an error in my code where I have two readers causes the issue.

Not sure if the error message could be clearer for diagnosis. Adding this quick comment here in case anyone comes across this issue and finds this thread.

[nhooyr]

Ah yea that would do it. I'll add a link to this thread in that error message.

Could also add some protection here perhaps without much performance cost.

[maggie44]

I wonder if there is also a way to have two readers, where both (or multiple) receive the same message. This would be helpful for me in a number of scenarios. Or maybe when initiating one reader it closes the other. I am already passing the ws connection around, but ideally wouldn't have to pass around the reader, and could just close one and open it up elsewhere.

[nhooyr]

There shouldn't be any scenario where you want to do a concurrent read on the connection. There should only ever be one goroutine reading from the connection. Can you clarify why you feel you need multiple readers or even why you're passing the reader around like that? Perhaps an example in code?

[maggie44]

On the multiple reader, makes sense. Closing and reopening or passing around, I read in a loop but some messages call a series of functions and those functions need to take over the connection and receive all future messages after their call. For those functions I need to either stop the loop and move the reader down or relay all messages through a channel on through another connection.

Hard to fully explain, maybe at some point it will hit me how best to do it. Didn’t want to hijack the thread though, will think on it.

[GoldenSheep402]

The problem still appear in my project?

[jazzcake-diverse]

Hello, I also have similar problem in my project. in my log, "readPacket() s.conn.ReadMessage() err: failed to get reader: received header with unexpected rsv bits set: true:false:true"

It could be helpful if I leave code snippet?

	default:
		ctx := context.Background()

		msgType, r, err := s.Conn.Reader(ctx)
		if err != nil {
			log.Warnf("session[ %d ] readPacket() s.conn.ReadMessage() msgType: %d, err: %s", s.sid, msgType, err.Error())
			s.packetChan <- Packet{S: s, Message: serializer.MakeDisconnected()}
			return
		}

		b, err := io.ReadAll(r)
		if err != nil {
			log.Warnf("session[ %d] io.ReadAll() err: %s", s.sid, err.Error())
			s.packetChan <- Packet{S: s, Message: serializer.MakeDisconnected()}
			return
		}

		// send packet to chan
		s.packetChan <- Packet{S: s, Message: b}

[mafredri]

@jazzcake-diverse I don't see anything wrong in your snippet, but there could be issues in the surrounding code. Have you made sure it's not possible s.Conn.Reader is being called simultaneously from different goroutines or places in the code?

[jazzcake-diverse]

@mafredri Thank you for feedback. The code snippet is part of goroutine, readPacket() which is run for each session(websocket.Conn from Accept). So I don't think it can be called simultaneously from other goroutine. In client side, I use 3 websocket, 1) UnrealEngine 5 websocket plugin, 2) Godot 4 Websocket, 3) this library as client. This error usually appear in Unreal and Godot side.

• My client just send custom heartbeat packet regularly with 68 bytes and log crc all the time. No other packet is there during test. When this error appeared, size is the same but crc is different. I protect send call and read call on each client and server with mutex for test.