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

websocket: Don't limit message sizes in the websocket reader #2193

Merged
merged 4 commits into from
Mar 16, 2023

Conversation

MarcoPolo
Copy link
Collaborator

Fixes #2188

Adds a regression test and makes sure the websocket library isn't allocating the whole message in a byte slice somewhere.

Copy link
Contributor

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from person who is very excited for this to be fixed but not actually capable of being a real reviewer.

p2p/transport/websocket/listener.go Outdated Show resolved Hide resolved
p2p/transport/websocket/listener.go Outdated Show resolved Hide resolved
MarcoPolo and others added 2 commits March 16, 2023 15:27
Co-authored-by: Marten Seemann <martenseemann@gmail.com>
Co-authored-by: Marten Seemann <martenseemann@gmail.com>
@MarcoPolo MarcoPolo merged commit 8086788 into master Mar 16, 2023
MarcoPolo added a commit that referenced this pull request Mar 16, 2023
* Don't limit message sizes in the websocket reader

* Remove fmt.println

* Update p2p/transport/websocket/listener.go

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

* Update p2p/transport/websocket/listener.go

Co-authored-by: Marten Seemann <martenseemann@gmail.com>

---------

Co-authored-by: Marten Seemann <martenseemann@gmail.com>
marten-seemann added a commit that referenced this pull request May 11, 2023
* Revert "websocket: don't set a WSS multiaddr for accepted unencrypted conns (#2199)"

This reverts commit eeb685f.

* Revert "websocket: Don't limit message sizes in the websocket reader (#2193)"

This reverts commit 2fe6600.

* Revert "websocket: Replace gorilla websocket transport with nhooyr websocket transport (#1982)"

* websocket: don't set a WSS multiaddr for accepted unencrypted conns
btwiuse pushed a commit to btwiuse/wsconn that referenced this pull request May 4, 2024
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

Successfully merging this pull request may close these issues.

Bitswap problems over websocket transport since switch to nhooyr websocket
3 participants