fix: use nats.ProxyPath when server is a websocket connection with a path - #1672
fix: use nats.ProxyPath when server is a websocket connection with a path#1672joeriddles wants to merge 4 commits into
nats.ProxyPath when server is a websocket connection with a path#1672Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d22953689
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 985ccdc534
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a473d11542
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| updated = append(updated, s) | ||
| continue | ||
| } | ||
| if (u.Scheme == "ws" || u.Scheme == "wss") && u.Path != "" && u.Path != "/" { |
There was a problem hiding this comment.
When a server list mixes a WebSocket URL with a proxy path and one that uses the default path, e.g. ws://a.example/nats,ws://b.example, this condition ignores the default-path URL while nats.ProxyPath(proxyPath) is still appended globally for the whole connection. If the client initially selects or reconnects to ws://b.example, it will handshake on /nats instead of /, so a valid fallback server can fail or route to the wrong backend; the extraction should also reject/handle ws/wss URLs whose request target is the default once any non-default proxy path is present.
Useful? React with 👍 / 👎.
| return nil, err | ||
| } | ||
| if proxyPath != "" { | ||
| copts = append(copts, nats.ProxyPath(proxyPath)) |
There was a problem hiding this comment.
this would be a natscontext behaviour instead wouldn't it? It handles the core connection ops setting and parsing etc
There was a problem hiding this comment.
I created a PR in jsm.go: nats-io/jsm.go#813
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2e7d6b560
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| continue | ||
| } | ||
| isWS := u.Scheme == "ws" || u.Scheme == "wss" | ||
| if isWS && u.Path != "" && u.Path != "/" { |
There was a problem hiding this comment.
For WebSocket URLs whose request target is the root plus a query, such as ws://proxy.example/?tenant=a, u.RequestURI() is /?tenant=a but this condition treats u.Path == "/" as the default path and leaves the URL unconverted. The nats.go WebSocket handshake only uses nats.ProxyPath for the request target, so the query is dropped and proxies that route or authorize on that query still receive just /; handle non-empty RawQuery/ForceQuery as a proxy path even when the path is /.
Useful? React with 👍 / 👎.
2 participants
Fix WebSocket connections when the URL has a path.
This is an alternative fix to nats-io/nats.go#2092.