When serveStatic and methodNotAllowed are used together, methods other than GET cannot be handled correctly

[na-trium-144]

What version of Hono are you using?

4.13.1

What runtime/platform is your app running on? (with version if possible)

Node.js

What steps can reproduce the bug?

The documentation includes examples of serveStatic middleware being used with both get() and use().
But in both cases methodNotAllowed does not work as expected.

with get:

const app = new Hono()
app.use(methodNotAllowed(app));
app.get("/api", (c) => c.text("ok"));
app.get("/*", serveStatic());

with use:

const app = new Hono()
app.use(methodNotAllowed(app));
app.get("/api", (c) => c.text("ok"));
app.use("/*", serveStatic());

What is the expected behavior?

• return 200 on GET /api,
• return 200 on GET existing file,
• return 404 on GET non-existent file,
• return 405 on POST /api,
• return 405 on POST existing file,
  • or maybe 404 is also accepted in this case?
• return 404 on POST non-existent file.

What do you see instead?

with get(serveStatic):

• return 405 on POST /api,
• return 405 on POST existing file,
• return 405 on POST non-existent file.

with use(serveStatic),

• return 405 on POST /api,
• return 200 on POST existing file,
• return 404 on POST non-existent file.

Additional information

I think the solution would be:

• modify serveStatic so it returns 404 or 405 when request method is neither GET or HEAD, and
• document the requirement to use serveStatic with use() rather than get(), (though its slightly counter-intuitive as serveStatic should handle only GET,) particularly when you are using methodNotAllowed together.

Alternatively,

• ignore wildcard path in methodNotAllowed middleware, just as .all() route is currently ignored. This feels more generic but I don't know if this is always the expected behavior.
  • This results in 404 on POST existing file, but I think it is at least better than 405 on POST non-existent file.

[usualoma]

Hi @na-trium-144,

Thanks for the detailed report. After considering the trade-offs, I think the behavior should be refined in the following way.

methodNotAllowed

I now think routes whose paths end in * should be ignored when inferring allowed methods.

A trailing wildcard matches an arbitrary remainder of the path, so it does not provide enough evidence that a particular target resource exists. Allowing it to contribute to method inference can cause every otherwise-unhandled request in that namespace to be converted from 404 to 405.

I do not think we need to add a route filter option at this point. Ignoring trailing wildcard routes by default seems like a reasonable behavior refinement.

Since methodNotAllowed was introduced only recently, I expect the compatibility impact to be limited. I am therefore inclined to treat this as a behavior refinement and include it in a patch release.

serveStatic

serveStatic should handle only GET and HEAD requests. For other methods, it should call next() before resolving or accessing the file.

This avoids performing a file-existence check solely to choose between 404 and 405, and prevents serveStatic from serving an existing file in response to a POST request.

This is effectively the same change proposed in #5224, although that PR was opened before the intended behavior had been fully discussed.

Since this is not directly related to methodNotAllowed, however, it may be better to discuss it in a separate issue.

Documentation

I think the current .use() examples can remain unchanged.

Although .get() also covers HEAD because Hono dispatches HEAD requests to GET routes internally, serveStatic is fallthrough middleware rather than a terminal GET handler, so .use() seems to fit its role better. With serveStatic itself restricted to GET and HEAD, using .use() should no longer cause other methods to serve files.

@yusukebe, what do you think about this direction?