emicklei / go-restful

package for building REST-style Web Services using Go
MIT License
5.03k stars 690 forks source link

regex route path allows partial match #545

Open haitch opened 3 months ago

haitch commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

haitch commented 3 months ago

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

emicklei commented 3 months ago

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

haitch commented 3 months ago

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.