diff --git a/sessions.go b/sessions.go index c052b28..97b8237 100644 --- a/sessions.go +++ b/sessions.go @@ -137,6 +137,12 @@ func (s *Registry) Get(store Store, name string) (session *Session, err error) { session, err = info.s, info.e } else { session, err = store.New(s.request, name) + // store.New may return a nil session together with an error. Guard + // against dereferencing it (previously this panicked with a nil deref + // when a store's New returned (nil, err)). + if session == nil { + return nil, err + } session.name = name s.sessions[name] = sessionInfo{s: session, e: err} } diff --git a/sessions_get_error_test.go b/sessions_get_error_test.go new file mode 100644 index 0000000..9cf677c --- /dev/null +++ b/sessions_get_error_test.go @@ -0,0 +1,39 @@ +package sessions + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// storeReturningError is a Store whose New always fails, returning a nil +// session together with an error. +type storeReturningError struct{} + +func (storeReturningError) Get(r *http.Request, name string) (*Session, error) { + return nil, http.ErrNoCookie +} + +func (storeReturningError) New(r *http.Request, name string) (*Session, error) { + return nil, http.ErrNoCookie +} + +func (storeReturningError) Save(r *http.Request, w http.ResponseWriter, s *Session) error { + return nil +} + +// TestRegistryGet_StoreNewError verifies that Registry.Get does not panic with +// a nil-pointer dereference when a store's New returns (nil, err); it should +// surface the error instead. +func TestRegistryGet_StoreNewError(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + reg := GetRegistry(req) + + session, err := reg.Get(storeReturningError{}, "session-name") + if err == nil { + t.Fatal("expected an error from Get when store.New fails") + } + if session != nil { + t.Fatalf("expected a nil session when store.New fails, got %#v", session) + } +}