-
Notifications
You must be signed in to change notification settings - Fork 8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix intercepting headers in middlewares #1271
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1271 +/- ##
=======================================
Coverage 98.62% 98.62%
=======================================
Files 41 41
Lines 2112 2112
=======================================
Hits 2083 2083
Misses 18 18
Partials 11 11
Continue to review full report at Codecov.
|
@manucorporat having a look at the history, you seems to have the most knowledge on |
@appleboy @thinkerou you both seem to be active lately, may I ask your help reviewing this change? |
As explained in the TestInterceptedHeader test, in case a middleware filters out the headers, this middleware can be done inefficient in case one following handler is using c.String or other methods writing to the response body directly. This commit fixes the issue by using c.Writer when writing the Status as done in other c.Header, c.SetCookie and other response writers. The bug has been originally discovered using https://github.com/gin-contrib/gzip where a failing test has been added here: https://github.com/tjamet/gzip/blob/header/gzip_test.go#L71 Signed-off-by: Thibault Jamet <tjamet@users.noreply.github.com>
Signed-off-by: Thibault Jamet <tjamet@users.noreply.github.com>
As explained in the TestInterceptedHeader test, in case a middleware
filters out the headers, this middleware can be done inefficient in case
one following handler is using c.String or other methods writing to the
response body directly.
This commit fixes the issue by using c.Writer when writing the Status as
done in other c.Header, c.SetCookie and other response writers.
The bug has been originally discovered using
https://github.com/gin-contrib/gzip where a failing test has been added
here: https://github.com/tjamet/gzip/blob/header/gzip_test.go#L71
Signed-off-by: Thibault Jamet tjamet@users.noreply.github.com