fix(websocket): Preserve stable client IP and expand websocket CI Go matrix#1802
fix(websocket): Preserve stable client IP and expand websocket CI Go matrix#1802Copilot wants to merge 3 commits into
Conversation
Agent-Logs-Url: https://github.com/gofiber/contrib/sessions/32e00bc3-0eb1-4422-a9dc-70e07ea291d0 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
gaby
changed the title
|
@copilot add go 1.26.x to test workflow for websocket. |
gaby
requested review from
ReneWerner87,
Copilot,
efectn and
sixcolors
and removed request for
a team
Agent-Logs-Url: https://github.com/gofiber/contrib/sessions/717fc8d7-74b2-45fe-9dd5-d904f39f6697 Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a data corruption issue in v3/websocket where Conn.IP() could become unstable due to retaining a request-scoped string on a pooled connection object, and adds a regression test to ensure the IP remains stable while other websocket requests are processed.
Changes:
- Copy the request IP with
utils.CopyStringwhen initializing websocket connection state to avoid fasthttp buffer reuse corruption. - Add a regression test that holds one websocket connection open while repeatedly creating additional websocket connections with different proxied IP headers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
v3/websocket/websocket.go |
Copies the IP string before storing it on the pooled websocket Conn, preventing request-buffer reuse corruption. |
v3/websocket/websocket_test.go |
Adds a regression test intended to validate Conn.IP() stability across subsequent websocket requests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| go app.Listen(":3000", fiber.ListenConfig{DisableStartupMessage: true}) | ||
| defer app.Shutdown() | ||
|
|
||
| readyCh := make(chan struct{}) | ||
| go func() { | ||
| for { | ||
| conn, err := net.Dial("tcp", "localhost:3000") | ||
| if err != nil { | ||
| continue | ||
| } | ||
| conn.Close() | ||
| readyCh <- struct{}{} | ||
| return | ||
| } | ||
| }() | ||
| <-readyCh |
There was a problem hiding this comment.
This test starts a real server on a hard-coded ":3000" and waits for readiness via an infinite busy-spin dial loop. If the port is already in use or app.Listen fails, the test can hang indefinitely and burn CPU. Consider using an ephemeral/in-memory listener (see v3/socketio/socketio_test.go using fasthttputil.NewInmemoryListener() + app.Listener) or, at minimum, capture/handle the Listen error and add a bounded timeout + small backoff in the readiness loop.
|
@copilot apply changes based on the comments in this thread |
|
replaced by #1803 |
Uh oh!
There was an error while loading. Please reload this page.