Skip to content

Commit 1eaff6d

Browse files
committed
registry: don't panic when store.New returns a nil session
Stores can legitimately return (nil, err) from New (e.g. backend unavailable). Registry.Get then dereferenced the nil session for session.name and session.store and crashed the request. Return the store's error instead. Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
1 parent bb4cd60 commit 1eaff6d

2 files changed

Lines changed: 49 additions & 0 deletions

File tree

sessions.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,12 @@ func (s *Registry) Get(store Store, name string) (session *Session, err error) {
137137
session, err = info.s, info.e
138138
} else {
139139
session, err = store.New(s.request, name)
140+
if session == nil {
141+
// Some stores return (nil, err) when initialization fails;
142+
// surface the error rather than panicking on session.name.
143+
s.sessions[name] = sessionInfo{s: nil, e: err}
144+
return nil, err
145+
}
140146
session.name = name
141147
s.sessions[name] = sessionInfo{s: session, e: err}
142148
}

sessions_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,49 @@ func TestCookieStoreMapPanic(t *testing.T) {
214214
}
215215
}
216216

217+
// nilSessionStore returns (nil, err) from New, which the registry used
218+
// to dereference unconditionally and panic on.
219+
type nilSessionStore struct{}
220+
221+
func (nilSessionStore) Get(_ *http.Request, _ string) (*Session, error) {
222+
return nil, errSentinel
223+
}
224+
225+
func (nilSessionStore) New(_ *http.Request, _ string) (*Session, error) {
226+
return nil, errSentinel
227+
}
228+
229+
func (nilSessionStore) Save(_ *http.Request, _ http.ResponseWriter, _ *Session) error {
230+
return errSentinel
231+
}
232+
233+
var errSentinel = stringError("store unavailable")
234+
235+
type stringError string
236+
237+
func (e stringError) Error() string { return string(e) }
238+
239+
func TestRegistryGetReturnsErrorWhenStoreReturnsNilSession(t *testing.T) {
240+
defer func() {
241+
if r := recover(); r != nil {
242+
t.Fatalf("Registry.Get panicked when store returned nil: %v", r)
243+
}
244+
}()
245+
246+
req, err := http.NewRequest("GET", "http://www.example.com", nil)
247+
if err != nil {
248+
t.Fatal(err)
249+
}
250+
reg := GetRegistry(req)
251+
sess, err := reg.Get(nilSessionStore{}, "name")
252+
if err == nil {
253+
t.Fatal("expected error from Registry.Get, got nil")
254+
}
255+
if sess != nil {
256+
t.Fatalf("expected nil session, got %+v", sess)
257+
}
258+
}
259+
217260
func init() {
218261
gob.Register(FlashMessage{})
219262
}

0 commit comments

Comments
 (0)