[PR #5204] [MERGED] fix: WebsocketListener nil panic and OIDC auth data race #5171

New issue

Closed

opened 2026-05-05 14:56:03 -06:00 by gitea-mirror · 0 comments

gitea-mirror commented

2026-05-05 14:56:03 -06:00

Owner

📋 Pull Request Information

Original PR: https://github.com/fatedier/frp/pull/5204
Author: @fatedier
Created: 3/6/2026
Status: ✅ Merged
Merged: 3/6/2026
Merged by: @fatedier

Base: dev ← Head: new

📝 Commits (1)

4d04c68 fix: WebsocketListener nil panic and OIDC auth data race

📊 Changes

2 files changed (+13 additions, -8 deletions)

View changed files

📝 pkg/auth/oidc.go (+12 -8)
📝 pkg/util/net/websocket.go (+1 -0)

📄 Description

Summary

pkg/util/net/websocket.go: NewWebsocketListener never stored the ln parameter in the struct, causing Addr() to panic with nil pointer dereference. Added ln: ln to the struct literal.
pkg/auth/oidc.go: OidcAuthConsumer.subjectsFromLogin was accessed concurrently without synchronization — VerifyLogin writes (append) while verifyPostLoginToken reads (slices.Contains) from separate goroutines. Replaced []string with map[string]struct{} protected by sync.RWMutex.

Impact

websocket.go: Latent bug — Addr() is not currently called but violates net.Listener contract. One-line fix, zero risk.
oidc.go: Production-reachable data race for OIDC users. The verifier is shared globally across all client connections (each in its own goroutine). Worst case: undefined behavior, auth misdecisions, crash.

Verification

make build passes
make test passes
go test ./pkg/auth ./pkg/util/net passes
Codex review: no introduced bugs
No copylock issues (struct accessed via pointer)

Test plan

Build and unit tests pass
Codex deep analysis confirmed both fixes are correct and necessary
Manual code review verified concurrent access paths

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/fatedier/frp/pull/5204 **Author:** [@fatedier](https://github.com/fatedier) **Created:** 3/6/2026 **Status:** ✅ Merged **Merged:** 3/6/2026 **Merged by:** [@fatedier](https://github.com/fatedier) **Base:** `dev` ← **Head:** `new` --- ### 📝 Commits (1) - [`4d04c68`](https://github.com/fatedier/frp/commit/4d04c68d19d8bbd195861d154c691339462a05ae) fix: WebsocketListener nil panic and OIDC auth data race ### 📊 Changes **2 files changed** (+13 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `pkg/auth/oidc.go` (+12 -8) 📝 `pkg/util/net/websocket.go` (+1 -0) </details> ### 📄 Description ## Summary - **pkg/util/net/websocket.go**: `NewWebsocketListener` never stored the `ln` parameter in the struct, causing `Addr()` to panic with nil pointer dereference. Added `ln: ln` to the struct literal. - **pkg/auth/oidc.go**: `OidcAuthConsumer.subjectsFromLogin` was accessed concurrently without synchronization — `VerifyLogin` writes (append) while `verifyPostLoginToken` reads (`slices.Contains`) from separate goroutines. Replaced `[]string` with `map[string]struct{}` protected by `sync.RWMutex`. ## Impact - **websocket.go**: Latent bug — `Addr()` is not currently called but violates `net.Listener` contract. One-line fix, zero risk. - **oidc.go**: Production-reachable data race for OIDC users. The verifier is shared globally across all client connections (each in its own goroutine). Worst case: undefined behavior, auth misdecisions, crash. ## Verification - `make build` passes - `make test` passes - `go test ./pkg/auth ./pkg/util/net` passes - Codex review: no introduced bugs - No copylock issues (struct accessed via pointer) ## Test plan - [x] Build and unit tests pass - [x] Codex deep analysis confirmed both fixes are correct and necessary - [x] Manual code review verified concurrent access paths --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>