Fix websocket "can't switch protocols" error#16
Open
steffenbusch wants to merge 6 commits into
Open
Conversation
…sts in cookie processing
…upport and remove shouldBypass function
…tailed logging and response writer chain tracking
…thods for improved response handling
### Summary
Fix websocket support when `cookiecrypt` is enabled for a route by updating the response wrapper so Caddy can traverse wrapper layers and preserve protocol upgrade behavior.
### What changed
- Added proper `http.Hijacker` delegation to `cookieInterceptResponseWriter`.
- Added `Unwrap()` support so `http.NewResponseController` can traverse nested Caddy wrappers.
- Preserved the existing `http.Flusher` and `io.ReaderFrom` behavior by forwarding those interfaces from the wrapped writer.
- Previously, `cookieInterceptResponseWriter` wrapped the underlying `ResponseWriter` without exposing the underlying wrapper chain, which broke websocket protocol upgrades.
- This produced the reverse proxy error:
```
ERROR http.handlers.reverse_proxy can't switch protocols using non-Hijacker ResponseWriter {"type": "*cookiecrypt.cookieInterceptResponseWriter"}
```
### Result
- Websocket upgrade requests now succeed again when `cookiecrypt` is enabled.
- Optional `ResponseWriter` interfaces needed for upgrades are now preserved transparently through wrapper layers.
### AI-Disclosure
The implementation approach was identified with the help of ChatGPT during troubleshooting. GitHub Copilot in VS Code was used to assist with implementing the required interface methods.
The initial implementation was based on plain Caddy configuration and appeared to work, but today’s troubleshooting session with GitHub Copilot in VS Code uncovered a deeper issue with more complex wrapper chains. GitHub Copilot (Raptor mini Preview) assisted in diagnosing the response writer chain, adding `Unwrap()` / `Hijack()` delegation, and cleaning up the middleware changes so the fix now works correctly for more complex Caddy setups.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix websocket support when
cookiecryptis enabled for a route by updating the response wrapper so Caddy can traverse wrapper layers and preserve protocol upgrade behavior.What changed
http.Hijackerdelegation tocookieInterceptResponseWriter.Unwrap()support sohttp.NewResponseControllercan traverse nested Caddy wrappers.http.Flusherandio.ReaderFrombehavior by forwarding those interfaces from the wrapped writer.cookieInterceptResponseWriterwrapped the underlyingResponseWriterwithout exposing the underlying wrapper chain, which broke websocket protocol upgrades.Result
cookiecryptis enabled.ResponseWriterinterfaces needed for upgrades are now preserved transparently through wrapper layers.AI-Disclosure
The implementation approach was identified with the help of ChatGPT during troubleshooting. GitHub Copilot in VS Code was used to assist with implementing the required interface methods.
The initial implementation was based on plain Caddy configuration and appeared to work, but today’s troubleshooting session with GitHub Copilot in VS Code uncovered a deeper issue with more complex wrapper chains. GitHub Copilot (Raptor mini Preview) assisted in diagnosing the response writer chain, adding
Unwrap()/Hijack()delegation, and cleaning up the middleware changes so the fix now works correctly for more complex Caddy setups.