Stop the slow test packages waiting (closes #263)

- Tests hash passwords at bcrypt.MinCost, set through the
  PasswordCost field of db.Params. TestPasswordCost checks the
  cost with and without it.
- Test IRC servers check the queue only on broker notification;
  one test keeps the 100 ms fallback poll.
- The stop-timeout test shortens the timeout through a test-only
  setter.
- service and handlers tests build config.Config directly instead
  of through config.New, so they run in parallel. config exports
  DefaultTrustedProxies and ParseTrustedProxies for the handlers
  test config.
- Repeated race trials run as parallel subtests.
- Fixed sleeps become waits on the broker.
- A new test covers the relay's failed queue read, which only the
  100 ms poll had reached.

Model: opus-5-5

README.md

@@ -157,10 +157,6 @@ either.
 ### Conventions
 
 - Request and response bodies are JSON.
-- Every `POST`, `PUT` and `DELETE` request under `/api/v1/` needs the header
-  `Content-Type: application/json`, even one with no body. A request with
-  another type, or none, gets `415` with
-  `{"error": "content type must be application/json"}`.
 - Endpoints marked "needs auth" need the `neoirc_auth` cookie.
   `POST /api/v1/session` and `POST /api/v1/login` set it. A request without a
   valid cookie gets `401` with `{"error": "not registered", "numeric": 451}`.
@@ -172,10 +168,8 @@ either.
 - Request bodies for `POST /api/v1/session`, `POST /api/v1/login` and
   `POST /api/v1/messages` are limited to `MAX_MESSAGE_SIZE` bytes (default
   `4096`). A larger body gets `400` `invalid request body`.
-- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
-  from any origin with `Access-Control-Allow-Origin: *`. Every other route sends
-  no `Access-Control-Allow-*` header, so a page on another origin cannot read
-  its responses.
+- The server allows cross-origin requests from any origin, with credentials. The
+  cookie is `SameSite=Strict`, so a browser sends it only from the same site.
 
 ### The auth cookie
 
@@ -521,8 +515,7 @@ for every HTTP request when metrics are on.
 Every other `GET` path serves the web client. A path that names one of its files
 gets that file; any other path gets its `index.html`. An unknown path under
 `/api/v1/` gets a plain-text `404`. A request whose method the path does not
-take gets `405` with no body. Under `/api/v1/`, a `POST`, `PUT` or `DELETE`
-without `Content-Type: application/json` gets `415` instead.
+take gets `405` with no body.
 
 ### The message envelope
 
@@ -1900,14 +1893,14 @@ The IRC listener has no TLS.
   seconds. A request still running after 60 seconds is cancelled, and the
   connection closes without a response. Headers may be up to 1 MiB.
 - A long poll waits at most 30 seconds.
-- Every response to a well-formed request carries a `Content-Security-Policy`
-  header:
+- Every response to a well-formed request, except to a cross-origin preflight,
+  carries a `Content-Security-Policy` header:
   `default-src 'self'; script-src 'self'; style-src 'self'; connect-src 'self'; img-src 'self'; font-src 'self'; object-src 'none'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'`.
   The web client works within it.
-- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
-  from any origin with `Access-Control-Allow-Origin: *`. Every other route sends
-  no `Access-Control-Allow-*` header, so a page on another origin cannot read
-  its responses.
+- Cross-origin requests are allowed from any origin, with credentials. The
+  allowed methods are `GET`, `POST`, `PUT`, `DELETE` and `OPTIONS`. The allowed
+  request headers are `Accept`, `Content-Type` and `X-CSRF-Token`. The cookie's
+  `SameSite=Strict` still keeps it to the same site.
 
 ### Logging
 
@@ -2011,8 +2004,7 @@ and their sessions to end. Then it closes the database.
 - `internal/hashcash/`: stamp checking and minting.
 - `internal/ratelimit/`: the per-address login rate limiter.
 - `internal/middleware/`: finding the client's address, request logging,
-  cross-origin requests, the API's `Content-Type` check, the
-  `Content-Security-Policy` header and metrics.
+  cross-origin requests, the `Content-Security-Policy` header and metrics.
 - `internal/healthcheck/` and `internal/stats/`: the health check and its
   counters.
 - `internal/logger/` and `internal/globals/`: logging setup and build

internal/db/db_test.go

@@ -27,11 +27,7 @@ const (
 		(SELECT 1 UNION ALL SELECT x + 1 FROM n)
 		SELECT COUNT(*) FROM n`
 
-	// endlessQueryTimeout is how long the endless query runs
-	// before its context ends. A query whose context ends before
-	// it starts is never run, and the pool keeps its connection,
-	// so the timeout leaves a loaded host time to start it.
-	endlessQueryTimeout = 2 * time.Second
+	endlessQueryTimeout = 100 * time.Millisecond
 )
 
 // TestDefaultDBURLSetsWALAndBusyTimeout opens a file database

internal/handlers/api_test.go

@@ -415,7 +415,7 @@ func doRequestAuth(
 		return nil, fmt.Errorf("new request: %w", err)
 	}
 
-	if method != http.MethodGet {
+	if body != nil {
 		request.Header.Set(
 			"Content-Type", "application/json",
 		)
@@ -903,131 +903,6 @@ func TestRequestBodyLimit(t *testing.T) {
 	}
 }
 
-// requestWithType sends a request with the Content-Type contentType,
-// or none when it is empty, and returns the status and the decoded
-// JSON body.
-func (tserver *testServer) requestWithType(
-	method, path, token, contentType, body string,
-) (int, map[string]any) {
-	tserver.t.Helper()
-
-	request, err := http.NewRequestWithContext(
-		tserver.t.Context(), method, tserver.url(path),
-		strings.NewReader(body),
-	)
-	if err != nil {
-		tserver.t.Fatalf("new request: %v", err)
-	}
-
-	if contentType != "" {
-		request.Header.Set("Content-Type", contentType)
-	}
-
-	request.AddCookie(&http.Cookie{ //nolint:exhaustruct,gosec // only name+value
-		Name:  authCookieName,
-		Value: token,
-	})
-
-	resp, err := http.DefaultClient.Do(request)
-	if err != nil {
-		tserver.t.Fatalf("%s %s: %v", method, path, err)
-	}
-
-	defer func() { _ = resp.Body.Close() }()
-
-	var result map[string]any
-
-	_ = json.NewDecoder(resp.Body).Decode(&result)
-
-	return resp.StatusCode, result
-}
-
-// TestRequireJSONContentType checks that a POST whose Content-Type is
-// not application/json gets 415 and changes nothing: the message is
-// not stored and the client is not logged out.
-func TestRequireJSONContentType(t *testing.T) {
-	t.Parallel()
-
-	tserver := newTestServer(t)
-	token := tserver.createSession("alice_ctype")
-
-	tserver.sendCommand(token, map[string]any{
-		commandKey: joinCmd, toKey: "#ctype",
-	})
-
-	status, _ := tserver.requestWithType(
-		http.MethodGet, apiState, token, "text/plain", "",
-	)
-	if status != http.StatusOK {
-		t.Errorf("GET with text/plain: expected 200, got %d", status)
-	}
-
-	tests := []struct {
-		name, contentType string
-		want              int
-	}{
-		{"text", "text/plain", http.StatusUnsupportedMediaType},
-		{
-			"form", "application/x-www-form-urlencoded",
-			http.StatusUnsupportedMediaType,
-		},
-		{
-			"multipart", "multipart/form-data; boundary=x",
-			http.StatusUnsupportedMediaType,
-		},
-		{"missing", "", http.StatusUnsupportedMediaType},
-		{"json", "application/json", http.StatusOK},
-		{"json with charset", "application/json; charset=utf-8", http.StatusOK},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
-			status, result := tserver.requestWithType(
-				http.MethodPost, apiMessages, token, test.contentType,
-				`{"command":"PRIVMSG","to":"#ctype","body":["`+test.name+`"]}`,
-			)
-			if status != test.want {
-				t.Fatalf("send: expected %d, got %d: %v", test.want, status, result)
-			}
-
-			if status == http.StatusUnsupportedMediaType &&
-				result["error"] != "content type must be application/json" {
-				t.Errorf("send: unexpected error body %v", result)
-			}
-
-			msgs, _ := tserver.pollMessages(token, 0)
-
-			stored := slices.ContainsFunc(msgs, func(msg map[string]any) bool {
-				body, _ := msg[bodyKey].([]any)
-
-				return msg[commandKey] == privmsgCmd &&
-					slices.Contains(body, any(test.name))
-			})
-			if stored != (test.want == http.StatusOK) {
-				t.Errorf("message stored: %v, want %v", stored, !stored)
-			}
-
-			if test.want == http.StatusOK {
-				return
-			}
-
-			status, _ = tserver.requestWithType(
-				http.MethodPost, apiLogout, token, test.contentType, "",
-			)
-			if status != http.StatusUnsupportedMediaType {
-				t.Errorf("logout: expected 415, got %d", status)
-			}
-
-			status, _ = tserver.getState(token)
-			if status != http.StatusOK {
-				t.Errorf("state after refused logout: expected 200, got %d", status)
-			}
-		})
-	}
-}
-
 func TestAuthNoCookie(t *testing.T) {
 	t.Parallel()
 

internal/ircserver/server_test.go

@@ -31,10 +31,7 @@ import (
 	_ "modernc.org/sqlite"
 )
 
-// testTimeout is how long a test waits for something before it
-// fails. A passing test never waits this long, so it is generous
-// enough for a heavily loaded host.
-const testTimeout = 30 * time.Second
+const testTimeout = 5 * time.Second
 
 // testPollInterval is how often the relay of a test server
 // checks the client output queue without a broker
@@ -568,49 +565,27 @@ func TestQuietClientAnsweringPingStaysConnected(t *testing.T) {
 func TestQuietClientNotAnsweringPingIsClosed(t *testing.T) {
 	t.Parallel()
 
-	// A read deadline of two ping intervals would close the
-	// connection a second after latest. The ping interval is
-	// longer than testPingInterval, so that the time between
-	// the expected close and latest leaves room for a loaded
-	// host.
+	// latest leaves room for a loaded host, but is sooner than
+	// a read deadline of two ping intervals would close it.
 	const (
-		interval = 4 * time.Second
-		earliest = interval + testPongDeadline
-		latest   = 2*interval - time.Second
+		earliest = testPingInterval + testPongDeadline
+		latest   = 2 * testPingInterval
 	)
 
-	env := newTestEnvWithPingTimes(
-		t,
-		&config.Config{ServerName: testServerName},
-		interval, testPongDeadline,
-	)
+	env := newTestEnvWithShortPings(t)
 	client := env.dial(t)
 
 	lastSent := time.Now()
 
 	client.register("silent")
-
-	// The server starts both waits once it has handled the
-	// registration, so the time registration took does not
-	// count against latest.
-	registered := time.Now()
-
-	client.readUntil(isServerPing)
+	client.readServerPing(lastSent)
 	client.readUntilClosed()
 
 	waited := time.Since(lastSent)
-	if waited < earliest {
+	if waited < earliest || waited >= latest {
 		t.Errorf(
-			"closed %v after the client's last line, want at least %v",
-			waited, earliest,
-		)
-	}
-
-	sinceRegistered := time.Since(registered)
-	if sinceRegistered >= latest {
-		t.Errorf(
-			"closed %v after registration, want less than %v",
-			sinceRegistered, latest,
+			"closed %v after the client's last line, want %v to %v",
+			waited, earliest, latest,
 		)
 	}
 }

internal/middleware/middleware.go

@@ -3,7 +3,6 @@ package middleware
 
 import (
 	"log/slog"
-	"mime"
 	"net"
 	"net/http"
 	"net/netip"
@@ -201,74 +200,28 @@ func (mware *Middleware) Logging() func(http.Handler) http.Handler {
 	}
 }
 
-// CORS returns middleware that lets a page on any origin read the
-// response. Use it only on public, read-only routes.
+// CORS returns middleware that handles Cross-Origin Resource Sharing.
+// AllowCredentials is true so browsers include cookies in
+// cross-origin API requests.
 func (mware *Middleware) CORS() func(http.Handler) http.Handler {
 	return cors.Handler(cors.Options{ //nolint:exhaustruct // optional fields
-		AllowedOrigins: []string{"*"},
+		AllowOriginFunc: func(
+			_ *http.Request, _ string,
+		) bool {
+			return true
+		},
 		AllowedMethods: []string{
 			"GET", "POST", "PUT", "DELETE", "OPTIONS",
 		},
 		AllowedHeaders: []string{
-			"Accept", "Authorization", "Content-Type", "X-CSRF-Token",
+			"Accept", "Content-Type", "X-CSRF-Token",
 		},
 		ExposedHeaders:   []string{"Link"},
-		AllowCredentials: false,
+		AllowCredentials: true,
 		MaxAge:           corsMaxAge,
 	})
 }
 
-// notJSONError is the body of the 415 answer, in the API's usual
-// error format.
-const notJSONError = `{"error":"content type must be application/json"}` + "\n"
-
-// needsJSON reports whether a request with method must have the
-// Content-Type application/json, whether or not it has a body.
-func needsJSON(method string) bool {
-	return method == http.MethodPost ||
-		method == http.MethodPut ||
-		method == http.MethodDelete
-}
-
-// isJSON reports whether the request's Content-Type is
-// application/json, with or without parameters such as charset.
-func isJSON(request *http.Request) bool {
-	mediaType, _, err := mime.ParseMediaType(
-		request.Header.Get("Content-Type"),
-	)
-
-	return err == nil && mediaType == "application/json"
-}
-
-// RequireJSON returns middleware that answers 415 to a POST, PUT or
-// DELETE request whose Content-Type is not application/json, or that
-// has none. A browser sends a request with another type, such as
-// text/plain, to another origin of the same site without asking the
-// server first, and attaches the auth cookie. Refusing those types
-// stops a page on that origin from acting as the signed-in user.
-func (mware *Middleware) RequireJSON() func(http.Handler) http.Handler {
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(
-			func(
-				writer http.ResponseWriter,
-				request *http.Request,
-			) {
-				if needsJSON(request.Method) && !isJSON(request) {
-					writer.Header().Set(
-						"Content-Type",
-						"application/json; charset=utf-8",
-					)
-					writer.WriteHeader(http.StatusUnsupportedMediaType)
-					_, _ = writer.Write([]byte(notJSONError))
-
-					return
-				}
-
-				next.ServeHTTP(writer, request)
-			})
-	}
-}
-
 // Metrics returns middleware that records HTTP metrics.
 func (mware *Middleware) Metrics() func(http.Handler) http.Handler {
 	metricsMiddleware := ghmm.New(ghmm.Config{ //nolint:exhaustruct // optional fields

internal/middleware/middleware_test.go

@@ -152,73 +152,6 @@ func TestClientIP(t *testing.T) {
 	}
 }
 
-func TestRequireJSON(t *testing.T) {
-	t.Parallel()
-
-	mware := newMiddleware(t, trustedNetwork)
-
-	const (
-		jsonType = "application/json"
-		textType = "text/plain"
-		refused  = http.StatusUnsupportedMediaType
-	)
-
-	tests := []struct {
-		method, contentType string
-		want                int
-	}{
-		{http.MethodPost, jsonType, http.StatusOK},
-		{http.MethodPost, "application/json; charset=utf-8", http.StatusOK},
-		{http.MethodPost, "Application/JSON", http.StatusOK},
-		{http.MethodPost, textType, refused},
-		{http.MethodPost, "application/x-www-form-urlencoded", refused},
-		{http.MethodPost, "multipart/form-data; boundary=x", refused},
-		{http.MethodPost, "", refused},
-		{http.MethodPost, "application/json; charset", refused},
-		{http.MethodPut, textType, refused},
-		{http.MethodPut, jsonType, http.StatusOK},
-		{http.MethodDelete, "", refused},
-		{http.MethodDelete, jsonType, http.StatusOK},
-		{http.MethodGet, "", http.StatusOK},
-		{http.MethodGet, textType, http.StatusOK},
-	}
-
-	for _, test := range tests {
-		t.Run(test.method+" "+test.contentType, func(t *testing.T) {
-			t.Parallel()
-
-			handler := mware.RequireJSON()(http.HandlerFunc(func(
-				http.ResponseWriter, *http.Request,
-			) {
-			}))
-
-			request := httptest.NewRequestWithContext(
-				t.Context(), test.method, "/", nil,
-			)
-			if test.contentType != "" {
-				request.Header.Set("Content-Type", test.contentType)
-			}
-
-			recorder := httptest.NewRecorder()
-
-			handler.ServeHTTP(recorder, request)
-
-			if recorder.Code != test.want {
-				t.Fatalf("status %d, want %d", recorder.Code, test.want)
-			}
-
-			if test.want == http.StatusOK {
-				return
-			}
-
-			want := `{"error":"content type must be application/json"}` + "\n"
-			if recorder.Body.String() != want {
-				t.Errorf("body %q, want %q", recorder.Body.String(), want)
-			}
-		})
-	}
-}
-
 // newMiddleware returns a Middleware that trusts the network trusted.
 func newMiddleware(t *testing.T, trusted string) *middleware.Middleware {
 	t.Helper()

internal/server/routes.go

@@ -30,6 +30,7 @@ func (srv *Server) SetupRoutes() {
 		srv.router.Use(srv.mw.Metrics())
 	}
 
+	srv.router.Use(srv.mw.CORS())
 	srv.router.Use(srv.mw.CSP())
 	srv.router.Use(middleware.Timeout(routeTimeout))
 
@@ -44,7 +45,7 @@ func (srv *Server) SetupRoutes() {
 	}
 
 	// Health check.
-	srv.router.With(srv.mw.CORS()).Get(
+	srv.router.Get(
 		"/.well-known/healthcheck.json",
 		srv.handlers.HandleHealthCheck(),
 	)
@@ -68,9 +69,7 @@ func (srv *Server) SetupRoutes() {
 }
 
 func (srv *Server) setupAPIv1(router chi.Router) {
-	router.Use(srv.mw.RequireJSON())
-
-	router.With(srv.mw.CORS()).Get(
+	router.Get(
 		"/server",
 		srv.handlers.HandleServerInfo(),
 	)

internal/server/server_test.go

@@ -6,10 +6,8 @@ package server_test
 
 import (
 	"fmt"
-	"maps"
 	"net"
 	"net/http"
-	"net/http/httptest"
 	"strconv"
 	"strings"
 	"testing"
@@ -190,88 +188,6 @@ func TestServeAfterStopDoesNotStopApp(t *testing.T) {
 	}
 }
 
-// TestCORSHeaders checks the Access-Control-Allow-* headers on the
-// answers to requests from another origin. The public, read-only
-// routes answer with Access-Control-Allow-Origin: * alone. Routes
-// that need the cookie, and a preflight to one, answer with none.
-func TestCORSHeaders(t *testing.T) {
-	var srv *server.Server
-
-	startApp(t, fx.Populate(&srv))
-
-	tests := []struct {
-		name          string
-		method        string
-		path          string
-		requestMethod string
-		wantHeaders   map[string]string
-	}{
-		{
-			name:        "server information",
-			method:      http.MethodGet,
-			path:        "/api/v1/server",
-			wantHeaders: map[string]string{"Access-Control-Allow-Origin": "*"},
-		},
-		{
-			name:        "health check",
-			method:      http.MethodGet,
-			path:        "/.well-known/healthcheck.json",
-			wantHeaders: map[string]string{"Access-Control-Allow-Origin": "*"},
-		},
-		{
-			name:   "send",
-			method: http.MethodPost,
-			path:   "/api/v1/messages",
-		},
-		{
-			name:   "state",
-			method: http.MethodGet,
-			path:   "/api/v1/state",
-		},
-		{
-			name:          "preflight for send",
-			method:        http.MethodOptions,
-			path:          "/api/v1/messages",
-			requestMethod: http.MethodPost,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			request := httptest.NewRequestWithContext(
-				t.Context(), test.method,
-				"http://chat.example.com"+test.path, nil,
-			)
-			request.Header.Set("Origin", "http://blog.example.com")
-
-			if test.requestMethod != "" {
-				request.Header.Set(
-					"Access-Control-Request-Method", test.requestMethod,
-				)
-			}
-
-			recorder := httptest.NewRecorder()
-
-			srv.ServeHTTP(recorder, request)
-
-			gotHeaders := map[string]string{}
-
-			for name := range recorder.Header() {
-				if strings.HasPrefix(name, "Access-Control-Allow-") {
-					gotHeaders[name] = recorder.Header().Get(name)
-				}
-			}
-
-			if !maps.Equal(gotHeaders, test.wantHeaders) {
-				t.Errorf(
-					"Access-Control-Allow-* headers are %v, want %v",
-					gotHeaders, test.wantHeaders,
-				)
-			}
-		})
-	}
-}
-
 // startApp starts the app newApp builds, with the HTTP port
 // set to 0, so the system gives the server a free port.
 func startApp(