PATCH requests are being terminated by server

[kvrhdn]

Describe the bug

I came across a pretty gnarly issue with PATCH requests and our HTTP/gRPC server… #2681 added a PATCH endpoint to the user-configurable overrides, while this endpoint works fine in tests it does not work on the full Tempo app. Instead, PATCH requests are immediately terminated:

➜ curl -v -X PATCH http://localhost:3200/api/overrides --data '{}'
*   Trying 127.0.0.1:3200...
* Connected to localhost (127.0.0.1) port 3200 (#0)
> PATCH /api/overrides HTTP/1.1
> Host: localhost:3200
> User-Agent: curl/7.88.1
> Accept: */*
> Content-Length: 2
> Content-Type: application/x-www-form-urlencoded
>
* Recv failure: Connection reset by peer
* Closing connection 0
curl: (56) Recv failure: Connection reset by peer

I’ve been digging through the server code and the setting server.RouteHTTPToGRPC is at the root of this. We use this to serve HTTP and gRPC traffic on one endpoint. If I disable it, PATCH requests works fine.

Digging a bit deeper, this setting sets up a mux to route HTTP and gRPC requests to their respective listeners. It uses cmux.HTTP1Fast() to determine if the request is a HTTP requests:

tempo/vendor/github.com/weaveworks/common/server/server.go

Lines 256 to 261 in 8e4b812

	if cfg.RouteHTTPToGRPC {
	grpchttpmux = cmux.New(httpListener)
	httpListener = grpchttpmux.Match(cmux.HTTP1Fast())
	grpcOnHTTPListener = grpchttpmux.Match(cmux.HTTP2())
	}

https://github.com/weaveworks/common/blob/dd9e68f319d5ddfb68002a238d481265af23fa3b/server/server.go#L256-L261

The implementation of cmux.HTTP1Fast() only looks at the method of a request and matches it with a hard coded list... which does not have PATCH.

tempo/vendor/github.com/soheilhy/cmux/matchers.go

Lines 46 to 64 in 8e4b812

	var defaultHTTPMethods = []string{
	"OPTIONS",
	"GET",
	"HEAD",
	"POST",
	"PUT",
	"DELETE",
	"TRACE",
	"CONNECT",
	}
	// HTTP1Fast only matches the methods in the HTTP request.
	//
	// This matcher is very optimistic: if it returns true, it does not mean that
	// the request is a valid HTTP response. If you want a correct but slower HTTP1
	// matcher, use HTTP1 instead.
	func HTTP1Fast(extMethods ...string) Matcher {
	return PrefixMatcher(append(defaultHTTPMethods, extMethods...)...)
	}

https://github.com/soheilhy/cmux/blob/master/matchers.go#L46-L64

The solution would be to change the mux setup in weaveworks/common/server:

		httpListener = grpchttpmux.Match(cmux.HTTP1Fast(http.MethodPatch))

Or

		httpListener = grpchttpmux.Match(cmux.HTTP1())

To Reproduce
Steps to reproduce the behavior:

1. Run a recent version of Tempo including user-configurable overrides: handle versioning, add PATCH endpoint #2681
2. Make sure user-configurable overrides is enabled
3. curl -v -X PATCH http://localhost:3200/api/overrides --data '{}'

Expected behavior

PATCH requests are accepted and processed correctly:

➜ curl -v -X PATCH http://localhost:3200/api/overrides --data '{}'
*   Trying 127.0.0.1:3200...
* Connected to localhost (127.0.0.1) port 3200 (#0)
> PATCH /api/overrides HTTP/1.1
> Host: localhost:3200
> User-Agent: curl/7.88.1
> Accept: */*
> Content-Length: 2
> Content-Type: application/x-www-form-urlencoded
>
< HTTP/1.1 200 OK
< Content-Type: application/json
< Etag: 0
< Date: Wed, 02 Aug 2023 18:38:56 GMT
< Content-Length: 65
<
* Connection #0 to host localhost left intact
{"forwarders":[],"metrics_generator":{"disable_collection":true}}%

Environment:

• Infrastructure: my laptop
• Deployment tool: none

Additional Context