go icon indicating copy to clipboard operation
go copied to clipboard

expvar: creates conflict with "GET /" route

Open xpmatteo opened this issue 1 year ago • 5 comments

Go version

1.22.0

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/Users/matteo/Library/Caches/go-build'
GOENV='/Users/matteo/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/matteo/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/matteo/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/Cellar/go/1.22.0/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/Cellar/go/1.22.0/libexec/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/tb/wk8169j96gbcyz_jrppmchvm0000gn/T/go-build2446679495=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

With the new routing style in go 1.22, declaring

    http.Handle("GET /", h)

generates a conflict with route "/debug/vars" declared in the expvar package.

What did you see happen?

panic: pattern "GET /" (registered at ...) conflicts with pattern "/debug/vars" (registered at ...expvar.go:384):
GET / matches fewer methods than /debug/vars, but has a more general path pattern

What did you expect to see?

No error.

It can be fixed with this patch

--- a/src/expvar/expvar.go
+++ b/src/expvar/expvar.go
@@ -381,7 +381,7 @@ func memstats() any {
 }

 func init() {
-       http.HandleFunc("/debug/vars", expvarHandler)
+       http.HandleFunc("GET /debug/vars", expvarHandler)
        Publish("cmdline", Func(cmdline))
        Publish("memstats", Func(memstats))
 }

xpmatteo avatar Feb 15 '24 15:02 xpmatteo

I expect that similar problems will be caused by the many other places in the std lib where http.HandleFunc is invoked with the old syntax

xpmatteo avatar Feb 15 '24 15:02 xpmatteo

Change https://go.dev/cl/564435 mentions this issue: expvar: avoid conflict with a user-defined "GET /" route. Fixes #65723

gopherbot avatar Feb 15 '24 15:02 gopherbot

I guess it needs also to check whether the GODEBUG httpmuxgo121 == 1, so that it still works for code that runs with httpmuxgo121=0.

mateusz834 avatar Feb 15 '24 19:02 mateusz834

I guess it needs also to check whether the GODEBUG httpmuxgo121 == 1, so that it still works for code that runs with httpmuxgo121=0.

Indeed, this is needed, otherwise specifying GODEBUG httpmuxgo121=1 would make /debug/vars no longer available. I updated the patch accordingly.

(I closed the previous PR by mistake, so I had to open a new one. Newbie mistake!)

xpmatteo avatar Feb 16 '24 07:02 xpmatteo

Change https://go.dev/cl/564735 mentions this issue: expvar: avoid conflict with user-defined "GET /" route. Fixes #65723

gopherbot avatar Feb 16 '24 07:02 gopherbot

@bradfitz per owners

thanm avatar Feb 16 '24 13:02 thanm

@jba FYI

AlexanderYastrebov avatar Feb 16 '24 13:02 AlexanderYastrebov

net/http/pprof will also have a similar issue.

seankhliao avatar Feb 16 '24 13:02 seankhliao

@xpmatteo, would you like to handle net/http/pprof while you're at it?

jba avatar Feb 16 '24 13:02 jba

@xpmatteo, would you like to handle net/http/pprof while you're at it?

I will do it; should I open a separate PR?

xpmatteo avatar Feb 16 '24 15:02 xpmatteo

Yes please.

jba avatar Feb 16 '24 16:02 jba

Change https://go.dev/cl/565176 mentions this issue: net/http/pprof: avoid panic with user-defined "GET /" route

gopherbot avatar Feb 19 '24 13:02 gopherbot

FYI: This change broke some applications https://github.com/grafana/pyroscope-go/pull/119

korniltsev avatar Aug 15 '24 10:08 korniltsev

You can also use GODEBUG=httpmuxgo121=1 to adapt.

ianlancetaylor avatar Aug 15 '24 13:08 ianlancetaylor

You can also use GODEBUG=httpmuxgo121=1 to adapt.

Yeah, the problem is that i do not own the app. It is a library. We're fixing the library.

korniltsev avatar Aug 15 '24 13:08 korniltsev