diff --git a/internal/api/handlers_hasjoined.go b/internal/api/handlers_hasjoined.go index 0b20303..762a9be 100644 --- a/internal/api/handlers_hasjoined.go +++ b/internal/api/handlers_hasjoined.go @@ -230,7 +230,7 @@ var mojangProfileAPI = "https://api.mojang.com/users/profiles/minecraft/" // profileHTTPClient is deliberately more impatient than authHTTPClient: the name lookup is a // SECOND Mojang round-trip on a third-party login (the identity leg already spent one), and // api.mojang.com is exactly what is unreliable from the networks these servers sit on. A -// slow answer falls back to the cache instead of holding the login open. +// slow answer counts as taken instead of holding the login open. var profileHTTPClient = &http.Client{Timeout: 2 * time.Second, Transport: upstreamTransport} // A name's premium status changes on human timescales, not per login, so it is cached — but @@ -256,11 +256,11 @@ var premiumNames = struct { }{m: make(map[string]premiumEntry)} // isPremiumName reports whether username belongs to a real Mojang account — which is what -// makes a third-party player holding it a squatter. On a lookup failure it prefers a stale -// cached answer, and with nothing cached it fails CLOSED (assume premium → rename the -// third-party player): a Mojang outage must not let a squatter keep a name the real owner is -// about to log in with. Being wrong that way costs a cosmetic prefix; being wrong the other -// way bounces the name's actual owner off the proxy. +// makes a third-party player holding it a squatter. A lookup failure fails CLOSED (assume +// premium → rename the third-party player), even over an expired "free": the name may have +// been bought since, and a Mojang outage must not let a squatter keep it. Being wrong that +// way costs a cosmetic prefix; being wrong the other way bounces the name's actual owner off +// the proxy. func isPremiumName(ctx context.Context, username string) bool { key := strings.ToLower(username) @@ -273,9 +273,6 @@ func isPremiumName(ctx context.Context, username string) bool { taken, err := lookupPremiumName(ctx, username) if err != nil { - if hit { - return cached.taken - } return true } diff --git a/internal/api/handlers_hasjoined_test.go b/internal/api/handlers_hasjoined_test.go index f3b0f40..8a9a585 100644 --- a/internal/api/handlers_hasjoined_test.go +++ b/internal/api/handlers_hasjoined_test.go @@ -478,6 +478,16 @@ func TestPremiumNameCache(t *testing.T) { } }) + // The name may have been bought since it was last seen free, and its new owner is who a + // stale "free" would lock out for as long as the lookups keep failing. + t.Run("an expired free answer does not outlive an outage", func(t *testing.T) { + profileAPIAnswering(t, http.StatusTooManyRequests) + seedPremium("Steve0", false, premiumFreeTTL+time.Minute) + if !isPremiumName(ctx, "Steve0") { + t.Fatal("stale free answer trusted during an outage; a squatter keeps a just-bought name") + } + }) + t.Run("the cache is cleared rather than grown past its bound", func(t *testing.T) { profileAPIAnswering(t, http.StatusNotFound) for i := range premiumCacheMax {