Answer HEAD requests and give 405 a plain message (closes #111)

Every route was registered for GET only, so HEAD answered 405, and on
the JSON routes that 405 said "Internal server error". chi's GetHead
now serves HEAD with the GET handler. The /api/v1 and /ip route groups
answer a path they lack with a JSON 404 "Not found", and another method
with a JSON 405 "Method not allowed" whose Allow header names GET and
HEAD. JSONValidationMiddleware gives a 4xx sent without a JSON body the
status's standard text instead of "Internal server error".

Model: opus-5-5

README.md

@@ -36,6 +36,8 @@ DEBUG=routewatch ./routewatch
 
 ## HTTP Endpoints
 
+Every route answers `HEAD` as it answers `GET`, without the body.
+
 ### Web Interface
 
 - `GET /` - Home page (HTML): headline figures, and lookups by AS number and by
@@ -90,8 +92,11 @@ and 4 seconds for the database reads behind `/status.json` and `/api/v1/stats`.
 An AS number, prefix or mask length that does not parse answers 400, as does a
 mask length out of range for its IP version. An AS the database does not hold,
 or a prefix with no live route, answers 404. The HTML pages give these errors as
-plain text. A 500 answer says only `Internal server error`, and a host name that
+plain text. Under `/api/v1` and `/ip/`, a path no route matches answers 404 with
-does not resolve only `could not resolve host name`; the detail goes to the log.
+the message `Not found`, and a method other than `GET` and `HEAD` answers 405
+with `Method not allowed`. A 500 answer says only `Internal server error`, and a
+host name that does not resolve only `could not resolve host name`; the detail
+goes to the log.
 
 Every answer carries `Strict-Transport-Security`, `Content-Security-Policy`,
 `X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy` and

internal/server/middleware.go

@@ -142,8 +142,9 @@ func deadlinePassed(ctx context.Context) bool {
 //
 // If a response is not valid JSON or is empty when JSON is expected, the middleware
 // returns a properly formatted JSON error response. If the request's time limit
-// has passed, it answers 504 with the error message "Request timeout". For other
+// has passed, it answers 504 with the error message "Request timeout". A 4xx
-// errors, it returns "Internal server error" with status 500 if the original
+// keeps its status, with the status's standard text as the message. For other
+// errors, it returns "Internal server error", with status 500 if the original
 // status was 200.
 //
 // Non-JSON responses are passed through unchanged.
@@ -203,8 +204,13 @@ func JSONValidationMiddleware(next http.Handler) http.Handler {
 		w.WriteHeader(statusCode)
 
 		errorMsg := internalErrorMessage
-		if statusCode == http.StatusGatewayTimeout {
+
+		switch {
+		case statusCode == http.StatusGatewayTimeout:
 			errorMsg = requestTimeoutMessage
+		case statusCode >= http.StatusBadRequest &&
+			statusCode < http.StatusInternalServerError:
+			errorMsg = http.StatusText(statusCode)
 		}
 
 		response := map[string]any{

internal/server/middleware_internal_test.go

@@ -58,6 +58,34 @@ func TestResponsesCarrySecurityHeaders(t *testing.T) {
 	}
 }
 
+// TestJSONValidationKeeps4xxWithoutBody wraps a handler that answers 400
+// without a body in JSONValidationMiddleware and checks that the JSON error
+// body keeps the 400 and gives the status's own text. Before the fix it said
+// "Internal server error"
+// (https://git.eeqj.de/sneak/routewatch/issues/111).
+func TestJSONValidationKeeps4xxWithoutBody(t *testing.T) {
+	t.Parallel()
+
+	handler := JSONValidationMiddleware(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusBadRequest)
+		}))
+
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, httptest.NewRequestWithContext(
+		t.Context(), http.MethodGet, "/", nil))
+
+	const name = "400 without a body"
+
+	if rec.Code != http.StatusBadRequest {
+		t.Errorf("%s: status %d, want %d; body %s",
+			name, rec.Code, http.StatusBadRequest, rec.Body)
+	}
+
+	checkJSONErrorCode(t, name, rec, http.StatusBadRequest)
+	checkJSONErrorMessage(t, name, rec, http.StatusText(http.StatusBadRequest))
+}
+
 // TestPagesCarryTheCSPNonce checks that every script and style block of each
 // page carries the nonce its Content-Security-Policy header names, so that the
 // browser runs and applies them, that each answer has a nonce of its own, and

internal/server/routes.go

@@ -1,18 +1,31 @@
 package server
 
 import (
+	"net/http"
 	"time"
 
 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/chi/v5/middleware"
 )
 
+// What the JSON routes under /api/v1 and /ip answer a path none of them
+// matches, and a method they do not take, with.
+const (
+	notFoundMessage         = "Not found"
+	methodNotAllowedMessage = "Method not allowed"
+
+	// jsonRoutesAllow is the Allow header of their 405 answer: they are all
+	// GET routes, and GetHead serves HEAD on them as GET.
+	jsonRoutesAllow = "GET, HEAD"
+)
+
 // setupRoutes configures the HTTP routes
 func (s *Server) setupRoutes() {
 	r := chi.NewRouter()
 
 	// Middleware
 	r.Use(DecodedPathMiddleware)
+	r.Use(middleware.GetHead) // after DecodedPathMiddleware, to route by the same path
 	r.Use(SecurityHeadersMiddleware)
 	r.Use(middleware.RequestID)
 	r.Use(RealIPMiddleware(s.trustedProxies))
@@ -40,9 +53,22 @@ func (s *Server) setupRoutes() {
 	r.Get("/prefixlength/{length}", s.handlePrefixLength())
 	r.Get("/prefixlength6/{length}", s.handlePrefixLength6())
 
+	// The JSON routes answer a path or method they do not take with the JSON
+	// error body, as they answer everything else.
+	jsonNotFound := func(w http.ResponseWriter, _ *http.Request) {
+		s.writeJSONError(w, http.StatusNotFound, notFoundMessage)
+	}
+	jsonMethodNotAllowed := func(w http.ResponseWriter, _ *http.Request) {
+		// chi's own 405 answer sets Allow; one set in its place must too.
+		w.Header().Set("Allow", jsonRoutesAllow)
+		s.writeJSONError(w, http.StatusMethodNotAllowed, methodNotAllowedMessage)
+	}
+
 	// IP info JSON endpoints (replaces old /ip redirect)
 	r.Route("/ip", func(r chi.Router) {
 		r.Use(JSONValidationMiddleware)
+		r.NotFound(jsonNotFound)
+		r.MethodNotAllowed(jsonMethodNotAllowed)
 		r.Get("/", s.handleIPInfo())       // Client IP
 		r.Get("/{addr}", s.handleIPInfo()) // Specified IP
 	})
@@ -50,6 +76,8 @@ func (s *Server) setupRoutes() {
 	// API routes
 	r.Route("/api/v1", func(r chi.Router) {
 		r.Use(JSONValidationMiddleware)
+		r.NotFound(jsonNotFound)
+		r.MethodNotAllowed(jsonMethodNotAllowed)
 		r.Get("/stats", s.handleStats())
 		r.Get("/ip/{ip}", s.handleIPLookup())
 		r.Get("/as/{asn}", s.handleASDetailJSON())

internal/server/routes_internal_test.go

@@ -0,0 +1,99 @@
+package server
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// TestRoutesAnswerHEAD sends HEAD for a page and a JSON route through the
+// server's router and checks that each answers as GET does. Before the fix
+// every route answered HEAD with 405
+// (https://git.eeqj.de/sneak/routewatch/issues/111).
+func TestRoutesAnswerHEAD(t *testing.T) {
+	t.Parallel()
+
+	s := newServerWithTestRoutes(t)
+
+	for _, path := range []string{"/", statsPath} {
+		rec := httptest.NewRecorder()
+		s.router.ServeHTTP(rec, httptest.NewRequestWithContext(
+			t.Context(), http.MethodHead, path, nil))
+
+		if rec.Code != http.StatusOK {
+			t.Errorf("HEAD %s: status %d, want %d; body %s",
+				path, rec.Code, http.StatusOK, rec.Body)
+		}
+	}
+}
+
+// TestJSONRoutesAnswerUnmatchedRequestsWithJSONErrors sends a method the JSON
+// routes do not take, and a path none of them matches, to each group of JSON
+// routes through the server's router. Each must answer with the JSON error
+// body, and a 405 must name the methods the routes take in its Allow header.
+// Before the fix a 405 said "Internal server error" and a 404 was chi's plain
+// text answer (https://git.eeqj.de/sneak/routewatch/issues/111).
+func TestJSONRoutesAnswerUnmatchedRequestsWithJSONErrors(t *testing.T) {
+	t.Parallel()
+
+	s := newServerWithTestRoutes(t)
+
+	tests := []struct {
+		method   string
+		path     string
+		wantCode int
+		wantMsg  string
+	}{
+		{http.MethodPost, statsPath, http.StatusMethodNotAllowed, methodNotAllowedMessage},
+		{http.MethodPost, "/ip/", http.StatusMethodNotAllowed, methodNotAllowedMessage},
+		{http.MethodGet, "/api/v1/nothing", http.StatusNotFound, notFoundMessage},
+		{http.MethodGet, "/ip/192.0.2.1/nothing", http.StatusNotFound, notFoundMessage},
+	}
+
+	for _, tt := range tests {
+		name := tt.method + " " + tt.path
+
+		rec := httptest.NewRecorder()
+		s.router.ServeHTTP(rec, httptest.NewRequestWithContext(
+			t.Context(), tt.method, tt.path, nil))
+
+		if rec.Code != tt.wantCode {
+			t.Errorf("%s: status %d, want %d; body %s",
+				name, rec.Code, tt.wantCode, rec.Body)
+		}
+
+		checkJSONErrorCode(t, name, rec, tt.wantCode)
+		checkJSONErrorMessage(t, name, rec, tt.wantMsg)
+
+		allow := rec.Header().Get("Allow")
+		if tt.wantCode == http.StatusMethodNotAllowed && allow != jsonRoutesAllow {
+			t.Errorf("%s: Allow %q, want %q", name, allow, jsonRoutesAllow)
+		}
+	}
+}
+
+// checkJSONErrorMessage checks that rec holds the JSON error body with the
+// given message.
+func checkJSONErrorMessage(
+	t *testing.T, name string, rec *httptest.ResponseRecorder, want string,
+) {
+	t.Helper()
+
+	var body struct {
+		Error struct {
+			Msg string `json:"msg"`
+		} `json:"error"`
+	}
+
+	err := json.Unmarshal(rec.Body.Bytes(), &body)
+	if err != nil {
+		t.Errorf("%s: decoding the answer %q: %v", name, rec.Body, err)
+
+		return
+	}
+
+	if body.Error.Msg != want {
+		t.Errorf("%s: error message %q, want %q", name, body.Error.Msg, want)
+	}
+}