Undefined Route Request Leads to Infinite Loop in route matching

[soheilrt]

I was implementing an HTTP server for my service and noticed that when I use 'chi' as shown below, the router does not respond to undefined routes. Additionally, I suspect the router gets stuck in an infinite loop, leading to a memory leak in the service.

package main

import (
	"fmt"
	"github.com/go-chi/chi/v5"
	"github.com/go-chi/chi/v5/middleware"
	"net/http"
	_ "net/http/pprof"
)

func main() {
	r := chi.NewRouter().With(middleware.Logger)
	r.Mount("/", r.Group(func(r chi.Router) {
		r.Get("/ping", func(writer http.ResponseWriter, request *http.Request) {
			writer.WriteHeader(http.StatusOK)
			writer.Write([]byte("pong"))
		})
	}))

	fmt.Println("%v", http.ListenAndServe(":8080", r))
}

For the above-mentioned code, GET /ping request will be handled properly, but router never returns a response for an undefined request path.

[Kidsan]

I was playing around with this bug a bit and found that if you mount the middleware with Use() instead of With() (because With() is for inline middleware, i.e. router.With(<handler-specific-mw>).Get(...) your example actually crashes with a fatal error for me.

package main

import (
	"fmt"
	"net/http"

	"github.com/go-chi/chi/v5"
	"github.com/go-chi/chi/v5/middleware"
)

func main() {
	r := chi.NewRouter()
	r.Use(middleware.Logger)
	r.Mount("/", r.Group(func(r chi.Router) {
		r.Get("/ping", func(writer http.ResponseWriter, request *http.Request) {
			writer.WriteHeader(http.StatusOK)
			writer.Write([]byte("pong"))
		})
	}))

	fmt.Println("%v", http.ListenAndServe(":8080", r))
}

runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0xc0201e0408 stack=[0xc0201e0000, 0xc0401e0000]
fatal error: stack overflow

this seems to be due to some weirdness when doing r.Mount("/", r.Group(...)), seems like this is unnecessary anyway (?) but should not cause this behavior I think.

[Kidsan]

This issue was actually raised previously: #663

[soheilrt]

Hi @Kidsan,

Thanks for checking this out.

In general, I agree with you that with should be used for the route middleware definition only.

There is only one thing - I believe libraries should prevent misuse as much as possible and this case seems to be misuse, unless it's intended this way. If this is intended this way, then we have a bug here that needs to be addressed.

Also, regarding the usage of Use method and stack overflow error, it seems the error still exists, only in a different form!

Happy to hear your point of view on this!

[Kidsan]

@soheilrt I don't disagree 😄 would like a maintainer to chime in though, based on the other issue I linked it looks like they aren't interested in presenting "obvious" misuse.

[soheilrt]

Hey @pkieltyka , can we have your toughts on this as well?

[newacorn]

func (mx *Mux) Mount(pattern string, handler http.Handler) {
....
	subr, ok := handler.(*Mux)
	if ok {
		if subr.tree == mx.tree {
			panic("subRouter's tree equals its own.")
		}
	}