Failing test for 404 when no GET root

[lasseebert]

I have an app where there is no GET / route defined, but a POST / is defined (and several other GET routes).

This seems to mess up response status code for GET requests that does not resolve to a route.

See failing spec, which fails with message

  1) Failure:
Hanami::Router::root::path recognition#test_0003_handles not found on GET when only POST root is defined [/vagrant/hanami-router/test/routing_test.rb:116]:
Expected: 404
  Actual: 405

Is this behaviour somehow by design?

[jodosha]

@lasseebert Hey, thanks for catching this problem.

Does this failing test reveal the intention of writing a fix? 😉
If so, you should have a look at Hanami::Routing::HttpRouter#no_response.

[lasseebert]

@jodosha, yes it does ;)

Was just checking in before writing any code. I will try to make a fix for this bug.

[lasseebert]

Findings so far:

• The problem is not isolated to having a POST root and trying to GET /bogus, but also the other way around
• It is most likely a bug in HTTPRouter, possibly related to Incorrectly returns 405 for non-matching route if HTTP method is specified. joshbuddy/http_router#31

[jodosha]

@lasseebert Thank you for taking the time to check this. Please remember that we can "fix" http_router problems directly here. We have Hanami::Routing::HttpRouter which is a subclass of HttpRouter (from http_router).

[bruz]

It looks like the current behavior when the request doesn't match a route can be summarized as:

• If the request path is a subpath of one or more routes, and none of those routes use the method of the request, then it's a 405
• Otherwise, it's a 404

Some tests I tried, that demonstrate this:

route	request	status
post '/'	GET /	405
post '/'	GET /bogus	405
post '/books'	GET /	404
post '/books'	GET /books	405
post '/books'	GET /books/whatever	405
post '/books'	GET /booksasdf	405
post '/books'	GET /waffles	404

I'm wondering if this is actually intended behavior in http_router, and it's notion of being the same "resource" is being a subpath of another route, with a 405 indicating an invalid method on that resource.

What should Hanami be treating as 404s vs 405s? The current behavior gives a mixture of 404s and 405s for different methods on a RESTful resource, which seem a little confusing. Maybe 405 could be used strictly for when it's a route for the request path, but not the request method. Or, going the way of Rails and Sinatra, all route misses could be 404s.

[jodosha]

@bruz Yes. I agree.

To recap: missing paths should return 404. Existing paths called with wrong HTTP verb should return 405.

[lasseebert]

I'm glad we all agree :)

Sorry for not participating more in this issue. The issue was originally found on a customer's project, and that project doesn't have the problem anymore. So I couldn't really use their time to fix the bug.

Will see if I get some spare-time to look into it anytime soon :)

[jodosha]

@AlfonsoUceda Any news on this?

[AlfonsoUceda]

@jodosha sorry I didn't take a look from when I assigned it

[AlfonsoUceda]

@lasseebert thanks for this PR but I'm going to close it, our idea is to change http_router dependency by another maintained router or build our own router.

The problem is that http_router is a mess, it's very difficult to debug because it generates a lot of code with metaprogramming. I debugged this problem and the router generates allowed methods for one path when you make a request and because how it works, we have those problems.