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

Return 405, not 501, when an unsupported HTTP method is used #3286

Closed

Conversation

mcmillan
Copy link

@mcmillan mcmillan commented Dec 7, 2023

Description

Currently, when the supported_http_methods DSL config option is not set to :any, the behaviour upon encountering an unexpected HTTP method is to return an 501 Not Implemented response.

Whilst this is technically correct, it can be somewhat problematic; in situations where 5xx error responses are used as a heuristic to determine whether or not an application is healthy, a single person sending a flood of requests with invalid HTTP methods can result in an application being considered 'unhealthy' because Puma returns a 501.

It seems the somewhat more semantically correct response code to use in this scenario is 405 Method Not Allowed – this is how Rails deals with HTTP methods it can't handle, for example. The HTTP spec requires that an Allow header is returned alongside it with details of which methods are acceptable.

To that end, this modifies the behaviour of Puma to return a 405 when supplied an unsupported HTTP method, rather than a 501, and return an Allow header with the valid methods.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

Currently, when the `supported_http_methods` DSL config option is not
set to `:any`, the behaviour upon encountering an unexpected HTTP method
is to return an `501 Not Implemented` response.

Whilst this is technically correct, it can be somewhat problematic; in
situations where 5xx error responses are used as a heuristic to
determine whether or not an application is healthy, a single person
sending a flood of requests with invalid HTTP methods can result in an
application being considered 'unhealthy' because Puma returns a 501.

It seems the somewhat more semantically correct response code to use in
this scenario is `405 Method Not Allowed` – this is how Rails deals with
HTTP methods it can't handle, for example. The HTTP spec requires that
an `Allow` header is returned alongside it with details of which methods
_are_ acceptable.

To that end, this modifies the behaviour of Puma to return a 405 when
supplied an unsupported HTTP method, rather than a 501, and return an
`Allow` header with the valid methods.
@dentarg
Copy link
Member

dentarg commented Dec 7, 2023

Did you see #3284?

@dentarg
Copy link
Member

dentarg commented Dec 7, 2023

I will reiterate my comment from that issue: #3284 (comment)

I think we should go back to the way it was in Puma 5 and let the apps define this response

cc @nateberkopec again because he is the decision maker here

@mcmillan
Copy link
Author

mcmillan commented Dec 7, 2023

Ah – I didn't. Sorry folks; I'll close this!

@mcmillan mcmillan closed this Dec 7, 2023
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

Successfully merging this pull request may close these issues.

None yet

2 participants