fix(response): refine hijack behavior for response lifecycle#4373
Merged
appleboy merged 3 commits intogin-gonic:masterfrom Sep 26, 2025
Merged
fix(response): refine hijack behavior for response lifecycle#4373appleboy merged 3 commits intogin-gonic:masterfrom
appleboy merged 3 commits intogin-gonic:masterfrom
Conversation
- Clarify the error message for attempted hijack after response body data is written - Modify hijack behavior: allow hijacking after headers are written (for better websocket compatibility), but block hijacking after any body data is sent - Add comprehensive tests to validate allowed hijack after header write and disallowed hijack after body write fix gin-gonic#4372 Signed-off-by: appleboy <appleboy.tw@gmail.com>
This was referenced Sep 24, 2025
- Replace assert with require for error checks to ensure test failures immediately halt execution Signed-off-by: appleboy <appleboy.tw@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4373 +/- ##
==========================================
- Coverage 99.21% 98.92% -0.29%
==========================================
Files 42 44 +2
Lines 3182 3442 +260
==========================================
+ Hits 3157 3405 +248
- Misses 17 26 +9
- Partials 8 11 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refines the hijack behavior in Gin's response writer to improve WebSocket compatibility by allowing hijacking after headers are written but blocking it after response body data is sent.
- Modified hijack logic to check
w.size > 0instead ofWritten()to allow hijacking after header-only writes - Updated error message to be more specific about when hijacking is blocked
- Added comprehensive tests validating the new hijack behavior patterns
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| response_writer.go | Updates hijack logic and error message to allow hijacking after headers but block after body data |
| response_writer_test.go | Adds comprehensive test cases validating the new hijack behavior for WebSocket compatibility |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thanks a lot for this fix ! |
1 task
pdkovacs
added a commit
to pdkovacs/wsgw
that referenced
this pull request
Nov 1, 2025
i.e. upgrade to pre-v1.12
pdkovacs
added a commit
to pdkovacs/wsgw
that referenced
this pull request
Nov 2, 2025
i.e. upgrade to pre-v1.12
|
Hi @appleboy, any ETA for the release of this fix? 🙏 |
pdkovacs
added a commit
to pdkovacs/wsgw
that referenced
this pull request
Nov 28, 2025
i.e. upgrade to pre-v1.12
|
Hello, |
4 tasks
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
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.
fix #4372
Pull Request Checklist
Please ensure your pull request meets the following requirements:
masterbranch.docs/doc.md.Thank you for contributing!