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

Panic when supplying malformed URL for -external.web-path #1229

Closed
bluecmd opened this Issue Nov 19, 2015 · 4 comments

Comments

Projects
None yet
4 participants
@bluecmd
Copy link
Contributor

bluecmd commented Nov 19, 2015

When I tried to use -external.web-path in 0.16.1 I got the following panic:

wildcards must be named with a non-empty name in path '/'https://url/prometheus'/'

This turns out to be because I erroneously supplied extra quotes, but as @fabxc says it probably shouldn't panic but just print an error message.

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented Nov 19, 2015

Mh, could you provide the exact input and maybe the panic stack trace. I'm not quite sure where we would be throwing a panic.

@bluecmd

This comment has been minimized.

Copy link
Contributor Author

bluecmd commented Nov 20, 2015

bluecmd@dhmon:~/prometheus-0.16.1.linux-amd64$ ./prometheus -web.external-url="'http://test/ads'" 
prometheus, version 0.16.1 (branch: release-0.16.1, revision: 968ee35)
  build user:       fabianreinartz@macpro
  build date:       20151016-13:19:57
  go version:       1.5
panic: wildcards must be named with a non-empty name in path '/'http://test/ads'/'

goroutine 1 [running]:
github.com/prometheus/prometheus/vendor/github.com/julienschmidt/httprouter.(*node).insertChild(0xc82008da40, 0x1, 0xc8201130c1, 0x12, 0xc8201130c0, 0x13, 0xc8201c42b0)
    /Users/fabianreinartz/repos/src/github.com/prometheus/prometheus/vendor/github.com/julienschmidt/httprouter/tree.go:233 +0x46b
github.com/prometheus/prometheus/vendor/github.com/julienschmidt/httprouter.(*node).addRoute(0xc82008da40, 0xc8201130c1, 0x12, 0xc8201c42b0)
    /Users/fabianreinartz/repos/src/github.com/prometheus/prometheus/vendor/github.com/julienschmidt/httprouter/tree.go:185 +0xb21
github.com/prometheus/prometheus/vendor/github.com/julienschmidt/httprouter.(*Router).Handle(0xc82018f140, 0xdcf208, 0x3, 0xc8201130c0, 0x13, 0xc8201c42b0)
    /Users/fabianreinartz/repos/src/github.com/prometheus/prometheus/vendor/github.com/julienschmidt/httprouter/router.go:229 +0x22b
github.com/prometheus/prometheus/vendor/github.com/julienschmidt/httprouter.(*Router).GET(0xc82018f140, 0xc8201130c0, 0x13, 0xc8201c42b0)
    /Users/fabianreinartz/repos/src/github.com/prometheus/prometheus/vendor/github.com/julienschmidt/httprouter/router.go:173 +0x54
github.com/prometheus/prometheus/vendor/github.com/prometheus/common/route.(*Router).Get(0xc8201130a0, 0xdcde10, 0x1, 0xc8201c42a0)
    /Users/fabianreinartz/repos/src/github.com/prometheus/prometheus/vendor/github.com/prometheus/common/route/route.go:75 +0xa5
github.com/prometheus/prometheus/web.New(0x7f7405dd9080, 0xc8201043c0, 0xc82018f100, 0xc820089620, 0xc820089680, 0x1351858, 0xc820112fc0)
    /Users/fabianreinartz/repos/src/github.com/prometheus/prometheus/web/web.go:164 +0x608
main.Main(0x0)
    /Users/fabianreinartz/repos/src/github.com/prometheus/prometheus/cmd/prometheus/main.go:110 +0xa55
main.main()
    /Users/fabianreinartz/repos/src/github.com/prometheus/prometheus/cmd/prometheus/main.go:46 +0x18

goroutine 30 [syscall]:
os/signal.loop()
    /usr/local/opt/go/libexec/src/os/signal/signal_unix.go:22 +0x18
created by os/signal.init.1
    /usr/local/opt/go/libexec/src/os/signal/signal_unix.go:28 +0x37
@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Nov 21, 2015

Ok, so the problem is that github.com/julienschmidt/httprouter panics when an invalid path is provided, and that url.Parse in https://github.com/prometheus/prometheus/blob/master/cmd/prometheus/config.go#L263 sets the path to the entire provided string if it's in quotes.

So unless we want to swap out the underlying HTTP library, we'll have to sanity-check the path / external URL by ourselves somehow.

@brian-brazil brian-brazil added the bug label Dec 16, 2015

@fabxc fabxc closed this in #1340 Jan 27, 2016

pgier added a commit to pgier/prometheus that referenced this issue Oct 4, 2017

cmd/prometheus: don't allow quotes at beginning or end of url
This prevents accidental copy/paste error where a the web.external-url
or alertmanager.url params could have an extra set of quotes.
See also: prometheus#1229
@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.