Skip to content

fix: avoid nil-pointer panic when store.New returns an error - #298

Open
piyush295 wants to merge 1 commit into
gorilla:mainfrom
piyush295:fix/get-nil-session-panic
Open

piyush295 wants to merge 1 commit into
gorilla:mainfrom
piyush295:fix/get-nil-session-panic

Conversation

@piyush295

Copy link
Copy Markdown

Fixes #288.

Current behavior

Registry.Get calls store.New(...) but does not check its result before use:

session, err = store.New(s.request, name)
session.name = name   // nil-pointer dereference if store.New returned (nil, err)

A Store whose New returns (nil, err) — which the interface permits — causes a nil-pointer panic at session.name = name (and again at session.store = store), instead of the error being returned to the caller.

Fix

Guard against a nil session: if store.New returns nil, return the error immediately rather than dereferencing it.

session, err = store.New(s.request, name)
if session == nil {
    return nil, err
}
session.name = name
...

Testing

  • Added TestRegistryGet_StoreNewError: a store whose New returns (nil, err) now yields a nil session + error from Get instead of panicking.
  • go test . — full suite passes; gofmt clean; go build . clean.

Registry.Get did not check the result of store.New before dereferencing it.
When a Store's New returned (nil, err), the next line (session.name = name)
panicked with a nil-pointer dereference instead of returning the error.

Guard against a nil session: if store.New returns nil, return the error to
the caller. Adds a regression test with a store whose New always fails.

Fixes gorilla#288
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Panic if store.New returns an error

1 participant