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

regex route path allows partial match #545

Open
haitch opened this issue May 21, 2024 · 5 comments
Open

regex route path allows partial match #545

haitch opened this issue May 21, 2024 · 5 comments
Labels
v4 could be part of a v4 major version

Comments

@haitch
Copy link
Contributor

haitch commented May 21, 2024

to accept case-insensitive paths, we use {h:(?i)hello}, to accept /HELLO, /hello, /Hello.

but with this approach, it also triggered a bug where /xxxHello, /yyyHello also got accepted.

package main

import (
	"io"
	"log"
	"net/http"

	restful "github.com/emicklei/go-restful/v3"
)

func main() {
	ws := new(restful.WebService)
	ws.Route(ws.GET("/{h:(?i)hello}").To(hello))
	restful.Add(ws)

	log.Fatal(http.ListenAndServe(":8080", nil))
}

func hello(req *restful.Request, resp *restful.Response) {
	io.WriteString(resp, "world")
}
@haitch haitch changed the title case-sensitive on url path? route patch allows partial match May 21, 2024
@haitch haitch changed the title route patch allows partial match route path allows partial match May 21, 2024
@haitch haitch changed the title route path allows partial match regex route path allows partial match May 21, 2024
@haitch
Copy link
Contributor Author

haitch commented May 21, 2024

After dig into the code, I think the key is func (c CurlyRouter) regularMatchesPathToken(routeToken, colon, requestToken), where it try to match a requestToken xxxhello to a routeToken {h:(?i)hello}.

in my example setup, c.regularMatchesPathToken("{h:(?i)hello}", 3, "xxxhello") was invoked. inside the function, curly bracket was removed, then, it's matching "(?i)hello" to "xxxhello", it did match hello, despect the prefix xxx.

helloRegex := regexp.MustCompile("(?i)hello")
matches := helloRegex.FindStringSubmatch("xxxhello")
log.Println(matches) // []string{"hello"}

@haitch
Copy link
Contributor Author

haitch commented May 21, 2024

did find a workaround by using more strict regex {h:(?i)^hello$}, the ^$ would ensure whole token match.

@haitch
Copy link
Contributor Author

haitch commented May 21, 2024

send PR with example on how to handle this issue safely #546

@emicklei
Copy link
Owner

thank you for reporting this issue and investigating the problem. I will have a look at the PR.

@haitch
Copy link
Contributor Author

haitch commented May 22, 2024

thanks for merging the example PR, hopefully that will help someone.

As a library, I think we want to prevent caller from make mistakes and cause misroute, can we consider have regex to match the whole routeToken with next minor release where you can introduce break changes.

@emicklei emicklei added the v4 could be part of a v4 major version label Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v4 could be part of a v4 major version
Projects
None yet
Development

No branches or pull requests

2 participants