Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Routing bug with empty path component #609

Open
yurivish opened this issue Mar 27, 2021 · 6 comments · May be fixed by #637
Open

Routing bug with empty path component #609

yurivish opened this issue Mar 27, 2021 · 6 comments · May be fixed by #637
Assignees
Labels

Comments

@yurivish
Copy link

yurivish commented Mar 27, 2021

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 yurivish changed the title Odd Regex Match Path match bug with empty path component Mar 27, 2021
@yurivish yurivish changed the title Path match bug with empty path component Routing bug with empty path component Mar 27, 2021
@sashayakovtseva
Copy link

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.

@yurivish
Copy link
Author

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.

@helios741
Copy link

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 added a commit to helios741/chi that referenced this issue Jul 13, 2021
helios741 added a commit to helios741/chi that referenced this issue Jul 13, 2021
helios741 added a commit to helios741/chi that referenced this issue Jul 13, 2021
@helios741 helios741 linked a pull request Jul 13, 2021 that will close this issue
@BLumia
Copy link

BLumia commented Sep 17, 2022

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.

@ilovejs
Copy link

ilovejs commented Feb 2, 2024

any progress for #609 (comment) @pkieltyka

@wafer-bw
Copy link

wafer-bw commented Apr 22, 2024

Surprising this bug still exists many years later. The work around I'm looking into using right now is:

// ...

r.Route("//", func(r chi.Router) {
    // blackhole *//* requests
})

r.Route("/key:[a-z]+"), func(r chi.Router) {
    // ...
})

// ...

chi will reply with 404 page not found


Alternatively you can just use a middleware that ensures the parameter matches your regex manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants