chi icon indicating copy to clipboard operation
chi copied to clipboard

Routing bug with empty path component

Open yurivish opened this issue 3 years ago • 6 comments

Hi, I found a surprising (to me) edge case regarding routes with empty path components:

package main

import (
	"fmt"
	"net/http"

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

func empty(w http.ResponseWriter, r *http.Request) {}

func main() {
	r := chi.NewRouter()
	r.Get(`/{:[a-z]+}/`, empty)
	ctx := chi.NewRouteContext()
	if r.Match(ctx, "GET", "//") {
		fmt.Println("The route matched the URL.")
	}
}

This prints The route matched the URL. even though there was clearly no match for the regular expression. My understanding of the semantics of a regex filter is that the route will match if and only if the regex filter matches the path component, but that's not what seems to be happening here.

I'd be curious if this is behaving as it should, and if so, if there's a good alternative to filtering out empty path components, since requirning a match on .+ doesn't work.

Thank you!

P. S. I think this a bug, since omitting the trailing slash from both the pattern and URL should not change the behavior but doing so makes the route no longer match (click to expand):
package main

import (
	"fmt"
	"net/http"

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

func empty(w http.ResponseWriter, r *http.Request) {}

func main() {
	r := chi.NewRouter()
	r.Get(`/{:[a-z]+}`, empty)
	ctx := chi.NewRouteContext()
	if r.Match(ctx, "GET", "/") {
		fmt.Println("The route matched the URL.")
	}
}

yurivish avatar Mar 27 '21 03:03 yurivish

Having the same issue with subroutes:


import (
	"fmt"
	"net/http"

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

func empty(w http.ResponseWriter, r *http.Request) {}

func main() {
	r := chi.NewRouter()
	r.Route("/{:[1-9]\\d*}", func(r chi.Router) {
		r.Get(`/test`, empty)
	})

	ctx := chi.NewRouteContext()
	if r.Match(ctx, "GET", "/123/test") {
		fmt.Println("The route matched the URL.")
	}
	if r.Match(ctx, "GET", "//test") {
		fmt.Println("The route matched the URL.")
	}
}

Second match is not expected.

sashayakovtseva avatar May 13 '21 08:05 sashayakovtseva

One note about empty route segments is that some browser (like Chrome) will automatically remove them during URL normalization. It's still probably correct to consider this a bug, but in practice having empty route segments should be discouraged due to the fact that the user may not be able to navigate to them.

yurivish avatar May 13 '21 19:05 yurivish

This bug is caused by not setting xn to nil in time when ntype is equal to ntRegexp and traversing the child nodes. the xn in these places should be set to empty in time.tree.go#L432tree.go#L438tree.go#L442.

I want to fix this bug, please assign this issue to me. @pkieltyka

helios741 avatar Jul 12 '21 13:07 helios741

Having the same issue with subroutes:

import (
	"fmt"
	"net/http"

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

func dummy(w http.ResponseWriter, r *http.Request) {}

func main() {
	r := chi.NewRouter()
	r.Route(`/{handle:[a-zA-Z\d._-]+}`, func(r chi.Router) {
		r.Get(`/`, dummy)
		// ...
	})
	http.ListenAndServe(":3333", r)
}

With this example, http://localhost:3333// will also match the route even if + is used in the regex which requires at least one character in the parameter.

BLumia avatar Sep 17 '22 09:09 BLumia

any progress for https://github.com/go-chi/chi/issues/609#issuecomment-1250040746 @pkieltyka

ilovejs avatar Feb 02 '24 03:02 ilovejs