golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.41k stars 17.7k forks source link

x/net/trace: registered routes conflict with "GET /" #69951

Open zamai opened 1 month ago

zamai commented 1 month ago

Go version

go version go1.23.2 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/alex/Library/Caches/go-build'
GOENV='/Users/alex/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/alex/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/alex/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.23.2/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.2'
GODEBUG=''
GOTELEMETRY='on'
GOTELEMETRYDIR='/Users/alex/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/alex/Code/test-trace-bug/go.mod'
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 arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/lg/kw6gcgh126lf1nw8q7wjssc00000gn/T/go-build2664326740=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

https://go.dev/play/p/oQEzMhqVvwn

When I added dependency "cloud.google.com/go/storage" (or any other google library that has sub dependency of "golang.org/x/net/trace") + handler func with "GET /", go run panics with error:

panic: pattern "GET /" (registered at /Users/alex/Code/test-trace-bug/main.go:12) conflicts with pattern "/debug/requests" (registered at /Users/alex/go/pkg/mod/golang.org/x/net@v0.29.0/trace/trace.go:130):

GET / matches fewer methods than /debug/requests, but has a more general path pattern

What did you see happen?

output of the go run command with the example program(https://go.dev/play/p/oQEzMhqVvwn)

-> % go run main.go
panic: pattern "GET /" (registered at /Users/alex/Code/test-trace-bug/main.go:12) conflicts with pattern "/debug/requests" (registered at /Users/alex/go/pkg/mod/golang.org/x/net@v0.29.0/trace/trace.go:130):
    GET / matches fewer methods than /debug/requests, but has a more general path pattern

goroutine 1 [running]:
net/http.(*ServeMux).register(...)
    /opt/homebrew/Cellar/go/1.23.2/libexec/src/net/http/server.go:2797
net/http.HandleFunc({0x1059e1273?, 0x1069f3860?}, 0x0?)
    /opt/homebrew/Cellar/go/1.23.2/libexec/src/net/http/server.go:2791 +0x9c
main.main()
    /Users/alex/Code/test-trace-bug/main.go:12 +0x50
exit status 2

What did you expect to see?

Expected build to work.

Let me know if this is an error related to x/net/trace or google cloud storage package.

gabyhelp commented 1 month ago

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

seankhliao commented 1 month ago

I suppose x/net/trace can register the paths with GET if the go version is >= 1.21 (though it would be inaccurate in the face of someone setting GODEBUG=httpmuxgo121=0 explicitly), and it may be considered breaking for it to no longer respond to other methods.

neild commented 1 month ago

/cc @jba

jba commented 1 month ago

It's unfortunate that it registers routes with the default ServeMux in init, so there's no way to avoid it.

20478 exported the handler functions, so anyone can register them on any ServeMux. But it's unfortunate that they didn't also provide a way to turn off registration in the default ServeMux.

So @zamai, one workaround is to create your own ServeMux for your own routes using a prefix, and let x/net/trace have the global routes. That's not very satisfactory, though.

Another choice is to have a way to turn off x/net/trace's default registration. But since that happens in init, it would have to be an environment variable.

The third choice is being clever about when to add GET to the routes, looking at the go version and GODEBUG flag.

Any preferences?

gopherbot commented 1 month ago

Change https://go.dev/cl/621176 mentions this issue: golang.org/x/net: have conflict with path "GET /"

zamai commented 1 month ago

thanks @jba, The workaround that I implemented in this case is to drop the "GET" directive from the root handler: http.HandleFunc("/", helloHandler) This created inconsistency in my codebase: I specify HTTP methods in all handler registration except the root one.

I don't understand the design of the trace package enough to give thoughtful opinion, but as a user I think it's strange/unexpected that /debug/requests route is added to my handler when I import some package that uses the trace.

It would be okay if I did the explicit import, but in my case I stumbled upon it when I added "cloud.google.com/go/storage" package to my app.

jba commented 1 month ago

I agree, registering paths in init is a design flaw.

dominiquelefevre commented 1 month ago

There is more trouble with registering those routes in the package init function.

Render() uses html/template to generate responses, and html/template uses GetMethodByName() on random strings extracted from the template. When the go compiler finds a call to GetMethodByName() with a non-constant method name, it disables the DCE altogether. Since Render() is referenced by the init function, simply importing x/net/trace disables the DCE. This pessimises the code generation in a lot of applications since google.golang.org/grpc imports x/net/trace.

I believe that x/net/traces must not register routes in the init.

rmszwast commented 3 weeks ago

Ran into this same issue. Here is my workaround:

    mux := http.NewServeMux()
    mux.Handle("GET /", http.FileServer(http.Dir("")))
    log.Fatal(http.ListenAndServe(PORT, mux))
zamai commented 2 weeks ago

Ran into this same issue. Here is my workaround:

  mux := http.NewServeMux()
  mux.Handle("GET /", http.FileServer(http.Dir("")))
  log.Fatal(http.ListenAndServe(PORT, mux))

good to know, valid workaround for anyone who will stumble upon this.