Skip to content

Session.Close drops reloads for associated Requests that have not subscribed yet #215

Description

@linkdata

Summary

Session.Close silently loses the promised reload for an associated Request when the session is closed after the initial page render but before that Request's WebSocket subscribes.

Class / severity

Correctness / lifecycle — High. Session invalidation can leave a correctly rendered page running stale UI after logout, credential rotation, or other session teardown.

Contract and valid usage

Session.Close documents that existing Requests already associated with the Session "will ask the browser to reload the pages." Its only lifecycle precondition is that Jaws.Serve or Jaws.ServeWithTimeout is running.

The Request below is already associated, the processing loop is running, and the caller closes the Session before the browser opens its normal WebSocket.

Reproduction

package jaws_test

import (
	"context"
	"net/http"
	"net/http/httptest"
	"strings"
	"testing"
	"time"

	"github.com/coder/websocket"
	"github.com/linkdata/jaws"
	"github.com/linkdata/jaws/lib/what"
	"github.com/linkdata/jaws/lib/wire"
)

func TestSessionCloseReloadsAssociatedPendingRequest(t *testing.T) {
	jw, err := jaws.New()
	if err != nil {
		t.Fatal(err)
	}
	t.Cleanup(jw.Close)
	go jw.Serve()

	srv := httptest.NewServer(jw)
	t.Cleanup(srv.Close)

	initial := httptest.NewRequest(http.MethodGet, srv.URL+"/", nil)
	initial.RemoteAddr = "127.0.0.1:1"
	sess := jw.NewSession(httptest.NewRecorder(), initial)
	rq := jw.NewRequest(initial)
	if rq.Session() != sess {
		t.Fatal("request was not associated with session")
	}

	connected := make(chan struct{})
	rq.SetConnectFn(func(*jaws.Request) error {
		close(connected)
		return nil
	})

	// The page exists, but its normal WebSocket has not subscribed yet.
	_ = sess.Close()

	hdr := http.Header{}
	hdr.Set("Origin", srv.URL)
	ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second)
	defer cancel()
	conn, _, err := websocket.Dial(ctx,
		"ws"+strings.TrimPrefix(srv.URL, "http")+"/jaws/"+rq.JawsKeyString(),
		&websocket.DialOptions{HTTPHeader: hdr})
	if err != nil {
		t.Fatal(err)
	}
	defer conn.CloseNow()
	<-connected // subscription is now installed

	const marker = "post-connect marker"
	jw.Broadcast(wire.Message{Dest: rq.JawsKey, What: what.Alert, Data: marker})
	_, data, err := conn.Read(ctx)
	if err != nil {
		t.Fatal(err)
	}
	if !strings.Contains(string(data), marker) {
		t.Fatalf("healthy marker was not delivered: %q", data)
	}
	if !strings.Contains(string(data), what.Reload.String()+"\t") {
		t.Fatalf("Session.Close reload was lost: %q", data)
	}
}

The marker arrives, proving the WebSocket is healthy, but no Reload frame arrives.

Root cause

session.go:202-207 detaches each Request and sends a key-targeted Reload through Jaws.Broadcast. serve.go:79-93 distributes broadcasts only to subscriptions that exist at that instant. A valid pending Request has no subscription until its WebSocket starts, so the Reload is dropped permanently.

This is distinct from #161: that issue installed the subscription before ConnectFn, while this message is sent before the WebSocket request begins at all.

Impact

Pages open during session teardown do not reload. They can continue displaying stale authenticated state and behave inconsistently with the now-dead server session.

Suggested fix

Persist the reload requirement on the Request until its WebSocket subscribes (or reject/cancel the pending Request in a way that makes the browser reload), rather than relying solely on an instantaneous broadcast.

Confidence

Confirmed on main at 6fd13a2.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions