Jaws.ServeHTTP can recycle a Request while its initial renderer still owns it

[linkdata]

Severity: High

Summary

An early request to /jaws/<key> can claim, clear, and pool a Request while the initial HTTP handler is still rendering with that same pointer.

Contract and valid usage

NewRequest says callers should store the returned pointer and use it while constructing the HTML response, and only stop retaining it after initial HTTP handling and rendering (requestpool.go:22-32).

The reproduction remains inside that lifetime. The application mounts the public Jaws.ServeHTTP handler normally; the malformed callback is external network input, not misuse of Request.ServeHTTP.

Reproduction

package audit_test

import (
	"bytes"
	"net/http"
	"net/http/httptest"
	"testing"

	"github.com/linkdata/jaws"
)

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

	initial := httptest.NewRequest(http.MethodGet, "/", nil)
	initial.RemoteAddr = "192.0.2.1:1000"
	rq := jw.NewRequest(initial)

	var page bytes.Buffer
	if err := rq.HeadHTML(&page); err != nil {
		t.Fatal(err)
	}

	callback := httptest.NewRequest(http.MethodGet, "/jaws/"+rq.JawsKeyString(), nil)
	callback.RemoteAddr = initial.RemoteAddr
	jw.ServeHTTP(httptest.NewRecorder(), callback)

	if rq.JawsKeyString() == "" {
		t.Fatal("Request was cleared while the initial render still owned it")
	}
}

The test currently fails because JawsKeyString becomes empty.

Root cause

• jaws.go:549-552 calls UseRequest before the WebSocket handshake is validated.
• request.go:962-963 installs an unconditional defer rq.stopServe().
• websocket.Accept fails at request.go:989 for the malformed callback.
• stopServe calls recycle at request.go:910-912.
• requestpool.go:324-333 clears the Request and places it into reqPool.
• request.go:245-294 zeroes the key and initial request, deletes Elements, and clears request state.

Impact

The initial renderer observes a cleared object: its key disappears, registered Elements are deleted, and queued state is discarded. Once the pointer is reused from sync.Pool, continued rendering can mutate an unrelated client's Request, creating cross-request state corruption and races.

Suggested fix

Track initial-render ownership independently from WebSocket ownership and do not clear or pool a claimed Request until both lifetimes have ended. Retiring the object without pooling while an initial-render borrower may remain is a safe conservative alternative. Handshake prevalidation alone would not cover a valid WebSocket that connects and exits before initial rendering completes.