fix: close request body when BasicAuth rejects a request

[jpka]

/claim #1

Fixes the request-body leak described in issue #1: when the Basic auth middleware aborts an unauthenticated request, c.Request.Body was left open, so whatever backs it stayed held until the request was torn down.

The fix

Close the body on the rejection path, before aborting:

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 — a panic inside auth middleware would be a far worse outcome than a body left open. It 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 normally.

No manual drain before the Close

The previous attempt at this fix called io.CopyN(io.Discard, body, 4096) before closing. That's removed. net/http sets doEarlyClose on server request bodies (server.go, in conn.readRequest), which already makes Body.Close drain up to maxPostHandlerReadBytes (256 KiB) looking for EOF and leave the connection reusable — and flag the body so the server closes the connection when more than that is still pending, rather than reusing a desynchronised one. The hand-rolled 4096-byte drain duplicated that logic less correctly. Close is also idempotent, so the server's own Close after the handler returns is a no-op.

Why the diff is larger than the fix

Nothing in the repository compiled before this change:

• no go.mod at all
• auth.go referenced Context and HandlerFunc, defined nowhere in the repo
• auth_test.go called processAccounts and authorizationHeader, which did not exist
• encoding/base64 was imported but unused
• auth.go (package gin) shared a directory with main.go (package main)

So the middleware now builds against gin itself as a drop-in replacement for gin.BasicAuth — BasicAuth, BasicAuthForRealm, Accounts and AuthUserKey mirror gin's API, so an existing call site swaps over unchanged. The starter program moved to cmd/demo, which serves as a manual check.

Tests

go test -race ./... passes; gofmt and go vet are clean.

• Rejection path — body closed exactly once, across wrong passwords, unknown users, and missing/malformed/wrong-scheme headers
• Success path — body left open and still readable by the handler
• Nil-safety — nil context, nil Request, nil Body, http.NoBody
• Failing Close — still yields a clean 401, no panic
• Keep-Alive — a real server over a real socket, asserting via httptrace that three consecutive rejected requests carrying bodies reuse one connection ([false, true, true]), plus a 1 MiB over-the-drain-limit case
• Concurrency — the rejection path under 64 goroutines, under -race

Reverting just the closeRequestBody call makes the four body-close tests fail, so they genuinely bind to the fix. The Keep-Alive tests pass either way by design — they are no-regression guards.

One note on the premise

Worth being straight about scope. Under a plain net/http server the leak is bounded: the server closes the request body itself once the handler returns, so an unclosed body is held for the rest of that request rather than indefinitely, and the CLOSE_WAIT growth described in the issue is more likely to come from clients or proxies that never read the 401 response. Closing early is still worth doing — it shortens the hold time under load, and it is the only thing that releases bodies wrapping a resource of their own (a decompressor, a temp file, a metered reader). The README documents this rather than overstating the impact.

[opirebot]

👀 We've notified the reward creators here.
Make sure your payment account is ready to receive the payment for your hard work 💪