Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Only save Session once to avoid duplicate session cookie #2193

Merged
merged 2 commits into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ func New(opts Options) *App {
}
a.Use(a.PanicHandler)
a.Use(RequestLogger)
a.Use(sessionSaver)

return a
}
11 changes: 10 additions & 1 deletion default_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ func (d *DefaultContext) Render(status int, rr render.Renderer) error {
if d.Session() != nil {
d.Flash().Clear()
d.Flash().persist(d.Session())
if err := d.Session().Save(); err != nil {
return HTTPError{Status: http.StatusInternalServerError, Cause: err}
}
}

d.Response().Header().Set("Content-Type", rr.ContentType())
Expand Down Expand Up @@ -191,7 +194,13 @@ var mapType = reflect.ValueOf(map[string]interface{}{}).Type()

// Redirect a request with the given status to the given URL.
func (d *DefaultContext) Redirect(status int, url string, args ...interface{}) error {
d.Flash().persist(d.Session())
if d.Session() != nil {
d.Flash().Clear()
d.Flash().persist(d.Session())
if err := d.Session().Save(); err != nil {
return HTTPError{Status: http.StatusInternalServerError, Cause: err}
}
}

if strings.HasSuffix(url, "Path()") {
if len(args) > 1 {
Expand Down
1 change: 0 additions & 1 deletion flash.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ func (f Flash) Add(key, value string) {
func (f Flash) persist(session *Session) {
b, _ := json.Marshal(f.data)
session.Set(flashKey, b)
session.Save()
}

//newFlash creates a new Flash and loads the session data inside its data.
Expand Down
1 change: 0 additions & 1 deletion request_logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ func RequestLoggerFunc(h Handler) Handler {
}
irid = rs
c.Session().Set("requestor_id", irid)
c.Session().Save()
}

rid := irid.(string) + "-" + rs
Expand Down
1 change: 0 additions & 1 deletion route_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ func (ri RouteInfo) ServeHTTP(res http.ResponseWriter, req *http.Request) {

events.EmitPayload(EvtRouteStarted, payload)
err := a.Middleware.handler(ri)(c)
c.Flash().persist(c.Session())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was not clear why this is needed, so I removed it. persist() no longer saves Session, so is this needed? Only reference is: #2054


if err != nil {
status := http.StatusInternalServerError
Expand Down
10 changes: 0 additions & 10 deletions session.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,3 @@ func (a *App) getSession(r *http.Request, w http.ResponseWriter) *Session {
res: w,
}
}

func sessionSaver(next Handler) Handler {
return func(c Context) error {
err := next(c)
if err != nil {
return err
}
return c.Session().Save()
}
}
64 changes: 64 additions & 0 deletions session_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package buffalo

import (
"fmt"
"net/http"
"strings"
"testing"

"github.com/gobuffalo/buffalo/render"
"github.com/gobuffalo/httptest"

"github.com/stretchr/testify/require"
)

func Test_Session_SingleCookie(t *testing.T) {
r := require.New(t)

sessionName := "_test_session"
a := New(Options{SessionName: sessionName})
rr := render.New(render.Options{})

a.GET("/", func(c Context) error {
return c.Render(http.StatusCreated, rr.String(""))
})

w := httptest.New(a)
res := w.HTML("/").Get()

var sessionCookies []string
for _, c := range res.Header().Values("Set-Cookie") {
if strings.HasPrefix(c, sessionName) {
sessionCookies = append(sessionCookies, c)
}
}

r.Equal(1, len(sessionCookies))
}

func Test_Session_CustomValue(t *testing.T) {
r := require.New(t)

a := New(Options{})
rr := render.New(render.Options{})

// Root path sets a custom session value
a.GET("/", func(c Context) error {
c.Session().Set("example", "test")
return c.Render(http.StatusCreated, rr.String(""))
})
// /session path prints custom session value as response
a.GET("/session", func(c Context) error {
sessionValue := c.Session().Get("example")
return c.Render(http.StatusCreated, rr.String(fmt.Sprintf("%s", sessionValue)))
})

w := httptest.New(a)
_ = w.HTML("/").Get()

// Create second request that should contain the cookie from the first response
reqGetSession := w.HTML("/session")
resGetSession := reqGetSession.Get()

r.Equal(resGetSession.Body.String(), "test")
}