Fix DedupeSessionCookies panic with pgxstore (use FindCtx)
Production uses pgxstore, whose plain scs.Store.Find deliberately panics
("missing context arg") — only FindCtx works. DedupeSessionCookies called
Store.Find directly, so any request carrying duplicate session cookies
500'd (chi Recoverer caught the panic). Unit tests passed because the
in-memory store implements plain Find.
Mirror scs.doStoreFind: type-assert FindCtx(context.Context, string) and
use it with the request context, falling back to plain Find for simple
stores. Regression test adds a ctx-only store whose plain methods panic.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -377,8 +377,20 @@ func (s *Server) DedupeSessionCookies(next http.Handler) http.Handler {
|
|||||||
|
|
||||||
chosen := ""
|
chosen := ""
|
||||||
if s.Sessions != nil && s.Sessions.Store != nil {
|
if s.Sessions != nil && s.Sessions.Store != nil {
|
||||||
|
// Mirror scs.doStoreFind: context-aware stores (pgxstore) implement
|
||||||
|
// FindCtx and their plain Find PANICS ("missing context arg").
|
||||||
|
find := func(tok string) bool {
|
||||||
|
if cs, ok := s.Sessions.Store.(interface {
|
||||||
|
FindCtx(context.Context, string) ([]byte, bool, error)
|
||||||
|
}); ok {
|
||||||
|
_, found, err := cs.FindCtx(r.Context(), tok)
|
||||||
|
return err == nil && found
|
||||||
|
}
|
||||||
|
_, found, err := s.Sessions.Store.Find(tok)
|
||||||
|
return err == nil && found
|
||||||
|
}
|
||||||
for _, tok := range tokens {
|
for _, tok := range tokens {
|
||||||
if _, found, err := s.Sessions.Store.Find(tok); err == nil && found {
|
if find(tok) {
|
||||||
chosen = tok
|
chosen = tok
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package httpapi
|
package httpapi
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -10,6 +11,27 @@ import (
|
|||||||
"github.com/descrybe/descrybe-v2/apps/api/internal/config"
|
"github.com/descrybe/descrybe-v2/apps/api/internal/config"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// ctxOnlyStore mimics pgxstore: the plain scs.Store methods PANIC and only the
|
||||||
|
// *Ctx variants work. DedupeSessionCookies must use FindCtx for such stores.
|
||||||
|
type ctxOnlyStore struct {
|
||||||
|
tokens map[string]bool
|
||||||
|
}
|
||||||
|
|
||||||
|
func (s *ctxOnlyStore) Find(string) ([]byte, bool, error) { panic("missing context arg") }
|
||||||
|
func (s *ctxOnlyStore) Commit(string, []byte, time.Time) error { panic("missing context arg") }
|
||||||
|
func (s *ctxOnlyStore) Delete(string) error { panic("missing context arg") }
|
||||||
|
func (s *ctxOnlyStore) FindCtx(_ context.Context, token string) ([]byte, bool, error) {
|
||||||
|
return []byte("x"), s.tokens[token], nil
|
||||||
|
}
|
||||||
|
func (s *ctxOnlyStore) CommitCtx(_ context.Context, token string, _ []byte, _ time.Time) error {
|
||||||
|
s.tokens[token] = true
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
func (s *ctxOnlyStore) DeleteCtx(_ context.Context, token string) error {
|
||||||
|
delete(s.tokens, token)
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
func dedupeTestServer(t *testing.T) (*Server, *scs.SessionManager) {
|
func dedupeTestServer(t *testing.T) (*Server, *scs.SessionManager) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
sm := scs.New() // in-memory store
|
sm := scs.New() // in-memory store
|
||||||
@@ -71,6 +93,30 @@ func TestDedupeSessionCookies_staleShadowsValid(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDedupeSessionCookies_ctxOnlyStoreNoPanic(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
// Regression: production pgxstore panics on plain Find ("missing context
|
||||||
|
// arg") — the middleware must go through FindCtx.
|
||||||
|
s, sm := dedupeTestServer(t)
|
||||||
|
sm.Store = &ctxOnlyStore{tokens: map[string]bool{"valid-token": true}}
|
||||||
|
|
||||||
|
var seen string
|
||||||
|
h := s.DedupeSessionCookies(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
c, err := r.Cookie("descrybe_session")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("session cookie missing downstream: %v", err)
|
||||||
|
}
|
||||||
|
seen = c.Value
|
||||||
|
}))
|
||||||
|
req := httptest.NewRequest(http.MethodGet, "/api/auth/me", nil)
|
||||||
|
req.Header.Set("Cookie", "descrybe_session=stale-relic; descrybe_session=valid-token")
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
h.ServeHTTP(rec, req) // must not panic
|
||||||
|
if seen != "valid-token" {
|
||||||
|
t.Fatalf("downstream token=%q want valid-token", seen)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestDedupeSessionCookies_singleCookieUntouched(t *testing.T) {
|
func TestDedupeSessionCookies_singleCookieUntouched(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
s, _ := dedupeTestServer(t)
|
s, _ := dedupeTestServer(t)
|
||||||
|
|||||||
Reference in New Issue
Block a user