[bug] Pop doesn't remove value if w.Write happened before sm.Pop

[jackielii]

This is caused by

scs/session.go

Lines 158 to 160 in 7e11d57

	if !sw.written {
	s.commitAndWriteSessionCookie(w, sr)
	}

As demostrated in #216

scs/session_test.go

Lines 351 to 391 in 8650757

	func TestFlushPop(t *testing.T) {
	t.Parallel()
	sessionManager := New()
	mux := http.NewServeMux()
	mux.HandleFunc("/put", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	sessionManager.Put(r.Context(), "foo", "bar")
	}))
	mux.HandleFunc("/get", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
	w.Write([]byte(""))
	s, _ := sessionManager.Pop(r.Context(), "foo").(string)
	w.Write([]byte(s))
	}))
	ts := newTestServer(t, sessionManager.LoadAndSave(mux))
	defer ts.Close()
	header, _ := ts.execute(t, "/put")
	token := extractTokenFromCookie(header.Get("Set-Cookie"))
	header, body := ts.execute(t, "/get")
	if body != "bar" {
	t.Errorf("want %q; got %q", "bar", body)
	}
	cookie := header.Get("Set-Cookie")
	if cookie == "" || extractTokenFromCookie(cookie) != token {
	t.Errorf("want %q; got %q", token, cookie)
	}
	header, body = ts.execute(t, "/get")
	if body != "" {
	t.Errorf("want %q; got %q", "", body)
	}
	cookie = header.Get("Set-Cookie")
	if cookie != "" {
	t.Errorf("want %q; got %q", "", cookie)
	}
	}

If Pop happens after a w.Write, sw.written would be true. Therefore the commit would never happen

The second /get should have empty response, but it got bar instead:

go test github.com/alexedwards/scs/v2 -run '^TestFlushPop$' -timeout 30s -v -count 1
=== RUN   TestFlushPop
=== PAUSE TestFlushPop
=== CONT  TestFlushPop
    session_test.go:379: want "yhKe4NJJYf3SgN2bcxyKgbBApVF312oIEg2Bo-qpC2U"; got ""
    session_test.go:384: want ""; got "bar"
--- FAIL: TestFlushPop (0.00s)
FAIL
FAIL	github.com/alexedwards/scs/v2	0.011s
FAIL

[jum]

In general, all HTTP headers must be set and finalized before any content is written. This means you basically have to buffer the output if you intent to modify the session while producing output (very common when popping flash messages). This has bitten me quite a few times.

[jackielii]

In general, all HTTP headers must be set and finalized before any content is written. This means you basically have to buffer the output if you intent to modify the session while producing output (very common when popping flash messages). This has bitten me quite a few times.

Yes, I spend one afternoon tracking down the problem. My call stack was too deep, I didn't even suspect session manager at first.

I think even if it's decided that this is expected, I prefer to have a error or even panic for this so that I know what not to do.

In addition, my initial issue was the message not removed after Pop call. So I'm fine with the set-cookie header not present, but the values have to be removed from store.