diff --git a/README.md b/README.md index 9621ff8..5164900 100644 --- a/README.md +++ b/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 -does not resolve only `could not resolve host name`; the detail goes to the log. +plain text. Under `/api/v1` and `/ip/`, a path no route matches answers 404 with +the message `Not found`, and `POST`, `PUT`, `PATCH`, `DELETE`, `OPTIONS`, +`TRACE` or `CONNECT` 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 diff --git a/internal/server/middleware.go b/internal/server/middleware.go index 9b28999..a0eab9f 100644 --- a/internal/server/middleware.go +++ b/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 -// errors, it returns "Internal server error" with status 500 if the original +// has passed, it answers 504 with the error message "Request timeout". A 4xx +// 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{ diff --git a/internal/server/middleware_internal_test.go b/internal/server/middleware_internal_test.go index 3db0924..e3803fb 100644 --- a/internal/server/middleware_internal_test.go +++ b/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 diff --git a/internal/server/routes.go b/internal/server/routes.go index 38fe83d..f995c22 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -1,18 +1,33 @@ package server import ( + "net/http" "time" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" ) +// The error messages the JSON routes under /api/v1 and /ip answer with when no +// route matches the path, and when none takes the method. +const ( + notFoundMessage = "Not found" + methodNotAllowedMessage = "Method not allowed" + + // jsonRoutesAllow is the Allow header of their 405 answer: every one of + // them takes GET and HEAD, and no other method. + jsonRoutesAllow = "GET, HEAD" +) + // setupRoutes configures the HTTP routes func (s *Server) setupRoutes() { r := chi.NewRouter() // Middleware r.Use(DecodedPathMiddleware) + // GetHead serves HEAD with the GET handler. It looks the route up by the + // path DecodedPathMiddleware sets, so it comes after it. + r.Use(middleware.GetHead) r.Use(SecurityHeadersMiddleware) r.Use(middleware.RequestID) r.Use(RealIPMiddleware(s.trustedProxies)) @@ -40,16 +55,34 @@ 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.Get("/", s.handleIPInfo()) // Client IP + r.NotFound(jsonNotFound) + r.MethodNotAllowed(jsonMethodNotAllowed) + r.Get("/", s.handleIPInfo()) // Client IP + // GetHead does not reach this route: mounting the group registers + // /ip and /ip/ for every method, so HEAD arrives here as HEAD. + r.Head("/", s.handleIPInfo()) r.Get("/{addr}", s.handleIPInfo()) // Specified IP }) // 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()) diff --git a/internal/server/routes_internal_test.go b/internal/server/routes_internal_test.go new file mode 100644 index 0000000..f4f0d6a --- /dev/null +++ b/internal/server/routes_internal_test.go @@ -0,0 +1,100 @@ +package server + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +// TestRoutesAnswerHEAD sends HEAD for a page and JSON routes through the +// server's router and checks that each answers as GET does. /ip and /ip/, the +// lookup of the caller's own address, are in the list because chi's GetHead +// does not reach them. 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, "/ip", "/ip/"} { + 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) + } +}