Skip to content

Commit

Permalink
Use golang context pkg instead of gorilla/context to fix memory leaks (
Browse files Browse the repository at this point in the history
…#175)

* - use golang context pkg instead of gorilla/context to fix memory leaks
* - add test case for checking request context content upon shallow copy
* - update docs, readme.md and travis.yml
  • Loading branch information
secracon authored and elithrar committed Dec 8, 2018
1 parent 726776d commit 12bd476
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 28 deletions.
4 changes: 0 additions & 4 deletions .travis.yml
Expand Up @@ -3,10 +3,6 @@ sudo: false

matrix:
include:
- go: 1.3.x
- go: 1.4.x
- go: 1.5.x
- go: 1.6.x
- go: 1.7.x
- go: 1.8.x
- go: 1.9.x
Expand Down
12 changes: 0 additions & 12 deletions README.md
Expand Up @@ -51,18 +51,6 @@ secret key used to authenticate the session. Inside the handler, we call
some session values in session.Values, which is a `map[interface{}]interface{}`.
And finally we call `session.Save()` to save the session in the response.

Important Note: If you aren't using gorilla/mux, you need to wrap your handlers
with
[`context.ClearHandler`](https://www.gorillatoolkit.org/pkg/context#ClearHandler)
or else you will leak memory! An easy way to do this is to wrap the top-level
mux when calling http.ListenAndServe:

```go
http.ListenAndServe(":8080", context.ClearHandler(http.DefaultServeMux))
```

The ClearHandler function is provided by the gorilla/context package.

More examples are available [on the Gorilla
website](https://www.gorillatoolkit.org/pkg/sessions).

Expand Down
8 changes: 0 additions & 8 deletions doc.go
Expand Up @@ -59,14 +59,6 @@ session.Save(r, w), and either display an error message or otherwise handle it.
Save must be called before writing to the response, otherwise the session
cookie will not be sent to the client.
Important Note: If you aren't using gorilla/mux, you need to wrap your handlers
with context.ClearHandler as or else you will leak memory! An easy way to do this
is to wrap the top-level mux when calling http.ListenAndServe:
http.ListenAndServe(":8080", context.ClearHandler(http.DefaultServeMux))
The ClearHandler function is provided by the gorilla/context package.
That's all you need to know for the basic usage. Let's take a look at other
options, starting with flash messages.
Expand Down
8 changes: 4 additions & 4 deletions sessions.go
Expand Up @@ -5,12 +5,11 @@
package sessions

import (
"context"
"encoding/gob"
"fmt"
"net/http"
"time"

"github.com/gorilla/context"
)

// Default flashes key.
Expand Down Expand Up @@ -108,15 +107,16 @@ const registryKey contextKey = 0

// GetRegistry returns a registry instance for the current request.
func GetRegistry(r *http.Request) *Registry {
registry := context.Get(r, registryKey)
var ctx = r.Context()
registry := ctx.Value(registryKey)
if registry != nil {
return registry.(*Registry)
}
newRegistry := &Registry{
request: r,
sessions: make(map[string]sessionInfo),
}
context.Set(r, registryKey, newRegistry)
*r = *r.WithContext(context.WithValue(ctx, registryKey, newRegistry))
return newRegistry
}

Expand Down
31 changes: 31 additions & 0 deletions sessions_test.go
Expand Up @@ -153,6 +153,37 @@ func TestFlashes(t *testing.T) {
if custom.Type != 42 || custom.Message != "foo" {
t.Errorf("Expected %#v, got %#v", FlashMessage{42, "foo"}, custom)
}

// Round 5 ----------------------------------------------------------------
// Check if a request shallow copy resets the request context data store.

req, _ = http.NewRequest("GET", "http://localhost:8080/", nil)

// Get a session.
if session, err = store.Get(req, "session-key"); err != nil {
t.Fatalf("Error getting session: %v", err)
}

// Put a test value into the session data store.
session.Values["test"] = "test-value"

// Create a shallow copy of the request.
req = req.WithContext(req.Context())

// Get the session again.
if session, err = store.Get(req, "session-key"); err != nil {
t.Fatalf("Error getting session: %v", err)
}

// Check if the previous inserted value still exists.
if session.Values["test"] == nil {
t.Fatalf("Session test value is lost in the request context!")
}

// Check if the previous inserted value has the same value.
if session.Values["test"] != "test-value" {
t.Fatalf("Session test value is changed in the request context!")
}
}

func TestCookieStoreMapPanic(t *testing.T) {
Expand Down

1 comment on commit 12bd476

@eacp
Copy link

@eacp eacp commented on 12bd476 Apr 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you kind sir you saved me a lot of time

Please sign in to comment.