mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-10 16:04:04 +00:00
feat(multiplayer): reject outdated clients with distinct messaging
Closes §8.43's 'version-mismatch-specific client messaging' gap. Per the user's explicit go-ahead to design new server behavior for this (rather than only wiring up something that already existed, the pattern every other fix this session followed): before this, there was no server-side protocol rejection at all. queue_create accepted any protocol_version >= 1 unconditionally, so an outdated client could only ever discover a mismatch by waiting in the queue forever unmatched -- the matcher's own compatibility check requires every formed player to share an identical protocol_version -- with no error and no explanation given to the player. Server: Service.MinProtocolVersion (opt-in, zero by default so every existing caller keeps accepting protocol_version 1 unconditionally) rejects a below-floor queue_create with 426 Upgrade Required / client_outdated before the request ever reaches the candidate provider. Wired via cmd/control-plane's new --min-protocol-version flag (validated non-negative at startup). Client: ControlPlaneClient recognises HTTPClient.RESPONSE_UPGRADE_REQUIRED on queue_create specifically and sets a distinct 'Your client is out of date -- please update to continue searching' message instead of the server's raw generic error string, and clears _last_queue_create so can_retry_queue_create() never offers 'Retry Search' for a failure that retrying with the same build can never fix. Verified: go build/vet/test -race clean across every server package. TestQueueCreateEnforcesMinProtocolVersion covers below-floor rejection (candidate provider never reached), exactly-at-floor acceptance, and the error body naming client_outdated; TestQueueCreateMinProtocolVersionZeroIsDisabled proves the opt-in default doesn't change behavior for every existing caller. Godot: test_outdated_client_receives_a_distinct_message_and_no_retry_offer proves the distinct message and suppressed retry offer. Full Godot suite (217/217, 0 failed, no crash), full make verify-multiplayer-local gate, zero new crash reports.
This commit is contained in:
@@ -502,6 +502,13 @@ func _on_request_completed(result: HTTPRequest.Result, response_code: int, _head
|
||||
state.expire("Queue ticket expired")
|
||||
elif response_code == HTTPClient.RESPONSE_SERVICE_UNAVAILABLE:
|
||||
state.set_notice("Matchmaking is temporarily unavailable; retrying is safe")
|
||||
elif response_code == HTTPClient.RESPONSE_UPGRADE_REQUIRED and operation == "queue_create":
|
||||
# Distinct from the generic queue_create failure below: retrying
|
||||
# with the same client build can never succeed, so the retry
|
||||
# offer must not be shown (can_retry_queue_create() checks
|
||||
# _last_queue_create; clearing it here suppresses "Retry Search").
|
||||
_last_queue_create = {}
|
||||
state.fail("Your client is out of date -- please update to continue searching")
|
||||
elif operation == "ranked_profile":
|
||||
ranked_profile.set_error(detail)
|
||||
elif response_code == HTTPClient.RESPONSE_NOT_FOUND and (operation == "queue_recover" or operation == "proposal_recover"):
|
||||
|
||||
@@ -338,6 +338,24 @@ func test_queue_conflict_response_handler_defers_ticket_recovery() -> void:
|
||||
client.free()
|
||||
|
||||
|
||||
# Covers §8.43's "version-mismatch-specific client messaging": a 426 Upgrade
|
||||
# Required on queue_create (the server-side floor added alongside this test)
|
||||
# must surface a distinct, actionable message rather than the server's raw
|
||||
# generic error string, and must not offer a futile "Retry Search" -- the
|
||||
# same client build will fail again identically every time.
|
||||
func test_outdated_client_receives_a_distinct_message_and_no_retry_offer() -> void:
|
||||
var client := ControlPlaneClient.new()
|
||||
client._ready()
|
||||
assert_true(client.configure("https://match.example", "session-id:opaque-token"), "client configures")
|
||||
client._operation = "queue_create"
|
||||
client._last_queue_create = {"ticket_id": "ticket-outdated", "playlist": "casual", "client_build": "build-1", "protocol_version": 4, "key": "outdated-key-123456"}
|
||||
client._on_request_completed(HTTPRequest.RESULT_SUCCESS, HTTPClient.RESPONSE_UPGRADE_REQUIRED, PackedStringArray(), JSON.stringify({"error": "client_outdated"}).to_utf8_buffer())
|
||||
assert_eq(client.state.phase, MatchmakingState.FAILED, "outdated client fails the search")
|
||||
assert_true(client.state.message.to_lower().contains("update"), "message tells the player to update rather than repeating the raw server error: %s" % client.state.message)
|
||||
assert_true(not client.can_retry_queue_create(), "retrying with the same outdated client build is never offered")
|
||||
client.free()
|
||||
|
||||
|
||||
func test_rest_responses_reject_malformed_resource_identifiers() -> void:
|
||||
var client := ControlPlaneClient.new()
|
||||
client._ready()
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
@@ -136,6 +136,15 @@ type Service struct {
|
||||
ClientIPs *ClientIPResolver
|
||||
Admission AdmissionController
|
||||
ReadinessCheck ReadinessCheck
|
||||
// MinProtocolVersion, when positive, is the floor below which queue_create
|
||||
// is refused outright with 426 Upgrade Required rather than silently
|
||||
// queueing a client the matcher can never actually pair with anyone (its
|
||||
// own compatibility check requires every formed player to share an
|
||||
// identical protocol_version -- an outdated client below every other
|
||||
// player's version would otherwise wait forever with no explanation).
|
||||
// Zero (the default) disables the floor entirely, preserving the prior
|
||||
// permissive behavior for callers that never set it.
|
||||
MinProtocolVersion int
|
||||
// Log receives a credential-safe structured event for lifecycle-relevant
|
||||
// reads and mutations. Nil
|
||||
// is a valid, silent no-op -- every call site must stay optional so
|
||||
@@ -414,6 +423,11 @@ func (s *Service) queueCreate(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusBadRequest, "invalid_request")
|
||||
return
|
||||
}
|
||||
if s.MinProtocolVersion > 0 && input.ProtocolVersion < s.MinProtocolVersion {
|
||||
s.logEvent(observability.Event{Event: "queue_create", QueueID: input.TicketID, Stage: "outdated_client", OccurredAt: s.now(), Fields: map[string]any{"protocol_version": input.ProtocolVersion, "min_protocol_version": s.MinProtocolVersion}})
|
||||
writeError(w, http.StatusUpgradeRequired, "client_outdated")
|
||||
return
|
||||
}
|
||||
key := r.Header.Get("Idempotency-Key")
|
||||
if len(key) < 16 || len(key) > 128 {
|
||||
writeError(w, http.StatusBadRequest, "invalid_idempotency_key")
|
||||
|
||||
@@ -767,6 +767,98 @@ func TestQueueCreateRequiresCompatibilityMetadataAndPassesItToProvider(t *testin
|
||||
}
|
||||
}
|
||||
|
||||
// TestQueueCreateEnforcesMinProtocolVersion covers the gap multiplayer-next.md
|
||||
// 8.43 named "version-mismatch-specific client messaging": before this,
|
||||
// queue_create accepted any protocol_version >= 1 unconditionally, so an
|
||||
// outdated client below every other queued player's version would simply
|
||||
// queue forever with no error at all -- the matcher's own compatibility
|
||||
// check requires every formed player to share an identical protocol_version,
|
||||
// so it could never be paired, and nothing ever told it why.
|
||||
func TestQueueCreateEnforcesMinProtocolVersion(t *testing.T) {
|
||||
now := time.Unix(1000, 0).UTC()
|
||||
sessions := domain.NewSessionStore()
|
||||
session, token, err := sessions.Issue("player-1", time.Hour, now)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
calls := 0
|
||||
service := &Service{
|
||||
Sessions: sessions,
|
||||
Queue: domain.NewQueue(),
|
||||
Now: func() time.Time { return now },
|
||||
MinProtocolVersion: 5,
|
||||
CandidateV2: func(_ string, ticketID string, spec domain.QueueSpec) (domain.Candidate, error) {
|
||||
calls++
|
||||
return domain.Candidate{PlayerID: "player-1", TicketID: ticketID, Playlist: spec.Playlist, ClientBuild: spec.ClientBuild, ProtocolVersion: spec.ProtocolVersion, EnqueuedAt: now}, nil
|
||||
},
|
||||
}
|
||||
server := httptest.NewServer(service.Handler())
|
||||
defer server.Close()
|
||||
request := func(body string) (*http.Response, string) {
|
||||
req, _ := http.NewRequest(http.MethodPost, server.URL+"/v1/queue", strings.NewReader(body))
|
||||
req.Header.Set("Authorization", "Bearer "+session.SessionID+":"+token)
|
||||
req.Header.Set("Idempotency-Key", "create-key-123456")
|
||||
response, requestErr := http.DefaultClient.Do(req)
|
||||
if requestErr != nil {
|
||||
t.Fatal(requestErr)
|
||||
}
|
||||
decoded, _ := io.ReadAll(response.Body)
|
||||
response.Body.Close()
|
||||
return response, string(decoded)
|
||||
}
|
||||
response, body := request(`{"ticket_id":"ticket-1","playlist":"casual","client_build":"build-1","protocol_version":4}`)
|
||||
if response.StatusCode != http.StatusUpgradeRequired {
|
||||
t.Fatalf("below-floor status = %d, want 426 Upgrade Required; body=%s", response.StatusCode, body)
|
||||
}
|
||||
if !strings.Contains(body, "client_outdated") {
|
||||
t.Fatalf("below-floor body does not name the outdated-client error: %s", body)
|
||||
}
|
||||
if calls != 0 {
|
||||
t.Fatalf("candidate provider must not be reached for a rejected below-floor request, calls=%d", calls)
|
||||
}
|
||||
response, _ = request(`{"ticket_id":"ticket-1","playlist":"casual","client_build":"build-1","protocol_version":5}`)
|
||||
if response.StatusCode != http.StatusCreated {
|
||||
t.Fatalf("exactly-at-floor status = %d, want 201", response.StatusCode)
|
||||
}
|
||||
if calls != 1 {
|
||||
t.Fatalf("exactly-at-floor request should reach the provider once, calls=%d", calls)
|
||||
}
|
||||
}
|
||||
|
||||
// TestQueueCreateMinProtocolVersionZeroIsDisabled proves the floor is opt-in:
|
||||
// every existing Service literal across the codebase that never sets
|
||||
// MinProtocolVersion must keep accepting protocol_version 1 exactly as
|
||||
// before, unconditionally.
|
||||
func TestQueueCreateMinProtocolVersionZeroIsDisabled(t *testing.T) {
|
||||
now := time.Unix(1000, 0).UTC()
|
||||
sessions := domain.NewSessionStore()
|
||||
session, token, err := sessions.Issue("player-1", time.Hour, now)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
service := &Service{
|
||||
Sessions: sessions,
|
||||
Queue: domain.NewQueue(),
|
||||
Now: func() time.Time { return now },
|
||||
CandidateV2: func(_ string, ticketID string, spec domain.QueueSpec) (domain.Candidate, error) {
|
||||
return domain.Candidate{PlayerID: "player-1", TicketID: ticketID, Playlist: spec.Playlist, ClientBuild: spec.ClientBuild, ProtocolVersion: spec.ProtocolVersion, EnqueuedAt: now}, nil
|
||||
},
|
||||
}
|
||||
server := httptest.NewServer(service.Handler())
|
||||
defer server.Close()
|
||||
req, _ := http.NewRequest(http.MethodPost, server.URL+"/v1/queue", strings.NewReader(`{"ticket_id":"ticket-1","playlist":"casual","client_build":"build-1","protocol_version":1}`))
|
||||
req.Header.Set("Authorization", "Bearer "+session.SessionID+":"+token)
|
||||
req.Header.Set("Idempotency-Key", "create-key-123456")
|
||||
response, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer response.Body.Close()
|
||||
if response.StatusCode != http.StatusCreated {
|
||||
t.Fatalf("status = %d, want 201 with MinProtocolVersion left at its zero default", response.StatusCode)
|
||||
}
|
||||
}
|
||||
|
||||
func TestQueueCreateRejectsCandidateMetadataMismatch(t *testing.T) {
|
||||
now := time.Unix(1000, 0).UTC()
|
||||
sessions := domain.NewSessionStore()
|
||||
|
||||
@@ -34,6 +34,7 @@ func main() {
|
||||
rateWindow := flag.Duration("rate-limit-window", time.Minute, "fixed window for the per-replica request limiter")
|
||||
rateMaxKeys := flag.Int("rate-limit-max-keys", 10000, "maximum credential/IP keys retained by the per-replica request limiter")
|
||||
trustedProxyCIDRs := flag.String("trusted-proxy-cidrs", os.Getenv("COSMIC_CLASH_TRUSTED_PROXY_CIDRS"), "comma-separated immediate proxy CIDRs allowed to supply X-Forwarded-For")
|
||||
minProtocolVersion := flag.Int("min-protocol-version", 0, "reject queue_create below this protocol_version with 426 Upgrade Required instead of queueing a client the matcher can never pair with anyone; zero disables the floor")
|
||||
flag.Parse()
|
||||
if *role != "api" {
|
||||
fatalf("unsupported role %q (only api is implemented)", *role)
|
||||
@@ -44,6 +45,9 @@ func main() {
|
||||
if *redisTTL <= 0 {
|
||||
fatalf("--redis-ttl must be positive")
|
||||
}
|
||||
if *minProtocolVersion < 0 {
|
||||
fatalf("--min-protocol-version must be non-negative")
|
||||
}
|
||||
rateLimiter, err := api.NewRateLimiter(*rateLimit, *rateWindow, *rateMaxKeys)
|
||||
if err != nil {
|
||||
fatalf("invalid request limiter configuration: %v", err)
|
||||
@@ -78,6 +82,7 @@ func main() {
|
||||
service := newAPIService(db, *workloadSecret, candidateIndex)
|
||||
service.RateLimiter = rateLimiter
|
||||
service.ClientIPs = clientIPs
|
||||
service.MinProtocolVersion = *minProtocolVersion
|
||||
admission := api.NewAdmissionGate(*degraded)
|
||||
service.Admission = admission
|
||||
server := &http.Server{Addr: *listen, Handler: service.Handler(), ReadHeaderTimeout: 5 * time.Second}
|
||||
|
||||
Reference in New Issue
Block a user