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

IndexOutofBound panic during RemoveRoute() #471

Closed
jeffreydwalter opened this issue Jul 12, 2021 · 3 comments
Closed

IndexOutofBound panic during RemoveRoute() #471

jeffreydwalter opened this issue Jul 12, 2021 · 3 comments

Comments

@jeffreydwalter
Copy link
Contributor

The issue referenced in #276 is not really fixed by the commit that was merged in.

There is still a panic here

newRoutes[current] = w.routes[ix]
when you call RemoveRoute() with a route that is not in w.routes.

You can reproduce the issue with this code:

func main() {
	ws := new(WebService)
	ws.SetDynamicRoutes(true)
	ws.Route("GET", "/some/path")
	ws.RemoveRoute("/some/path/that/does/not/exist", "GET")
	ws.Routes()
}

I created a go playground that reproduces the issue here: https://play.golang.org/p/SYUMGQeY0cU

Changing RemoveRoute() to this fixes the issue:

func (w *WebService) RemoveRoute(path, method string) error {
	if !w.dynamicRoutes {
		return errors.New("dynamic routes are not enabled.")
	}
	w.routesLock.Lock()
	defer w.routesLock.Unlock()
	newRoutes := make([]Route, 0)
	for ix := range w.routes {
		if w.routes[ix].Method == method && w.routes[ix].Path == path {
			continue
		}
		newRoutes = append(newRoutes, w.routes[ix])
	}
	w.routes = newRoutes
	return nil
}
@jeffreydwalter
Copy link
Contributor Author

I created a PR which includes the suggested fix here: #472

@emicklei
Copy link
Owner

thank you reporting this and providing a fix using the PR. I will review the changes and merge on approval.

@jeffreydwalter
Copy link
Contributor Author

Fixed in PR #472.

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

No branches or pull requests

2 participants