Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

message reader has a default read limit of 32768, net.Conn wrapper doesn't limit size of messages #382

Closed
tomqwpl opened this issue Mar 14, 2023 · 3 comments

Comments

@tomqwpl
Copy link

tomqwpl commented Mar 14, 2023

I have been using the NetConn function to create a net.Conn from a websocket.Conn.
My client is then also written using the same library proxies between a websocket and stdin/stdout (not using NetConn, just (error handling removed for clarity)

		for {
			_, r, _:= c.Reader(ctx)
			_, _ = io.Copy(os.Stdout, r)
		}

By default, the message reader has a limit of 32768 per message, however the net.Conn wrapper produced by NetConn doesn't limit what it sends, it just writes everything from each Write call into a message and sends it on the websocket.

This means that without a call to c.SetReadLimit I'm going to get the socket being closed when the server writes something bigger than 32k to the socket.

I'm not in control of the server, I'm creating a net.Conn that wraps the websocket and then passing it to a third party library. I'm proxying a protocol over a websocket using the equivalent of "netcat" at the client.

Given that there is a read limit on the client, would it make sense to add a write limit to the net.Conn wrapper so that it will chop the data up into messages of at most that size? Otherwise my only real option seems to be to call s.SetReadLimit(math.MaxInt64-1) to effectively remove the read limit (passing math.MaxInt64 results in an overflow as SetReadLimit adds one to the passed in value).

Thanks.

@nhooyr
Copy link
Owner

nhooyr commented Mar 14, 2023

See #254 and #322

In the future the read limit will be disabled by default on NetConn but you'll also be able to pass -1 to c.SetReadLimit to disable it.

@nhooyr nhooyr closed this as completed Mar 14, 2023
@tomqwpl
Copy link
Author

tomqwpl commented Mar 14, 2023

OK, thanks. I see that PR is on the "dev" branch. What does that mean in terms of when that might be on a release? It seems to have been applied 2 years ago from what I can see?

@nhooyr
Copy link
Owner

nhooyr commented Mar 14, 2023

Yea unfortunate situation right now. See #256

I can't give you an ETA other than "soon"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants