Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: net/http: Custom handlers for 404 and 405 HTTP response codes #65648

Open
denpeshkov opened this issue Feb 10, 2024 · 11 comments
Open
Labels
Milestone

Comments

@denpeshkov
Copy link

Proposal Details

Go 1.22 introduced an enhanced HTTP routing. The current implementation utilizes unexported handlers for 404 and 405 responses. However, if there is a need for a custom 404 response (e.g., in a REST API with JSON responses), it is no longer possible to use a 'catch-all' pattern /, as it prevents the ability to return a 405 response. This issue is elaborated further in this discussion.

To address this challenge, one can define a custom http.ResponseWriter to intercept responses and their status codes and handle them appropriately. Nonetheless, this approach precludes the ability to return a custom 404 response based on whether the http.ServeMux couldn't locate the appropriate route or if the user's handler returned a response with a 404 status code. An example scenario is when responding to GET user/{id} for a non-existent user in the system.

Given these challenges, I believe it would be valuable to register custom 404 and 405 handlers.

@gopherbot gopherbot added this to the Proposal milestone Feb 10, 2024
@seankhliao
Copy link
Member

see also #21548

Do you have a concrete proposal for the api?

@polyscone
Copy link

polyscone commented Feb 11, 2024

I just landed here whilst trying to find out if this was supported or not.

This is my current workaround using a catch-all / pattern and the http.ServeMux.Handler method.

methods := []string{
	http.MethodGet,
	http.MethodHead,
	http.MethodPost,
	http.MethodPut,
	http.MethodPatch,
	http.MethodDelete,
	http.MethodConnect,
	http.MethodOptions,
	http.MethodTrace,
}

mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
	method := r.Method
	_, current := mux.Handler(r)
	var allowed []string
	for _, method := range methods {
		// If we find a pattern that's different from the pattern for the
		// current fallback handler then we know there are actually other handlers
		// that could match with a method change, so we should handle as
		// method not allowed
		r.Method = method
		if _, pattern := mux.Handler(r); pattern != current {
			allowed = append(allowed, method)
		}
	}

	r.Method = method

	if len(allowed) != 0 {
		w.Header().Set("allow", strings.Join(allowed, ", "))

		http.Error(w, "Custom Method Not Allowed", http.StatusMethodNotAllowed)

		return
	}

	http.Error(w, "Custom Not Found", http.StatusNotFound)
})

I haven't thought about it too deeply, so I don't know if there are cases where this workaround would be lacking.

I don't have any strong feelings about how an API might look, or even if it's needed at all, but I wanted to show at least one solution that's working for me in my simple use-cases at the moment, and doesn't require changes to the existing serve mux API.

@ianlancetaylor
Copy link
Contributor

CC @jba

@denpeshkov
Copy link
Author

@polyscone At the moment, I'm using this workaround: https://github.com/denpeshkov/greenlight/blob/c68f5a2111adcd5b1a65a06595acc93a02b6380e/internal/http/middleware.go#L16-L71

However, as I mentioned earlier, this approach doesn't allow me to handle 404 errors differently depending on whether the route is actually registered or if my handler just returns a 404 (e.g., when a user with a given ID is not found).

@polyscone
Copy link

@denpeshkov Thanks for the link; this is roughly what I imagined you were doing based on your original explanation.

Does the catch-all handler that I use in my own services not solve your problem?

In the workaround I suggested your handlers are free to respond in any way they need to, and the catch-all is where you can implement generic 404 and 405 responses, all without the need for any middleware that wraps http.ResponseWriter.

If you genuinely also needed to use the catch-all / route for other purposes you can also call a custom function in-between handling for 404 and 405.

@denpeshkov
Copy link
Author

@polyscone Yeah, I think your approach should be working. It just seems that you're reimplementing functionality that is already provided by ServeMux (handling 404 and 405). So I thought that adding the ability to specify custom handlers, like, for example, is done here: https://pkg.go.dev/github.com/julienschmidt/httprouter#Router, might be a good alternative.

@jba
Copy link
Contributor

jba commented Feb 20, 2024

An example scenario is when responding to GET user/{id} for a non-existent user in the system.

@denpeshkov, I don't see how this is an example of your problem. If the user is not in the system, the handler for GET user/{id} should serve whatever it wants, including custom page with a 404 status.

@jba
Copy link
Contributor

jba commented Feb 20, 2024

@denpeshkov, I read more deeply so I think I understand: you are calling the handler and using its response, but you don't know if a 404 is "I couldn't find a matching route" or "I couldn't find the user with that ID." So ignore my comment.

@jba
Copy link
Contributor

jba commented Feb 20, 2024

To summarize:

Before Go 1.22, you could produce a custom 404 page by registering it on "/". A net/http web server would never serve a 405 (unless user code did).

As of Go 1.22, you can also produce a custom 404 page using "/". Everything will work as it did pre-1.22: no 405s will be served.

So the only problem is that a custom 404 masks the new 405 behavior. How much does that matter?

Well, no one cared about 405s before Go 1.22. So maybe very little.

Also—this is conjecture and I would love a counterexample—if you're serving a custom 404 it's because your service is facing users and you want to show them a branded page of some kind. But 405s are not interesting to humans because they aren't typing raw PUT and DELETE requests to HTTP servers. They are only helpful to other computers. So maybe the use cases for custom 404s and automatic 405s are disjoint.

(I'm aware of one use case for serving a custom 404 to a computer: if you want your server to return only JSON. But that's probably unrealistic and also hopeless with the existing net/http package, because there are many places in the code that send a text response.)

If we allow people to customize 404, then we really should provide hooks for all errors, and custom error hooks have already been proposed and rejected.

So I don't see a clear way forward here, but I also don't see a burning problem. Please correct me if I'm wrong on either count.

/cc @neild @bradfitz

@denpeshkov
Copy link
Author

Hi, @jba. Yes, you are right. The problem I was facing is developing a 'JSON-only' REST API. I looked into some popular REST services (GitHub API and Stripe API), and it seems they don't send 405 responses. I think I am going to use a catch-all route as in the pre-1.22 release. Thank you for your response

@Rican7
Copy link

Rican7 commented May 8, 2024

So yea, I understand why this maybe hasn't been prioritized, but the workaround to get the desired behavior feels like a hack for sure.

I've figured out that hack, at least, in case anyone else comes across this:

package httpmux

import (
	"net/http"
)

// HeaderFlagDoNotIntercept defines a header that is (unfortunately) to be used
// as a flag of sorts, to denote to this routing engine to not intercept the
// response that is being written. It's an unfortunate artifact of an
// implementation detail within the standard library's net/http.ServeMux for how
// HTTP 404 and 405 responses can be customized, which requires writing a custom
// response writer and preventing the standard library from just writing it's
// own hard-coded response.
//
// See:
//   - https://github.com/golang/go/issues/10123
//   - https://github.com/golang/go/issues/21548
//   - https://github.com/golang/go/issues/65648
const HeaderFlagDoNotIntercept = "do_not_intercept"

type excludeHeaderWriter struct {
	http.ResponseWriter

	excludedHeaders []string
}

func (w *excludeHeaderWriter) WriteHeader(statusCode int) {
	for _, header := range w.excludedHeaders {
		w.Header().Del(header)
	}

	w.ResponseWriter.WriteHeader(statusCode)
}

type routingStatusInterceptWriter struct {
	http.ResponseWriter

	intercept404 func() bool
	intercept405 func() bool

	statusCode  int
	intercepted bool
}

func (w *routingStatusInterceptWriter) WriteHeader(statusCode int) {
	if w.intercepted {
		return
	}

	w.statusCode = statusCode

	if (w.intercept404() && statusCode == http.StatusNotFound) ||
		(w.intercept405() && statusCode == http.StatusMethodNotAllowed) {

		w.intercepted = true
		return
	}

	w.ResponseWriter.WriteHeader(statusCode)
}

func (w *routingStatusInterceptWriter) Write(data []byte) (int, error) {
	if w.intercepted {
		return 0, nil
	}

	return w.ResponseWriter.Write(data)
}

type Router struct {
	*http.ServeMux

	NotFoundHandler         http.HandlerFunc
	MethodNotAllowedHandler http.HandlerFunc
}

// ServeHTTP dispatches the request to the router.
func (rt *Router) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	interceptor := &routingStatusInterceptWriter{
		ResponseWriter: &excludeHeaderWriter{
			ResponseWriter: w,

			excludedHeaders: []string{HeaderFlagDoNotIntercept},
		},

		intercept404: func() bool {
			return rt.NotFoundHandler != nil && w.Header().Get(HeaderFlagDoNotIntercept) == ""
		},
		intercept405: func() bool {
			return rt.MethodNotAllowedHandler != nil && w.Header().Get(HeaderFlagDoNotIntercept) == ""
		},
	}

	rt.ServeMux.ServeHTTP(interceptor, r)

	switch {
	case interceptor.intercepted && interceptor.statusCode == http.StatusNotFound:
		rt.NotFoundHandler.ServeHTTP(interceptor.ResponseWriter, r)
	case interceptor.intercepted && interceptor.statusCode == http.StatusMethodNotAllowed:
		rt.MethodNotAllowedHandler.ServeHTTP(interceptor.ResponseWriter, r)
	}
}

... which you can use like this:

package main

import (
	"encoding/json"
	"fmt"
	"io"
	"net/http"
	"net/http/httptest"
)

func main() {
	jsonErrorHandler := func(statusCode int) http.HandlerFunc {
		return func(w http.ResponseWriter, r *http.Request) {
			w.Header().Set("Content-Type", "application/json; charset=utf-8")
			w.WriteHeader(statusCode)

			json.NewEncoder(w).Encode(struct{ Intercepted bool }{Intercepted: true})
		}
	}

	rt := &httpmux.Router{
		ServeMux: http.NewServeMux(),

		NotFoundHandler:         jsonErrorHandler(404),
		MethodNotAllowedHandler: jsonErrorHandler(405),
	}

	rt.HandleFunc("GET /foo", func(w http.ResponseWriter, r *http.Request) {
		// Set this flag to be able to return a 404 without having it be rewritten
		w.Header().Set(httpmux.HeaderFlagDoNotIntercept, "true")

		w.Header().Set("Content-Type", "application/json; charset=utf-8")
		w.WriteHeader(200)

		json.NewEncoder(w).Encode(struct{ Intercepted bool }{Intercepted: false})
	})

	for _, req := range []*http.Request{
		httptest.NewRequest("GET", "/foo", nil),
		httptest.NewRequest("GET", "/bar", nil),
		httptest.NewRequest("DELETE", "/foo", nil),
	} {
		w := httptest.NewRecorder()
		rt.ServeHTTP(w, req)

		resp := w.Result()
		body, _ := io.ReadAll(resp.Body)

		fmt.Printf("%s %s\n", req.Method, req.URL)
		fmt.Println(resp.StatusCode)
		fmt.Println(resp.Header.Get("Content-Type"))
		fmt.Println(string(body))
		fmt.Println("-------------------------------")
	}
}

(Check it out in the Go Playground)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

7 participants