Should return code 405 for method not allowed

[adambrod]

It would be great if Express would return status code 405 when a client requests a resource (aka a path) with an http verb that is not allowed. Today express will return a 404, which isn't quite correct since the path may be valid and found, it just doesn't support that verb. Here is the doc I'm reading for the definitions of status code 404 and 405: http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

404 Not Found
The server has not found anything matching the Request-URI. No indication is given of whether the condition is temporary or permanent. The 410 (Gone) status code SHOULD be used if the server knows, through some internally configurable mechanism, that an old resource is permanently unavailable and has no forwarding address. This status code is commonly used when the server does not wish to reveal exactly why the request has been refused, or when no other response is applicable.

405 Method Not Allowed
The method specified in the Request-Line is not allowed for the resource identified by the Request-URI. The response MUST include an Allow header containing a list of valid methods for the requested resource.

Here is an example. In my server.coffee file I have the following definition:

app.get '/', (req, res) ->
  res.json { players: "/v1/accounts/:accountId/players" }

Assuming I have not registered app.post '/', (req, res), when a client invokes

curl -v -d "some post data" http://localhost:9000/

The response includes "HTTP/1.1 404 Not Found" but it should be "HTTP/1.1 405 Method Not Allowed".

[adambrod]

I'm not terribly happy with it, but for future reference I've created some middleware that "fixes" this behavior. I have a strong suspicion that I could improve this quite a bit, but it seems to be working based on limited testing. NOTE: This middleware needs to be defined at the end, after all the routes and other middleware, but before the error handling middleware.

var Route, Util, buildRoutes;

Util = require('util');

Route = require('express/lib/router/route');

module.exports = function(app) {
  var pathRoutes;
  pathRoutes = null;
  return function(req, res, next) {
    var path, result, val;
    if (!pathRoutes) {
      pathRoutes = buildRoutes(app);
    }
    result = {
      error_code: 'Not Found',
      message: "" + req.path + " Not Found"
    };
    for (path in pathRoutes) {
      val = pathRoutes[path];
      if (val.route.match(req.path)) {
        res.set('Allow', "" + (val.methods.join(',')));
        res.set('Cache-Control', 'max-age=600');
        result = {
          error_code: 'Method Not Allowed',
          message: "" + req.method + " Not Allowed for " + req.path
        };
        res.json(405, result);
        return;
      }
    }
    res.set('Cache-Control', 'max-age=60');
    return res.json(404, result);
  };
};

buildRoutes = function(app) {
  var newRoute, path, route, routeMap, routes, verb, _i, _len, _ref;
  routeMap = {};
  _ref = app.routes;
  for (verb in _ref) {
    routes = _ref[verb];
    for (_i = 0, _len = routes.length; _i < _len; _i++) {
      route = routes[_i];
      path = route.path;
      if (!routeMap[path]) {
        newRoute = new Route(verb, path, null, {
          sensitive: app.enabled('case sensitive routing'),
          strict: app.enabled('strict routing')
        });
        routeMap[path] = {
          route: newRoute,
          methods: []
        };
      }
      routeMap[path].methods.push(verb.toUpperCase());
    }
  }
  return routeMap;
};

[jonathanong]

what if I have

app.all('*', middleware1)
app.get('/login', middleware2)

and you get a request at /register? when should the 405 error code by sent?

[adambrod]

Hmm, good question. I think there will be some scenarios where it is hard for the framework to do it 100% correctly. I'm not currently using any dynamic paths so I haven't solved this yet. In that example, since the developer is using app.all (and not app.use), I am interpreting that they want their server to (potentially) respond to any request path with any http verb and so a 404 would be perfectly reasonable if middleware1 doesn't respond to "/register". In my mind, if the developer wanted to use the framework to do the default thing correctly most of the time, I'd recommend he use the "use" method to be the catch-all middleware, so "*" isn't really a valid route or resource. Routes and/or Resources should be defined with app.VERB and then the server can do the right thing:

app.use(middleware1)
app.get('/login', middleware2)

and then Express would respond with a 404 for GET /register and respond with 405 for PUT /login. Does that make sense?

[jonathanong]

app.get('/thing/:id*', getThing)
app.get('/thing/:id/comments', getThingComments)

What about /thing/1/replies? There's just too much ambiguity IMO.

[defunctzombie]

Closing this issue since it seems to be an opinion left to the app and unclear on what we can do about it in a clean way.