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

detectWebService biased against webServices without path or rootpath #560

Open
haitch opened this issue Jul 8, 2024 · 1 comment
Open
Labels
v4 could be part of a v4 major version

Comments

@haitch
Copy link
Contributor

haitch commented Jul 8, 2024

When there are mix use of default webService and regexPath webService, the default webService may get biased and its route won't get selected.

with 2 webService setup like this:

  • one with default routes (empty Path, or rootPath '/')
  • one with regex Path
    during detectWebService, computeWebserviceScore, with a request trying to hit the default route.
  • defaultRoute would always get score 0
  • and regexPath would always get score 1 (regex not match)
    this would select wrong webService 100%, and won't find the correct route.

since I have fix on this part with issue #547 and PR #549 , I would image the fix is remove the default score for any regex routeToken, but that would definitely be a breaking change.

reproduce, also here is go playground link: https://go.dev/play/p/xbpUnvAETio

package main

import (
	"fmt"
	"io"
	"net/http/httptest"

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

func main() {
	defaultWS := new(restful.WebService) // or .Path("/")
	helloWS := new(restful.WebService).Path("/{:hello}")

	defaultWS.Route(defaultWS.GET("/{:robot.txt}").To(robotFile))
	restful.DefaultContainer.Add(defaultWS)

	helloWS.Route(helloWS.GET("/{name:*}").To(hello))
	restful.DefaultContainer.Add(helloWS) // comment out this line, and robot.txt will be served

	helloRequest := httptest.NewRequest("GET", "/hello/Juan", nil)
	helloRespWriter := httptest.NewRecorder()
	restful.DefaultContainer.ServeHTTP(helloRespWriter, helloRequest)

	robotRequest := httptest.NewRequest("GET", "/robot.txt", nil)
	robotRespWriter := httptest.NewRecorder()
	restful.DefaultContainer.ServeHTTP(robotRespWriter, robotRequest)

	fmt.Println(helloRequest.Method, helloRequest.URL.Path, "-", helloRespWriter.Result().Status, helloRespWriter.Body.String())
	fmt.Println(robotRequest.Method, robotRequest.URL.Path, "-", robotRespWriter.Result().Status, robotRespWriter.Body.String())
}

func hello(req *restful.Request, resp *restful.Response) {
	io.WriteString(resp, "hello "+req.PathParameter("name"))
}

func robotFile(req *restful.Request, resp *restful.Response) {
	io.WriteString(resp, "User-agent: *\nDisallow: /\n")
}
@emicklei
Copy link
Owner

"a breaking change" is what will prevent me from making a change in v3. We could add the v4 label to this issue.

@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