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

Should return 405 (Method Not Allowed) for invalid request methods #3238

Closed
tristanls opened this issue Jul 20, 2016 · 4 comments
Closed

Should return 405 (Method Not Allowed) for invalid request methods #3238

tristanls opened this issue Jul 20, 2016 · 4 comments
Assignees
Labels
feature New functionality or improvement

Comments

@tristanls
Copy link

According to RFC:

An origin server SHOULD return the status code 405 (Method Not Allowed)
if the method is known by the origin server but not allowed for the
requested resource, and 501 (Not Implemented) if the method is
unrecognized or not implemented by the origin server.

With a server

const Hapi = require("hapi");
const server = new Hapi.Server();
server.connection({port: 8888});
server.route(
{
    method: "POST",
    path: "/",
    handler: (request, reply) => reply("o/")
});
server.start();

I expected $ curl http://localhost:8888/ to result in 405 Method Not Allowed.

I observed:

$ curl -v http://localhost:8888/
*   Trying ::1...
* connect to ::1 port 8888 failed: Connection refused
*   Trying fe80::1...
* connect to fe80::1 port 8888 failed: Connection refused
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8888 (#0)
> GET / HTTP/1.1
> Host: localhost:8888
> User-Agent: curl/7.43.0
> Accept: */*
> 
< HTTP/1.1 404 Not Found
< content-type: application/json; charset=utf-8
< cache-control: no-cache
< content-length: 38
< Date: Wed, 20 Jul 2016 00:08:03 GMT
< Connection: keep-alive
< 
* Connection #0 to host localhost left intact

Unlike #3127, it is possible to return 405 Method Not Allowed from node.js:

With a server

const http = require('http');
http.createServer((req, res) =>
{
    res.statusCode = 405;
    res.end();
})
.listen(8888);

I observed:

$ curl -v http://localhost:8888/
*   Trying ::1...
* Connected to localhost (::1) port 8888 (#0)
> GET / HTTP/1.1
> Host: localhost:8888
> User-Agent: curl/7.43.0
> Accept: */*
> 
< HTTP/1.1 405 Method Not Allowed
< Date: Wed, 20 Jul 2016 00:12:08 GMT
< Connection: keep-alive
< Content-Length: 0
< 
* Connection #0 to host localhost left intact
@tristanls
Copy link
Author

Oh, actually...

with a server

const http = require('http');
http.createServer((req, res) =>
{
    res.statusCode = 501;
    res.end();
})
.listen(8888);

I observed:

$ curl -v http://localhost:8888/
*   Trying ::1...
* Connected to localhost (::1) port 8888 (#0)
> GET / HTTP/1.1
> Host: localhost:8888
> User-Agent: curl/7.43.0
> Accept: */*
> 
< HTTP/1.1 501 Not Implemented
< Date: Wed, 20 Jul 2016 00:13:31 GMT
< Connection: keep-alive
< Content-Length: 0
< 
* Connection #0 to host localhost left intact

I don't understand why 405 or 501 cannot be returned?

$ node -v
v6.3.0

@hueniverse
Copy link
Contributor

RFC compliance (it's a SHOULD so more like accommodation than compliance) in this case is not worth the cost. Checking for a matching route across all existing methods, just to return a slightly more accurate but really utterly useless information is not reasonable. If you care so much about this, you can manually add every possible verb to every path you configure with a 405 or 501 response. You can't do a catch all because that should remain a 404.

@hueniverse hueniverse self-assigned this Jul 20, 2016
@tristanls
Copy link
Author

I see that the method lookup is the shorter path in the implementation: https://github.com/hapijs/call/blob/260bb6c1fb447f27b5f53a2a7ebd46120c4271e7/lib/index.js#L108

Thanks for the response and an explanation.

@Marsup Marsup added feature New functionality or improvement and removed request labels Sep 20, 2019
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

3 participants