caddy: Send 501 responses as a fallback to unknown HTTP methods

Caddy’s default behavior appears to be treating unhandled HTTP request methods as if they were GET. This is a very bad idea: it seriously impedes the ability to add new HTTP methods, which is terrible for the Internet as a whole.

Caddy SHOULD be producing 405 responses to any method it recognizes but does not allow. This requires populating the “Allow” header, which could be difficult to do automatically, so I can understand reluctance on that front.

501 responses (which are explicitly meant for unknown methods) have no such requirement, and so are trivial to implement. For now, I’m using something along the lines[^1] of this to approximate that:

@unsupported {
        not method GET HEAD POST PUT DELETE CONNECT OPTIONS TRACE
}

error @unsupported 501

This is obviously rather messy. It would be much better if Caddy did this on its own, especially since I doubt many users actually read RFC 9110.

[^1]: What I’m actually doing is matching methods I allow separately, so I can configure 405 responses for the methods Caddy knows about but disallows.

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 23 (12 by maintainers)

Commits related to this issue

fileserver: Reject non-GET/HEAD requests (close #5166) — committed to caddyserver/caddy by mholt 2 years ago
fileserver: Reject non-GET/HEAD requests (close #5166) (#5167) * fileserver: Reject non-GET/HEAD requests (close #5166) * Set Allow header according to RFC 9110 10.2.1 — committed to caddyserver/caddy by mholt 2 years ago

Most upvoted comments

So first, I need to apologize for being incoherent: apparently the GitHub Mobile app continues to show me only part of the conversation, lagging way behind in what’s happening now, even after refreshing the thread. Only closing and restarting the app brought the content up-to-date. Now I’m at my computer so hopefully what I’m seeing is faithful.

I’m also really frustrated with the app right now. Sorry if I come off a bit terse.

I’ve just had to re-read the entire thread from the beginning to fill in many replies that didn’t show up for me. I’ll try to catch up here and reply to everything in-line.

@Saklad5

I would rather Caddy crashed than say something succeeded without even knowing what that means.

We can’t allow Caddy to crash; that’s out of the question. But we can’t say that there was an error either, without knowing what the error is 🤔 (Because there was no error.)

Hopefully our explanations of “what that means” (“something succeeded”) makes sense, about the request succeeding. 200 OK is the appropriate response for that.

Whether the server “succeeded” is irrelevant. A status of 200 means the request succeeded.

Exactly. The request succeeded. That’s what 200 means, and that’s why we return that.

If Caddy is configured to do nothing, why not just do literally nothing? Don’t respond.

Okay, now that I can see the full thread, I see the confusion with my later reply, which I thought was immediately on the heels of your comment here. I meant that not responding would leak resources. If we simply ignore the request, the client will continue to wait and hold the connection open until it gets a response or we close the connection. If we close the connection we also interrupt other requests on it, and the experience for the client is very jarring, tells you nothing, and makes you think the server crashed or is broken rather than telling you the server is at least responding. So closing the connection is also out of the question, as it’s akin with crashing.

Let me give a very specific example of why this is problematic. Let’s say you set up a file server with minimal configuration changes.

Now you try sending a DELETE request to a resource. RFC 9110 says that a 200 status in the response means the action is successful. Caddy obviously didn’t delete the file described by the request, but it’s going to explicitly say it did anyway.

Okay, now we’re getting somewhere. Thanks for the specifics.

Please recall that I said we cannot assume what the application is doing. In this case Caddy cannot know that the application is a static, read-only file server, and that DELETEs aren’t allowed. It doesn’t know that unless you tell it that or the application enforces that.

So maybe what you’re actually asking for is our file_server handler module to return 405 for methods that aren’t GET or HEAD? Since it can’t mutate the files at all.

Please differentiate between “Caddy” and “a Caddy module”, as they are distinct. All modules are plugins to Caddy, even the HTTP server as a whole is a plugin. All its handlers are plugins. It would be more productive to talk about the behavior of the specific plugin you’re having trouble with, than suggesting that Caddy should reject all the methods you specified by default, which as I said would break a ton of servers and we aren’t going to do that.

I can give more specific examples of how this is dangerous, if you prefer.

If you can demonstrate a security breach with the latest commits, I would be definitely be interested in that.

Anyway, it sounds like all we need in the file server is to reject methods that aren’t GET or HEAD. Yeah?

mholt on Oct 23, 2022

Again, our view is that the request did succeed.

It’s fine that you disagree, but we avoid adding implicit behaviour so that you have a clean slate to build up from. Add config to do what you need, and constrain what Caddy does.

francislavoie on Oct 23, 2022

Well, it did succeed. That’s the thing. Caddy did exactly what it was configured to do (i.e. nothing).

francislavoie on Oct 23, 2022