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

Include Allow header with 405 response #68

Open
alexedwards opened this issue Jul 3, 2018 · 5 comments
Open

Include Allow header with 405 response #68

alexedwards opened this issue Jul 3, 2018 · 5 comments

Comments

@alexedwards
Copy link

alexedwards commented Jul 3, 2018

When bone returns a 405 Method Not Allowed response, it would be good if it could include an Allow header detailing which methods are permitted.

I think it could perhaps be done by amending the otherMethods helper. Something like:

func (m *Mux) otherMethods(rw http.ResponseWriter, req *http.Request) bool {
	allowed := []string{}
	for _, met := range method {
		if met != req.Method {
			for _, r := range m.Routes[met] {
				ok := r.exists(rw, req)
				if ok {
					allowed = append(allowed, r.Method)
				}
			}
		}
	}
	if len(allowed) > 0 {
		rw.Header()["Allow"] = allowed
		rw.WriteHeader(http.StatusMethodNotAllowed)
		return true
	}
	return false
}

Also, what are your thoughts on including a plain text "Method Not Allowed" message in the response body, similar to the "404 page not allowed" body for 404s?

@squiidz
Copy link
Member

squiidz commented Jul 6, 2018

It could be a good idea, do you think you could make a pull request for it ? :)

@harshvladha
Copy link
Contributor

Good idea about adding Allow header. Missed it to add that.

@alexedwards - Text Method Not Allowed, can be user (dev) driven instead of hard-coding it. Blank response suffice the use-case, even in case of 404 page not found returning blank response with just status code will work. What I think is text/response body should be handled by user, default being blank.

@alexedwards
Copy link
Author

alexedwards commented Jul 14, 2018

@squiidz Yep, I'm happy to submit a PR for this.

I'm not sure I agree on the blank response body though. I think that default behavior should be useful, and having a human-readable message (http.Error(w, http.StatusText(405), 405)) seems more useful and user-friendly than showing them a blank page. There's also a precedent set with the way that this repository handles 404 responses (it shows a message by default) and I think the default 405 response should be consistent with that. Otherwise it makes the behavior of bone less predictable from a user (dev) perspective.

Longer term, would it be sensible to provide a new mux.MethodNotAllowed method to allow users to register a custom hander for 405s? Similar to the way [mux.NotFound()](https://godoc.org/github.com/go-zoo/bone#Mux.NotFound) works.

@alexedwards
Copy link
Author

PR submitted.

@szenti
Copy link

szenti commented Feb 18, 2022

Hmm, is this project still alive? cc @squiidz

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

No branches or pull requests

4 participants