From b5cfe4bdb5bd13966c0c0e944c3e80afce5f03bd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 23:14:55 +0000 Subject: [PATCH 1/2] fix: close request body when BasicAuth rejects a request When the middleware aborts an unauthenticated request, c.Request.Body was left open, so whatever backs it stayed held until the request was torn down. Close it on the rejection path, before aborting. closeRequestBody guards a nil context, a nil Request and a nil or NoBody Body, and drops the error from Close: the request is already being rejected, so failing to drain a body being discarded changes nothing. The success path is untouched, so downstream handlers still read the body. No manual drain in front of the Close. net/http sets doEarlyClose on server request bodies, so Body.Close already drains up to 256 KiB looking for EOF and leaves the connection reusable, and flags the body for the server to close the connection when more than that is pending. Close is idempotent, so the server's own Close after the handler returns is a no-op. This also makes the package build. It had no go.mod, auth.go referenced gin types that were not defined anywhere in the repo, auth_test.go called processAccounts and authorizationHeader which did not exist, base64 was imported but unused, and auth.go (package gin) shared a directory with main.go (package main). The middleware now builds against gin itself as a drop-in replacement for gin.BasicAuth, with the starter program moved to cmd/demo. Tests cover the rejection path (body closed exactly once, for wrong passwords, unknown users, missing and malformed headers), the success path (body left open and readable), nil-safety, a failing Close, Keep-Alive reuse under and over the drain limit, and concurrency. The body-close tests fail with the fix reverted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LehYDiBQajTGivEPN7umCT --- README.md | 108 +++++++++++++- auth.go | 143 ++++++++++++++---- auth_test.go | 369 +++++++++++++++++++++++++++++++++++++++-------- cmd/demo/main.go | 35 +++++ go.mod | 39 +++++ go.sum | 89 ++++++++++++ main.go | 7 - 7 files changed, 692 insertions(+), 98 deletions(-) create mode 100644 cmd/demo/main.go create mode 100644 go.mod create mode 100644 go.sum delete mode 100644 main.go diff --git a/README.md b/README.md index e9966bf..13c2920 100644 --- a/README.md +++ b/README.md @@ -1 +1,107 @@ -# gin \ No newline at end of file +# gin + +HTTP Basic authentication middleware for [gin](https://github.com/gin-gonic/gin) +that closes the request body when it rejects a request. + +This addresses +[issue #1](https://github.com/madalynerlge2/gin/issues/1): when `BasicAuth` +aborts an unauthenticated request, `c.Request.Body` is left open, so whatever +backs it stays held until the request is torn down. + +## Usage + +```go +import ( + "github.com/gin-gonic/gin" + ginauth "github.com/jpka/gin" +) + +router := gin.Default() +router.Use(ginauth.BasicAuth(ginauth.Accounts{ + "admin": "password", +})) +``` + +`BasicAuth`, `BasicAuthForRealm`, `Accounts` and `AuthUserKey` mirror gin's own +API, so an existing `gin.BasicAuth(...)` call can be swapped for +`ginauth.BasicAuth(...)` unchanged. + +`go run ./cmd/demo` starts a server on `:8080` to try it against: + +``` +curl -i -u admin:password -d 'payload' http://localhost:8080/login # 200 +curl -i -u admin:wrong -d 'payload' http://localhost:8080/login # 401 +``` + +## The change + +On the rejection path the body is closed before the chain is aborted: + +```go +if !found { + closeRequestBody(c) + c.Header("WWW-Authenticate", realm) + c.AbortWithStatus(http.StatusUnauthorized) + return +} +``` + +`closeRequestBody` guards a nil context, a nil `Request` and a nil or `NoBody` +`Body` before calling `Close`, and drops the returned error — the request is +already being rejected, so failing to drain a body that is being discarded +changes nothing. On the success path the body is untouched, so downstream +handlers still read it normally. + +Applied to gin's own `auth.go`, the equivalent patch is: + +```diff + return func(c *Context) { + user, found := pairs.searchCredential(c.requestHeader("Authorization")) + if !found { ++ // The request is rejected here, so no downstream handler will ever ++ // read the body. Close it now rather than leaving it pinned until ++ // the server tears the request down. ++ if c.Request != nil && c.Request.Body != nil { ++ _ = c.Request.Body.Close() ++ } + c.Header("WWW-Authenticate", realm) + c.AbortWithStatus(http.StatusUnauthorized) + return + } +``` + +### Keep-Alive + +Closing the body in the handler is safe for connection reuse, and no manual +drain belongs in front of it. `net/http` sets `doEarlyClose` on server request +bodies (`server.go`, in `conn.readRequest`), which makes `Body.Close` drain at +most `maxPostHandlerReadBytes` (256 KiB) looking for EOF and then leave the +connection reusable. If more than that is still pending it flags the body +instead, and the server closes the connection rather than reusing a +desynchronised one. `Close` is idempotent, so the server's own `Close` after the +handler returns is a no-op. + +`TestBasicAuthKeepAlive` asserts the reuse behaviour against a real server over +a real socket; `TestBasicAuthKeepAliveLargeBody` covers the over-limit case. + +### Scope + +Worth being straight about what this does and does not fix. Under a plain +`net/http` server the leak in the report is bounded: the server closes the +request body itself once the handler returns, so an unclosed body is held for +the rest of the request, not forever, and the `CLOSE_WAIT` growth described in +the issue is more likely to come from clients or proxies that do not read the +401 response. What closing early does buy is a shorter hold time under load, and +correct behaviour for bodies that wrap a resource of their own — a decompressor, +a temp file, a metered reader — where `Close` is the only thing that releases it. + +## Tests + +``` +go test -race ./... +``` + +Covering the rejection path (body closed exactly once, for wrong passwords, +unknown users, missing and malformed headers), the success path (body left open +and readable by the handler), nil-safety, a failing `Close`, Keep-Alive reuse +under and over the drain limit, and the rejection path under concurrency. diff --git a/auth.go b/auth.go index a89772d..c96d3e2 100644 --- a/auth.go +++ b/auth.go @@ -2,50 +2,137 @@ // Use of this source code is governed by a MIT style // license that can be found in the LICENSE file. -package gin +// Package ginauth provides HTTP Basic authentication middleware for gin. +// +// It is a drop-in replacement for gin.BasicAuth / gin.BasicAuthForRealm that +// closes the request body before aborting an unauthenticated request, so the +// resources backing the body are released as soon as the request is rejected +// rather than at the end of the request lifecycle. +package ginauth import ( "crypto/subtle" "encoding/base64" - "io" "net/http" + "strconv" + + "github.com/gin-gonic/gin" ) -// AuthUserKey is the cookie name for user credential in basic auth. +// AuthUserKey is the key under which the name of the authenticated user is +// stored in the gin context. It mirrors gin.AuthUserKey. const AuthUserKey = "user" -// Accounts is a shortcut for map[string]string +// Accounts is a shortcut for map[string]string, mapping user names to passwords. type Accounts map[string]string -// BasicAuthForRealm returns a gin.HandlerFunc ... -func BasicAuthForRealm(accounts Accounts, realm string) HandlerFunc { - if realm == "" { - realm = "Basic realm=\"Authorization Required\"" - } else { - realm = "Basic realm=\"" + realm + "\"" - } - - return func(c *Context) { - user, password, hasAuth := c.Request.BasicAuth() - if hasAuth { - if secret, ok := accounts[user]; ok { - if subtle.ConstantTimeCompare([]byte(secret), []byte(password)) == 1 { - c.Set(AuthUserKey, user) - return - } - } +type authPair struct { + value string + user string +} + +type authPairs []authPair + +// searchCredential returns the user matching the given Authorization header +// value. The comparison is constant time so that a caller cannot learn a valid +// credential from the time the lookup takes. +func (a authPairs) searchCredential(authValue string) (string, bool) { + if authValue == "" { + return "", false + } + for _, pair := range a { + if subtle.ConstantTimeCompare([]byte(pair.value), []byte(authValue)) == 1 { + return pair.user, true } + } + return "", false +} - c.Header("WWW-Authenticate", realm) - if c.Request != nil && c.Request.Body != nil { - _, _ = io.CopyN(io.Discard, c.Request.Body, 4096) - c.Request.Body.Close() +func processAccounts(accounts Accounts) authPairs { + if len(accounts) == 0 { + panic("empty list of authorized credentials") + } + pairs := make(authPairs, 0, len(accounts)) + for user, password := range accounts { + if user == "" { + panic("user can not be empty") } - c.AbortWithStatus(http.StatusUnauthorized) + pairs = append(pairs, authPair{ + value: authorizationHeader(user, password), + user: user, + }) } + return pairs } -// BasicAuth returns a gin.HandlerFunc ... -func BasicAuth(accounts Accounts) HandlerFunc { +func authorizationHeader(user, password string) string { + base := user + ":" + password + return "Basic " + base64.StdEncoding.EncodeToString([]byte(base)) +} + +// BasicAuthForRealm returns a Basic HTTP Authorization middleware. It takes as +// argument a map[string]string where the key is the user name and the value is +// the password, as well as the name of the realm. If the realm is empty, +// "Authorization Required" is used by default. +// +// On success the user name is stored in the context under AuthUserKey and the +// handler chain continues. On failure the request body is closed, the +// WWW-Authenticate header is set and the chain is aborted with 401. +func BasicAuthForRealm(accounts Accounts, realm string) gin.HandlerFunc { + if realm == "" { + realm = "Authorization Required" + } + realm = "Basic realm=" + strconv.Quote(realm) + pairs := processAccounts(accounts) + + return func(c *gin.Context) { + user, found := pairs.searchCredential(c.GetHeader("Authorization")) + if !found { + // The request is rejected here, so no downstream handler will ever + // read the body. Close it now instead of leaving it pinned until + // the server tears the request down. + closeRequestBody(c) + c.Header("WWW-Authenticate", realm) + c.AbortWithStatus(http.StatusUnauthorized) + return + } + + // The user credentials were found, set the user's id under AuthUserKey + // so downstream handlers can read it with c.MustGet(AuthUserKey). + c.Set(AuthUserKey, user) + } +} + +// BasicAuth returns a Basic HTTP Authorization middleware. It takes as argument +// a map[string]string where the key is the user name and the value is the +// password. +func BasicAuth(accounts Accounts) gin.HandlerFunc { return BasicAuthForRealm(accounts, "") } + +// closeRequestBody closes the request body, if there is one to close. +// +// Both the context and the request can legitimately be nil (a Context built by +// hand in a test, for example), and a hand-built Request may carry a nil Body, +// so every hop is guarded: a panic inside auth middleware would be a far worse +// outcome than a body left open. +// +// Closing here is safe for Keep-Alive. net/http marks server request bodies +// with doEarlyClose, so Close drains at most maxPostHandlerReadBytes (256 KiB) +// looking for EOF and then lets the connection be reused; if more than that is +// still pending it flags the body instead and the server closes the connection +// rather than desynchronising it. Draining by hand before closing would only +// duplicate that logic, less correctly. Close is also idempotent, so the +// server's own Close after the handler returns is a no-op. +func closeRequestBody(c *gin.Context) { + if c == nil || c.Request == nil { + return + } + body := c.Request.Body + if body == nil || body == http.NoBody { + return + } + // The error is deliberately dropped: the request is already being rejected, + // and failing to drain a body we are discarding changes nothing. + _ = body.Close() +} diff --git a/auth_test.go b/auth_test.go index 3e65ce5..63ece77 100644 --- a/auth_test.go +++ b/auth_test.go @@ -2,117 +2,362 @@ // Use of this source code is governed by a MIT style // license that can be found in the LICENSE file. -package gin +package ginauth import ( - "encoding/base64" + "bytes" + "errors" "io" "net/http" "net/http/httptest" + "net/http/httptrace" + "strings" + "sync/atomic" "testing" + "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func init() { + gin.SetMode(gin.TestMode) +} + +// closeTrackingBody is a request body that records whether Close was called and +// how many times, so tests can assert on the middleware's handling of it. type closeTrackingBody struct { - closed bool - data []byte - read int + reader *bytes.Reader + closeCount int32 + closeErr error } -func (c *closeTrackingBody) Read(p []byte) (n int, err error) { - if c.read >= len(c.data) { - return 0, io.EOF - } - n = copy(p, c.data[c.read:]) - c.read += n - return n, nil +func newTrackingBody(data string) *closeTrackingBody { + return &closeTrackingBody{reader: bytes.NewReader([]byte(data))} } -func (c *closeTrackingBody) Close() error { - c.closed = true - return nil +func (b *closeTrackingBody) Read(p []byte) (int, error) { + return b.reader.Read(p) } -func TestBasicAuth(t *testing.T) { - pairs := processAccounts(Accounts{ - "admin": "password", - }) +func (b *closeTrackingBody) Close() error { + atomic.AddInt32(&b.closeCount, 1) + return b.closeErr +} - assert.Len(t, pairs, 1) - assert.Equal(t, authorizationHeader("admin", "password"), pairs[0].value) +func (b *closeTrackingBody) closed() bool { + return atomic.LoadInt32(&b.closeCount) > 0 +} + +func (b *closeTrackingBody) closes() int { + return int(atomic.LoadInt32(&b.closeCount)) +} - router := New() +// newRouter builds a router guarded by BasicAuth. The final handler records +// whether it ran and echoes back the authenticated user. +func newRouter(t *testing.T, reached *bool) *gin.Engine { + t.Helper() + + handler := func(c *gin.Context) { + if reached != nil { + *reached = true + } + c.String(http.StatusOK, c.MustGet(AuthUserKey).(string)) + } + + router := gin.New() router.Use(BasicAuth(Accounts{ "admin": "password", "foo": "bar", })) + router.GET("/login", handler) + router.POST("/login", handler) + return router +} - router.GET("/login", func(c *Context) { - c.String(http.StatusOK, c.MustGet(AuthUserKey).(string)) - }) +func TestAuthorizationHeader(t *testing.T) { + // "admin:password", base64-encoded. + assert.Equal(t, "Basic YWRtaW46cGFzc3dvcmQ=", authorizationHeader("admin", "password")) +} - w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/login", nil) - req.Header.Set("Authorization", authorizationHeader("admin", "password")) - router.ServeHTTP(w, req) +func TestProcessAccounts(t *testing.T) { + assert.Panics(t, func() { processAccounts(nil) }) + assert.Panics(t, func() { processAccounts(Accounts{}) }) + assert.Panics(t, func() { processAccounts(Accounts{"": "password"}) }) - assert.Equal(t, http.StatusOK, w.Code) - assert.Equal(t, "admin", w.Body.String()) + pairs := processAccounts(Accounts{"admin": "password"}) + require.Len(t, pairs, 1) + assert.Equal(t, authorizationHeader("admin", "password"), pairs[0].value) + assert.Equal(t, "admin", pairs[0].user) +} - w = httptest.NewRecorder() - req, _ = http.NewRequest("GET", "/login", nil) - req.Header.Set("Authorization", authorizationHeader("foo", "bar")) - router.ServeHTTP(w, req) +func TestSearchCredential(t *testing.T) { + pairs := processAccounts(Accounts{"admin": "password", "foo": "bar"}) - assert.Equal(t, http.StatusOK, w.Code) - assert.Equal(t, "foo", w.Body.String()) + user, found := pairs.searchCredential(authorizationHeader("foo", "bar")) + assert.True(t, found) + assert.Equal(t, "foo", user) + + user, found = pairs.searchCredential(authorizationHeader("foo", "wrong")) + assert.False(t, found) + assert.Empty(t, user) + + user, found = pairs.searchCredential("") + assert.False(t, found) + assert.Empty(t, user) +} - w = httptest.NewRecorder() - req, _ = http.NewRequest("GET", "/login", nil) +func TestBasicAuthSucceeds(t *testing.T) { + for _, tc := range []struct{ user, password string }{ + {"admin", "password"}, + {"foo", "bar"}, + } { + reached := false + router := newRouter(t, &reached) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/login", nil) + req.Header.Set("Authorization", authorizationHeader(tc.user, tc.password)) + router.ServeHTTP(w, req) + + assert.True(t, reached, "handler should run for %s", tc.user) + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, tc.user, w.Body.String()) + assert.Empty(t, w.Header().Get("WWW-Authenticate")) + } +} + +func TestBasicAuthRejects(t *testing.T) { + for name, header := range map[string]string{ + "wrong password": authorizationHeader("admin", "wrong"), + "unknown user": authorizationHeader("nobody", "password"), + "missing header": "", + "malformed": "Basic not-base64", + "wrong scheme": "Bearer abcdefgh", + } { + t.Run(name, func(t *testing.T) { + reached := false + router := newRouter(t, &reached) + + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/login", nil) + if header != "" { + req.Header.Set("Authorization", header) + } + router.ServeHTTP(w, req) + + assert.False(t, reached, "handler must not run") + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.Equal(t, `Basic realm="Authorization Required"`, w.Header().Get("WWW-Authenticate")) + }) + } +} + +func TestBasicAuthForRealmUsesCustomRealm(t *testing.T) { + router := gin.New() + router.Use(BasicAuthForRealm(Accounts{"admin": "password"}, `My "Site"`)) + router.GET("/login", func(c *gin.Context) { c.String(http.StatusOK, "ok") }) + + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/login", nil)) + + assert.Equal(t, http.StatusUnauthorized, w.Code) + // The realm is quoted, so an embedded quote is escaped rather than closing + // the header value early. + assert.Equal(t, `Basic realm="My \"Site\""`, w.Header().Get("WWW-Authenticate")) +} + +// TestBasicAuthClosesBodyOnFailure is the regression test for the reported +// leak: an aborted request must not leave its body open. +func TestBasicAuthClosesBodyOnFailure(t *testing.T) { + router := newRouter(t, nil) + + body := newTrackingBody("a request body that is never consumed") + req := httptest.NewRequest(http.MethodPost, "/login", body) req.Header.Set("Authorization", authorizationHeader("admin", "wrong")) + + w := httptest.NewRecorder() router.ServeHTTP(w, req) assert.Equal(t, http.StatusUnauthorized, w.Code) - assert.Equal(t, "Basic realm=\"Authorization Required\"", w.Header().Get("WWW-Authenticate")) + assert.True(t, body.closed(), "request body must be closed when auth fails") + assert.Equal(t, 1, body.closes(), "request body must be closed exactly once") } -func TestBasicAuthBodyClosed(t *testing.T) { - router := New() - router.Use(BasicAuth(Accounts{ - "admin": "password", - })) +// TestBasicAuthClosesBodyWithoutAuthorizationHeader covers the most common +// unauthenticated case: a client that sends a payload but no credentials. +func TestBasicAuthClosesBodyWithoutAuthorizationHeader(t *testing.T) { + router := newRouter(t, nil) - router.POST("/login", func(c *Context) { - c.String(http.StatusOK, "ok") - }) + body := newTrackingBody("payload") + req := httptest.NewRequest(http.MethodPost, "/login", body) - body := &closeTrackingBody{data: []byte("some body data")} 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) + assert.True(t, body.closed()) } -func TestBasicAuthBodyNotClosedOnSuccess(t *testing.T) { - router := New() - router.Use(BasicAuth(Accounts{ - "admin": "password", - })) +// TestBasicAuthCloseErrorIsIgnored makes sure a body whose Close fails still +// produces a clean 401 rather than a panic or a 500. +func TestBasicAuthCloseErrorIsIgnored(t *testing.T) { + router := newRouter(t, nil) + + body := newTrackingBody("payload") + body.closeErr = errors.New("close failed") + req := httptest.NewRequest(http.MethodPost, "/login", body) + + w := httptest.NewRecorder() + assert.NotPanics(t, func() { router.ServeHTTP(w, req) }) + assert.Equal(t, http.StatusUnauthorized, w.Code) + assert.True(t, body.closed()) +} + +// TestBasicAuthDoesNotCloseBodyOnSuccess guards against the obvious regression: +// closing the body of an authenticated request would break every handler that +// reads it. +func TestBasicAuthDoesNotCloseBodyOnSuccess(t *testing.T) { + var got string - router.POST("/login", func(c *Context) { + router := gin.New() + router.Use(BasicAuth(Accounts{"admin": "password"})) + router.POST("/login", func(c *gin.Context) { + data, err := io.ReadAll(c.Request.Body) + require.NoError(t, err) + got = string(data) c.String(http.StatusOK, "ok") }) - body := &closeTrackingBody{data: []byte("some body data")} - w := httptest.NewRecorder() - req, _ := http.NewRequest("POST", "/login", body) + body := newTrackingBody("the handler must still be able to read this") + req := httptest.NewRequest(http.MethodPost, "/login", body) req.Header.Set("Authorization", authorizationHeader("admin", "password")) + + w := httptest.NewRecorder() router.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) - assert.False(t, body.closed) + assert.Equal(t, "the handler must still be able to read this", got) + assert.False(t, body.closed(), "the body of an authenticated request must stay open") +} + +// TestCloseRequestBodyIsNilSafe covers the guards in closeRequestBody directly. +// None of these shapes occur on a served request, but middleware is also driven +// by hand-built contexts in tests and must not panic. +func TestCloseRequestBodyIsNilSafe(t *testing.T) { + assert.NotPanics(t, func() { closeRequestBody(nil) }) + assert.NotPanics(t, func() { closeRequestBody(&gin.Context{}) }) + assert.NotPanics(t, func() { + closeRequestBody(&gin.Context{Request: &http.Request{}}) + }) + assert.NotPanics(t, func() { + closeRequestBody(&gin.Context{Request: &http.Request{Body: http.NoBody}}) + }) +} + +// TestBasicAuthNilRequestBody exercises the same guard through the middleware, +// via a request whose Body was cleared by an earlier handler. +func TestBasicAuthNilRequestBody(t *testing.T) { + router := gin.New() + router.Use(func(c *gin.Context) { c.Request.Body = nil }) + router.Use(BasicAuth(Accounts{"admin": "password"})) + router.POST("/login", func(c *gin.Context) { c.String(http.StatusOK, "ok") }) + + req := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader("payload")) + w := httptest.NewRecorder() + + assert.NotPanics(t, func() { router.ServeHTTP(w, req) }) + assert.Equal(t, http.StatusUnauthorized, w.Code) +} + +// TestBasicAuthKeepAlive is the acceptance test for the connection-reuse +// requirement. It runs a real server over a real socket and asserts that +// consecutive rejected requests, each carrying a body, share one connection — +// i.e. closing the body early does not desynchronise the connection or force +// the server to hang up. +func TestBasicAuthKeepAlive(t *testing.T) { + router := gin.New() + router.Use(BasicAuth(Accounts{"admin": "password"})) + router.POST("/login", func(c *gin.Context) { c.String(http.StatusOK, "ok") }) + + server := httptest.NewServer(router) + defer server.Close() + + transport := &http.Transport{} + defer transport.CloseIdleConnections() + client := &http.Client{Transport: transport} + + var reused []bool + for i := 0; i < 3; i++ { + var connReused bool + trace := &httptrace.ClientTrace{ + GotConn: func(info httptrace.GotConnInfo) { connReused = info.Reused }, + } + + req, err := http.NewRequest(http.MethodPost, server.URL+"/login", strings.NewReader("payload")) + require.NoError(t, err) + req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace)) + + resp, err := client.Do(req) + require.NoError(t, err) + _, err = io.Copy(io.Discard, resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) + reused = append(reused, connReused) + } + + assert.Equal(t, []bool{false, true, true}, reused, + "rejected requests must keep the connection alive for reuse") +} + +// TestBasicAuthKeepAliveLargeBody covers the other side of that contract. When +// more than maxPostHandlerReadBytes (256 KiB) is still pending, net/http gives +// up on draining and closes the connection rather than reusing a desynchronised +// one. The request must still be answered correctly. +func TestBasicAuthKeepAliveLargeBody(t *testing.T) { + router := gin.New() + router.Use(BasicAuth(Accounts{"admin": "password"})) + router.POST("/login", func(c *gin.Context) { c.String(http.StatusOK, "ok") }) + + server := httptest.NewServer(router) + defer server.Close() + + transport := &http.Transport{} + defer transport.CloseIdleConnections() + client := &http.Client{Transport: transport} + + large := strings.Repeat("x", 1<<20) // 1 MiB, well over the 256 KiB drain limit. + for i := 0; i < 2; i++ { + resp, err := client.Post(server.URL+"/login", "text/plain", strings.NewReader(large)) + require.NoError(t, err) + _, err = io.Copy(io.Discard, resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) + } +} + +// TestBasicAuthConcurrent runs the rejection path under load to shake out data +// races on the shared credential table and on the body handling. +func TestBasicAuthConcurrent(t *testing.T) { + router := newRouter(t, nil) + + done := make(chan *closeTrackingBody, 64) + for i := 0; i < 64; i++ { + go func() { + body := newTrackingBody("payload") + req := httptest.NewRequest(http.MethodPost, "/login", body) + req.Header.Set("Authorization", authorizationHeader("admin", "wrong")) + router.ServeHTTP(httptest.NewRecorder(), req) + done <- body + }() + } + + for i := 0; i < 64; i++ { + body := <-done + assert.True(t, body.closed()) + } } diff --git a/cmd/demo/main.go b/cmd/demo/main.go new file mode 100644 index 0000000..6798d40 --- /dev/null +++ b/cmd/demo/main.go @@ -0,0 +1,35 @@ +// Command demo runs a small gin server guarded by the BasicAuth middleware in +// this repository, so the behaviour described in the README can be exercised by +// hand: +// +// go run ./cmd/demo +// curl -i -u admin:password -d 'payload' http://localhost:8080/login # 200 +// curl -i -u admin:wrong -d 'payload' http://localhost:8080/login # 401 +// +// The second request is rejected by the middleware, which closes the request +// body before aborting. +package main + +import ( + "log" + "net/http" + + "github.com/gin-gonic/gin" + + ginauth "github.com/jpka/gin" +) + +func main() { + router := gin.Default() + router.Use(ginauth.BasicAuth(ginauth.Accounts{ + "admin": "password", + })) + + router.POST("/login", func(c *gin.Context) { + c.String(http.StatusOK, "hello, %s", c.MustGet(ginauth.AuthUserKey)) + }) + + if err := router.Run(":8080"); err != nil { + log.Fatal(err) + } +} diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..300eb82 --- /dev/null +++ b/go.mod @@ -0,0 +1,39 @@ +module github.com/jpka/gin + +go 1.21 + +require ( + github.com/gin-gonic/gin v1.10.0 + github.com/stretchr/testify v1.9.0 +) + +require ( + github.com/bytedance/sonic v1.11.6 // indirect + github.com/bytedance/sonic/loader v0.1.1 // indirect + github.com/cloudwego/base64x v0.1.4 // indirect + github.com/cloudwego/iasm v0.2.0 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/gabriel-vasile/mimetype v1.4.3 // indirect + github.com/gin-contrib/sse v0.1.0 // indirect + github.com/go-playground/locales v0.14.1 // indirect + github.com/go-playground/universal-translator v0.18.1 // indirect + github.com/go-playground/validator/v10 v10.20.0 // indirect + github.com/goccy/go-json v0.10.2 // indirect + github.com/json-iterator/go v1.1.12 // indirect + github.com/klauspost/cpuid/v2 v2.2.7 // indirect + github.com/leodido/go-urn v1.4.0 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect + github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect + github.com/modern-go/reflect2 v1.0.2 // indirect + github.com/pelletier/go-toml/v2 v2.2.2 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/twitchyliquid64/golang-asm v0.15.1 // indirect + github.com/ugorji/go/codec v1.2.12 // indirect + golang.org/x/arch v0.8.0 // indirect + golang.org/x/crypto v0.23.0 // indirect + golang.org/x/net v0.25.0 // indirect + golang.org/x/sys v0.20.0 // indirect + golang.org/x/text v0.15.0 // indirect + google.golang.org/protobuf v1.34.1 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..7f08abb --- /dev/null +++ b/go.sum @@ -0,0 +1,89 @@ +github.com/bytedance/sonic v1.11.6 h1:oUp34TzMlL+OY1OUWxHqsdkgC/Zfc85zGqw9siXjrc0= +github.com/bytedance/sonic v1.11.6/go.mod h1:LysEHSvpvDySVdC2f87zGWf6CIKJcAvqab1ZaiQtds4= +github.com/bytedance/sonic/loader v0.1.1 h1:c+e5Pt1k/cy5wMveRDyk2X4B9hF4g7an8N3zCYjJFNM= +github.com/bytedance/sonic/loader v0.1.1/go.mod h1:ncP89zfokxS5LZrJxl5z0UJcsk4M4yY2JpfqGeCtNLU= +github.com/cloudwego/base64x v0.1.4 h1:jwCgWpFanWmN8xoIUHa2rtzmkd5J2plF/dnLS6Xd/0Y= +github.com/cloudwego/base64x v0.1.4/go.mod h1:0zlkT4Wn5C6NdauXdJRhSKRlJvmclQ1hhJgA0rcu/8w= +github.com/cloudwego/iasm v0.2.0 h1:1KNIy1I1H9hNNFEEH3DVnI4UujN+1zjpuk6gwHLTssg= +github.com/cloudwego/iasm v0.2.0/go.mod h1:8rXZaNYT2n95jn+zTI1sDr+IgcD2GVs0nlbbQPiEFhY= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +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/gabriel-vasile/mimetype v1.4.3 h1:in2uUcidCuFcDKtdcBxlR0rJ1+fsokWf+uqxgUFjbI0= +github.com/gabriel-vasile/mimetype v1.4.3/go.mod h1:d8uq/6HKRL6CGdk+aubisF/M5GcPfT7nKyLpA0lbSSk= +github.com/gin-contrib/sse v0.1.0 h1:Y/yl/+YNO8GZSjAhjMsSuLt29uWRFHdHYUb5lYOV9qE= +github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm+fLHvGI= +github.com/gin-gonic/gin v1.10.0 h1:nTuyha1TYqgedzytsKYqna+DfLos46nTv2ygFy86HFU= +github.com/gin-gonic/gin v1.10.0/go.mod h1:4PMNQiOhvDRa013RKVbsiNwoyezlm2rm0uX/T7kzp5Y= +github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s= +github.com/go-playground/assert/v2 v2.2.0/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= +github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= +github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= +github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= +github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= +github.com/go-playground/validator/v10 v10.20.0 h1:K9ISHbSaI0lyB2eWMPJo+kOS/FBExVwjEviJTixqxL8= +github.com/go-playground/validator/v10 v10.20.0/go.mod h1:dbuPbCMFw/DrkbEynArYaCwl3amGuJotoKCe95atGMM= +github.com/goccy/go-json v0.10.2 h1:CrxCmQqYDkv1z7lO7Wbh2HN93uovUHgrECaO5ZrCXAU= +github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= +github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= +github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= +github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= +github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= +github.com/klauspost/cpuid/v2 v2.2.7 h1:ZWSB3igEs+d0qvnxR/ZBzXVmxkgt8DdzP6m9pfuVLDM= +github.com/klauspost/cpuid/v2 v2.2.7/go.mod h1:Lcz8mBdAVJIBVzewtcLocK12l3Y+JytZYpaMropDUws= +github.com/knz/go-libedit v1.10.1/go.mod h1:MZTVkCWyz0oBc7JOWP3wNAzd002ZbM/5hgShxwh4x8M= +github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= +github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= +github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= +github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= +github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M= +github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= +github.com/pelletier/go-toml/v2 v2.2.2 h1:aYUidT7k73Pcl9nb2gScu7NSrKCSHIDE89b3+6Wq+LM= +github.com/pelletier/go-toml/v2 v2.2.2/go.mod h1:1t835xjRzz80PqgE6HHgN2JOsmgYu/h4qDAS4n929Rs= +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/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/twitchyliquid64/golang-asm v0.15.1 h1:SU5vSMR7hnwNxj24w34ZyCi/FmDZTkS4MhqMhdFk5YI= +github.com/twitchyliquid64/golang-asm v0.15.1/go.mod h1:a1lVb/DtPvCB8fslRZhAngC2+aY1QWCk3Cedj/Gdt08= +github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE= +github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= +golang.org/x/arch v0.0.0-20210923205945-b76863e36670/go.mod h1:5om86z9Hs0C8fWVUuoMHwpExlXzs5Tkyp9hOrfG7pp8= +golang.org/x/arch v0.8.0 h1:3wRIsP3pM4yUptoR96otTUOXI367OS0+c9eeRi9doIc= +golang.org/x/arch v0.8.0/go.mod h1:FEVrYAQjsQXMVJ1nsMoVVXPZg6p2JE2mx8psSWTDQys= +golang.org/x/crypto v0.23.0 h1:dIJU/v2J8Mdglj/8rJ6UUOM3Zc9zLZxVZwwxMooUSAI= +golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8= +golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac= +golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= +golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/text v0.15.0 h1:h1V/4gjBv8v9cjcR6+AR5+/cIYK5N/WAgiv4xlsEtAk= +golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg= +google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +nullprogram.com/x/optparse v1.0.0/go.mod h1:KdyPE+Igbe0jQUrVfMqDMeJQIJZEuyV7pjYmp6pbG50= +rsc.io/pdf v0.1.1/go.mod h1:n8OzWcQ6Sp37PL01nO98y4iUCRdTGarVfzxY20ICaU4= diff --git a/main.go b/main.go deleted file mode 100644 index 49f4dee..0000000 --- a/main.go +++ /dev/null @@ -1,7 +0,0 @@ -package main - -import "fmt" - -func main() { - fmt.Println("Hello, Bounty Hunter!") -} From 0230a70d94553d2884fd82141d8cf7db7237853d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 23:29:31 +0000 Subject: [PATCH 2/2] chore: use the upstream module path This branch targets madalynerlge2/gin, so the module is named for that repository rather than the fork hosting the branch. Updates the import in cmd/demo and the README example to match. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LehYDiBQajTGivEPN7umCT --- README.md | 2 +- cmd/demo/main.go | 2 +- go.mod | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 13c2920..50bd5d8 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ backs it stays held until the request is torn down. ```go import ( "github.com/gin-gonic/gin" - ginauth "github.com/jpka/gin" + ginauth "github.com/madalynerlge2/gin" ) router := gin.Default() diff --git a/cmd/demo/main.go b/cmd/demo/main.go index 6798d40..71d0dcd 100644 --- a/cmd/demo/main.go +++ b/cmd/demo/main.go @@ -16,7 +16,7 @@ import ( "github.com/gin-gonic/gin" - ginauth "github.com/jpka/gin" + ginauth "github.com/madalynerlge2/gin" ) func main() { diff --git a/go.mod b/go.mod index 300eb82..3c4b9ae 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/jpka/gin +module github.com/madalynerlge2/gin go 1.21