From a4779186a4f11eccd70ee9ce5c24c3c2047c28ce Mon Sep 17 00:00:00 2001 From: Minseong Choi Date: Tue, 22 Sep 2026 13:19:55 +0900 Subject: [PATCH] fix(nano): refuse hasJoined requests that declare a body A GET to hasJoined with a Content-Length and no body held its connection indefinitely. The handler returned, but net/http tries to drain an unread body before it writes the answer, and nothing bounds that wait: ReadHeaderTimeout ends with the headers. One such request per socket pins a goroutine and a descriptor on nano or on felis-api's internal face. Velocity never sends a body, so any request that declares one, including a chunked one, now gets a 400 with Connection: close, which skips the drain and releases the connection once the answer is written. The new subtest writes that request over a raw socket and waits three seconds for an answer. Before the change it times out with no response at all; now it reads a 400 marked close. --- internal/api/handlers_hasjoined.go | 9 +++++++++ internal/api/handlers_hasjoined_test.go | 27 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/internal/api/handlers_hasjoined.go b/internal/api/handlers_hasjoined.go index b556b9e..7454ebe 100644 --- a/internal/api/handlers_hasjoined.go +++ b/internal/api/handlers_hasjoined.go @@ -106,6 +106,15 @@ func HasJoinedHandler(sources []AuthSource, repo Repo) http.Handler { // service token. A rejected login is 204 No Content — exactly what Mojang returns for an // invalid session, which authlib maps to "failed to verify username". func (a *API) handleHasJoined(w http.ResponseWriter, r *http.Request) { + // Velocity sends no body. When a request declares one anyway, net/http tries to drain it + // before writing any answer, so one that never arrives holds the connection with no + // timeout: ReadHeaderTimeout stopped at the headers. Marking the reply as the last one on + // this connection skips the drain. + if r.ContentLength != 0 { + w.Header().Set("Connection", "close") + w.WriteHeader(http.StatusBadRequest) + return + } q := r.URL.Query() username, serverID := q.Get("username"), q.Get("serverId") if username == "" || serverID == "" { diff --git a/internal/api/handlers_hasjoined_test.go b/internal/api/handlers_hasjoined_test.go index 6e7c556..459e080 100644 --- a/internal/api/handlers_hasjoined_test.go +++ b/internal/api/handlers_hasjoined_test.go @@ -1,14 +1,18 @@ package api import ( + "bufio" "encoding/hex" "encoding/json" + "io" + "net" "net/http" "net/http/httptest" "path" "strings" "sync/atomic" "testing" + "time" "github.com/google/uuid" ) @@ -311,6 +315,29 @@ func TestHasJoined(t *testing.T) { } }) + // A GET that declares a body it never sends must still be answered and lose its + // connection; otherwise each such socket stays open for as long as the client likes. + t.Run("request declaring a body is refused and closed", func(t *testing.T) { + api := newTestAPI(newFakeRepo(), newFakeCluster()) + srv := httptest.NewServer(api.InternalHandler()) + t.Cleanup(srv.Close) + conn, err := net.Dial("tcp", srv.Listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + _, _ = io.WriteString(conn, "GET /session/minecraft/hasJoined?username=a&serverId=b HTTP/1.1\r\nHost: x\r\nContent-Length: 1000\r\n\r\n") + _ = conn.SetReadDeadline(time.Now().Add(3 * time.Second)) + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + if err != nil { + t.Fatalf("no answer while the declared body never arrives: %v", err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusBadRequest || !resp.Close { + t.Fatalf("code = %d close = %v, want 400 with Connection: close", resp.StatusCode, resp.Close) + } + }) + // Missing query fields → 204 without touching any source. t.Run("missing username -> 204", func(t *testing.T) { api := newTestAPI(newFakeRepo(), newFakeCluster())