Repository: caddyserver/caddy (language: go)

Issue:
POST to fileserver causes error handling failure
It's a bit weird to POST to a fileserver, but some of our users have a configuration which likes to try it quite frequently :upside_down_face: 

Although we have an error template https://github.com/caddyserver/caddy/issues/5698#issuecomment-1666705511 clients see an empty response body, with a 404 status.  This wouldn't be much of a problem because nobody sees it, but it also causes caddy to log an error (to stderr or stdout) which spams up the journal.  
`{"level":"error","ts":1691295507.1841986,"logger":"http.log.error","msg":"error handling handler error","request":{"remote_ip":"<my_ip>","remote_port":"60788","client_ip":"<my_ip>","proto":"HTTP/2.0","method":"POST","host":"xonotic.org","uri":"/foo","headers":{"User-Agent":["curl/7.88.1"],"Accept":["*/*"],"Content-Length":["6"],"Content-Type":["application/x-www-form-urlencoded"]},"tls":{"resumed":false,"version":772,"cipher_suite":4865,"proto":"h2","server_name":"xonotic.org"}},"duration":0.000209028,"error":"{id=hu8vqnzjr} fileserver.(*FileServer).ServeHTTP (staticfiles.go:422): HTTP 405","first_error":{"msg":"{id=5fgwbu868} fileserver.(*FileServer).notFound (staticfiles.go:607): HTTP 404","status":404,"err_id":"5fgwbu868","err_trace":"fileserver.(*FileServer).notFound (staticfiles.go:607)"}}`  
Triggering that was `curl -v -d 'cheese' https://xonotic.org/foo`

It looks to me like it doesn't try to execute the template, but fails before getting that far.

This happens in 2.6.4 and 2.7.3.


Discussion / hints:
What's the template? What's your config? Without seeing those, it's unclear to me what's going on exactly, or how to replicate it.

A POST to the fileserver triggers an HTTP 405 Method Not Allowed (that's shown in the error log as `fileserver.(*FileServer).ServeHTTP (staticfiles.go:422): HTTP 405`). This was changed as of v2.6.3 with https://github.com/caddyserver/caddy/pull/5167 to reject non-GET and non-HEAD requests to `file_server`.
The main fileserver handling this is just
```
        root * /var/www/xonotic.org/HTML
        file_server
```
Same error handler as in the other thread https://github.com/caddyserver/caddy/issues/5698#issuecomment-1666705511

There's misc stuff in that vhost block (setting headers, reverse proxying specific subdirectories), but to reproduce it seems to only need an error template, a fileserver, and a POST request for that fileserver to handle.  It happens both with paths that exist (client gets 405), and paths that don't (client gets 404) but either way it logs the `"error handling handler error"` without executing the template.
Please post your _entire_ config, not just bits and pieces. The _entire_ config is relevant, because things affect eachother. It slows down the process of debugging when you omit information.
My initial post contains everything required to reproduce it, but I can post a minimal walkthrough if it'll help:

Grabbing 2.7.3 from somewhere:  
`scp srv04:/usr/bin/caddy ./`  
Creating a basic config (equivalent to the relevant parts of the production config):  
`vim Caddyfile`  
```
http://127.0.0.1:80 {
        root * /tmp/html
        file_server

        handle_errors {
                root * /tmp/html
                file_server
                rewrite * /error/error.html
                templates
        }
}
```
Creating the template:  
`mkdir -p /tmp/html/error`
`vim /tmp/html/error/error.html`
```
{{placeholder "http.error.status_code"}} {{placeholder "http.error.message"}}
```
Letting caddy listen on port 80 as a normal user:  
`sudo setcap CAP_NET_BIND_SERVICE=+eip caddy`  
Starting it up:  
`./caddy run --environ --config ./Caddyfile`  
Reproducing the error:  
`curl -v -d 'cheese' http://127.0.0.1/foo`
Result output by caddy:  
`ERROR   http.log.error  error handling handler error    {"request": {"remote_ip": "127.0.0.1", "remote_port": "52730", "client_ip": "127.0.0.1", "proto": "HTTP/1.1", "method": "POST", "host": "127.0.0.1", "uri": "/foo", "headers": {"User-Agent": ["curl/7.88.1"], "Accept": ["*/*"], "Content-Length": ["6"], "Content-Type": ["application/x-www-form-urlencoded"]}}, "duration": 0.000077962, "error": "{id=unm311stf} fileserver.(*FileServer).ServeHTTP (staticfiles.go:422): HTTP 405", "first_error": {"msg": "{id=wba877j9h} fileserver.(*FileServer).notFound (staticfiles.go:607): HTTP 404", "status": 404, "err_id": "wba877j9h", "err_trace": "fileserver.(*FileServer).notFound (staticfiles.go:607)"}}`
Result output by curl (note the lack of response body):
```
*   Trying 127.0.0.1:80...
* Connected to 127.0.0.1 (127.0.0.1) port 80 (#0)
> POST /foo HTTP/1.1
> Host: 127.0.0.1
> User-Agent: curl/7.88.1
> Accept: */*
> Content-Length: 6
> Content-Type: application/x-www-form-urlencoded
> 
< HTTP/1.1 404 Not Found
< Allow: GET, HEAD
< Server: Caddy
< Date: Sun, 06 Aug 2023 04:53:02 GMT
< Content-Length: 0
< 
* Connection #0 to host 127.0.0.1 left intact
* ```
Okay I see what's going on and I think I have a fix. Thanks for the full example, it helps a lot.

The trouble is that `file_server` errors due to `405 Method Not Allowed`, then the method is _still_ POST during error handlers so it triggers an error again.

This obviously doesn't really make sense to do, since pretty much the only reason to have a `file_server` inside `handle_errors` is to write a file as a response, so erroring out again is no good.

So the fix is to just check if we're in `handle_errors` and skip the recursive 405 error in that case.

Fix patch:
diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go
index 3261093d9e0..133578cf69f 100644
--- a/modules/caddyhttp/fileserver/staticfiles.go
+++ b/modules/caddyhttp/fileserver/staticfiles.go
@@ -418,8 +418,13 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c
 	// GET and HEAD, which is sensible for a static file server - reject
 	// any other methods (see issue #5166)
 	if r.Method != http.MethodGet && r.Method != http.MethodHead {
-		w.Header().Add("Allow", "GET, HEAD")
-		return caddyhttp.Error(http.StatusMethodNotAllowed, nil)
+		// if we're in an error context, then it doesn't make sense
+		// to repeat the error; just continue because we're probably
+		// trying to write an error page response (see issue #5703)
+		if _, ok := r.Context().Value(caddyhttp.ErrorCtxKey).(error); !ok {
+			w.Header().Add("Allow", "GET, HEAD")
+			return caddyhttp.Error(http.StatusMethodNotAllowed, nil)
+		}
 	}
 
 	// set the Etag - note that a conditional If-None-Match request is handled


Test patch: