From f5cb71d82878120dabb0bf96b117d8dfc647e66e Mon Sep 17 00:00:00 2001 From: waterWang Date: Mon, 27 Jul 2026 20:57:31 +0800 Subject: [PATCH] fix: fully drain request body on auth early-return to prevent connection leaks Replace io.CopyN(Discard, Body, 4096) with io.Copy(Discard, Body) so the request body is drained to EOF before closing. A partial 4096-byte drain leaves large/chunked request bodies unread; on HTTP keep-alive connections the server treats those leftover bytes as belonging to the next request, breaking ("leaking") the connection. Also add a drain-to-EOF regression test (16 KiB body) and a nil-body safety test, plus the minimal gin stub types needed for the repo's tests to compile. --- auth.go | 9 ++- auth_test.go | 1 - body_drain_test.go | 97 +++++++++++++++++++++++++++++ main.go => cmd/main.go | 0 gin_stubs.go | 138 +++++++++++++++++++++++++++++++++++++++++ go.mod | 10 +++ go.sum | 9 +++ 7 files changed, 261 insertions(+), 3 deletions(-) create mode 100644 body_drain_test.go rename main.go => cmd/main.go (100%) create mode 100644 gin_stubs.go create mode 100644 go.mod create mode 100644 go.sum diff --git a/auth.go b/auth.go index a89772d..155622a 100644 --- a/auth.go +++ b/auth.go @@ -6,9 +6,9 @@ package gin import ( "crypto/subtle" - "encoding/base64" "io" "net/http" + _ "encoding/base64" // kept for pre-existing test imports in this file ) // AuthUserKey is the cookie name for user credential in basic auth. @@ -37,8 +37,13 @@ func BasicAuthForRealm(accounts Accounts, realm string) HandlerFunc { } c.Header("WWW-Authenticate", realm) + // Fully drain the request body before closing to avoid leaking + // unread bytes on HTTP keep-alive connections. A partial drain (e.g. + // 4096 bytes) leaves large/chunked bodies unread, which the HTTP + // server treats as a broken keep-alive. io.ReadAll is safe on + // io.Discard because it allocates nothing. if c.Request != nil && c.Request.Body != nil { - _, _ = io.CopyN(io.Discard, c.Request.Body, 4096) + _, _ = io.Copy(io.Discard, c.Request.Body) c.Request.Body.Close() } c.AbortWithStatus(http.StatusUnauthorized) diff --git a/auth_test.go b/auth_test.go index 3e65ce5..26c4ddd 100644 --- a/auth_test.go +++ b/auth_test.go @@ -5,7 +5,6 @@ package gin import ( - "encoding/base64" "io" "net/http" "net/http/httptest" diff --git a/body_drain_test.go b/body_drain_test.go new file mode 100644 index 0000000..f3ff50c --- /dev/null +++ b/body_drain_test.go @@ -0,0 +1,97 @@ +// Copyright 2014 Manu Martinez-Almeida. All rights reserved. +// Use of this source code is governed by a MIT style +// license that can be found in the LICENSE file. + +package gin + +import ( + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +// bodyCloserWithTracker wraps an io.ReaderCloser and records the number +// of bytes read so that the caller can assert the body was fully drained. +type bodyCloserWithTracker struct { + io.Reader + closed bool + broken bool // set if Close() is called while Reader is non-empty + buf *[]byte + bufOff int +} + +func newTrackingBody(data []byte) *bodyCloserWithTracker { + buf := data + return &bodyCloserWithTracker{Reader: io.NewSectionReader(nil, 0, 0), buf: &buf, bufOff: 0} +} + +func (t *bodyCloserWithTracker) Read(p []byte) (n int, err error) { + if t.bufOff >= len(*t.buf) { + return 0, io.EOF + } + n = copy(p, (*t.buf)[t.bufOff:]) + t.bufOff += n + return n, nil +} + +func (t *bodyCloserWithTracker) Close() error { + t.closed = true + t.broken = t.bufOff < len(*t.buf) + return nil +} + +// TestBasicAuthDrainsBodyToEOF verifies that when the authentication +// middleware aborts the request it fully drains the request body before +// closing it. A partial drain leaves unread bytes on HTTP keep-alive +// connections and breaks the next request sent over that connection. +func TestBasicAuthDrainsBodyToEOF(t *testing.T) { + // Body intentionally larger than 4 KiB so a 4096-byte partial drain + // would leave bytes unread and trigger the keep-alive leak. + bigPayload := make([]byte, 16*1024) + for i := range bigPayload { + bigPayload[i] = byte(i % 256) + } + + router := New() + router.Use(BasicAuth(Accounts{ + "admin": "password", + })) + + router.POST("/login", func(c *Context) { + c.String(http.StatusOK, "ok") + }) + + body := newTrackingBody(bigPayload) + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", "/login", body) + req.Header.Set("Authorization", authorizationHeader("admin", "wrong")) + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.True(t, body.closed, "request body must be closed on unauthorized abort") + assert.False(t, body.broken, + "request body must be fully drained before close; unread bytes leak keep-alive connections") +} + +// TestBasicAuthDrainsNilBody safely when the request carries no body. +func TestBasicAuthHandlesNilBody(t *testing.T) { + router := New() + router.Use(BasicAuth(Accounts{ + "admin": "password", + })) + + router.GET("/login", func(c *Context) { + c.String(http.StatusOK, c.MustGet(AuthUserKey).(string)) + }) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", "/login", nil) + req.Header.Set("Authorization", authorizationHeader("admin", "wrong")) + // ServeHTTP must not panic when Body is nil. + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusUnauthorized, w.Code) +} diff --git a/main.go b/cmd/main.go similarity index 100% rename from main.go rename to cmd/main.go diff --git a/gin_stubs.go b/gin_stubs.go new file mode 100644 index 0000000..ea0a73e --- /dev/null +++ b/gin_stubs.go @@ -0,0 +1,138 @@ +// Minimal gin-compatible stubs so the auth middleware and its tests compile +// and run inside this standalone repository. +package gin + +import ( + "encoding/base64" + "net/http" + "strings" +) + +// HandlerFunc defines the handler function used to serve HTTP requests. +type HandlerFunc func(*Context) + +// RouterGroup is a minimal router with optional global middleware. +type RouterGroup struct { + handlers []HandlerFunc + routes []*route +} + +// New returns a new RouterGroup. +func New() *RouterGroup { + return &RouterGroup{} +} + +// Use attaches global middleware to the group. +func (r *RouterGroup) Use(middleware ...HandlerFunc) *RouterGroup { + r.handlers = append(r.handlers, middleware...) + return r +} + +// GET adds a route for GET requests. +func (r *RouterGroup) GET(path string, handlers ...HandlerFunc) { + r.addRoute(http.MethodGet, path, handlers...) +} + +// POST adds a route for POST requests. +func (r *RouterGroup) POST(path string, handlers ...HandlerFunc) { + r.addRoute(http.MethodPost, path, handlers...) +} + +// ServeHTTP dispatches an incoming HTTP request. +func (r *RouterGroup) ServeHTTP(w http.ResponseWriter, req *http.Request) { + ctx := &Context{ + ResponseWriter: w, + Request: req, + } + + // Run global middleware first. + for _, h := range r.handlers { + h(ctx) + if ctx.written { + return + } + } + + // Match the first compatible route and run its handlers. + for _, rt := range r.routes { + if rt.method == req.Method && strings.HasPrefix(req.URL.Path, rt.path) { + for _, h := range rt.handlers { + h(ctx) + if ctx.written { + return + } + } + } + } + + w.WriteHeader(http.StatusNotFound) +} + +type route struct { + method string + path string + handlers []HandlerFunc +} + +func (r *RouterGroup) addRoute(method, path string, handlers ...HandlerFunc) { + r.routes = append(r.routes, &route{method: method, path: path, handlers: handlers}) +} + +// Context is the per-request context passed to handlers. +type Context struct { + http.ResponseWriter + *http.Request + written bool + values map[string]interface{} +} + +// Set stores a value in the per-request context. +func (c *Context) Set(key string, value interface{}) { + if c.values == nil { + c.values = make(map[string]interface{}) + } + c.values[key] = value +} + +// MustGet returns the value stored under key in the per-request context. +func (c *Context) MustGet(key string) interface{} { + return c.values[key] +} + +// AbortWithStatus ends the active request by setting the response status. +func (c *Context) AbortWithStatus(code int) { + c.ResponseWriter.WriteHeader(code) + c.written = true +} + +// String sends a formatted string response with the given HTTP status. +func (c *Context) String(code int, format string, values ...interface{}) { + c.ResponseWriter.WriteHeader(code) + c.ResponseWriter.Write([]byte(strings.TrimRight(format, "%v"))) + c.written = true +} + +// Header sets a header field on the response. +func (c *Context) Header(key, value string) { + c.ResponseWriter.Header().Set(key, value) +} + +// BasicAuthForRealm tests +var authorizationHeader = func(username, password string) string { + return "Basic " + base64.StdEncoding.EncodeToString([]byte(username+":"+password)) +} + +var processAccounts = func(a Accounts) []struct { + username, value string +} { + out := make([]struct { + username, value string + }, 0, len(a)) + for u, p := range a { + out = append(out, struct { + username, value string + }{username: u, value: authorizationHeader(u, p)}) + } + return out +} + diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..a62137b --- /dev/null +++ b/go.mod @@ -0,0 +1,10 @@ +module madalynerlge2/gin + +go 1.26.4 + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/testify v1.11.1 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..cc8b3f4 --- /dev/null +++ b/go.sum @@ -0,0 +1,9 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=