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

If user formats rawURL to contain '#' before channel name - execution ends up in segfault #70

Closed
atighineanu opened this issue Nov 5, 2020 · 2 comments

Comments

@atighineanu
Copy link
Contributor

atighineanu commented Nov 5, 2020

When I tried shoutrrr code for a rocket chat automated notification, and formatted the channel name with a '#' prefix - that is what I got:

go run cli/main.go send --url "rocketchat://shoutrrr@<myhost>/<tokenA>/<tokenB>/#general" --message "hello hello"
panic: runtime error: slice bounds out of range [:1] with length 0
goroutine 1 [running]:
github.com/containrrr/shoutrrr/pkg/services/rocketchat.(*Config).SetURL(0xc0000c27e0, 0xc0000d8120, 0x1548080, 0xc0000b8980)
        /Users/alexeitighineanu/go/src/github.com/shoutrrr/pkg/services/rocketchat/rocketchat_config.go:69 +0x77d
github.com/containrrr/shoutrrr/pkg/services/rocketchat.(*Service).Initialize(0xc0000b8980, 0xc0000d8120, 0xc0000c8280, 0xa, 0xc0000b1938)
        /Users/alexeitighineanu/go/src/github.com/shoutrrr/pkg/services/rocketchat/rocketchat.go:24 +0x85
github.com/containrrr/shoutrrr/pkg/router.(*ServiceRouter).initService(0x191ce20, 0x7ffeefbff694, 0x73, 0x15aa11a, 0x6, 0x15d0868, 0x1501b60)
        /Users/alexeitighineanu/go/src/github.com/shoutrrr/pkg/router/router.go:171 +0xe6
github.com/containrrr/shoutrrr/pkg/router.(*ServiceRouter).Locate(...)
        /Users/alexeitighineanu/go/src/github.com/shoutrrr/pkg/router/router.go:203
github.com/containrrr/shoutrrr.Send(0x7ffeefbff694, 0x73, 0x7ffeefbff712, 0x15, 0x15, 0x0)
        /Users/alexeitighineanu/go/src/github.com/shoutrrr/shoutrrr.go:19 +0x45
github.com/containrrr/shoutrrr/cli/cmd/send.Run(0x1914580, 0xc0000dce00, 0x0, 0x4)
        /Users/alexeitighineanu/go/src/github.com/shoutrrr/cli/cmd/send/send.go:49 +0x146
github.com/spf13/cobra.(*Command).execute(0x1914580, 0xc0000dcdc0, 0x4, 0x4, 0x1914580, 0xc0000dcdc0)
        /Users/alexeitighineanu/go/pkg/mod/github.com/spf13/[email protected]/command.go:842 +0x2c2
github.com/spf13/cobra.(*Command).ExecuteC(0x19142e0, 0x0, 0x0, 0x0)
        /Users/alexeitighineanu/go/pkg/mod/github.com/spf13/[email protected]/command.go:943 +0x336
github.com/spf13/cobra.(*Command).Execute(...)
        /Users/alexeitighineanu/go/pkg/mod/github.com/spf13/[email protected]/command.go:883
main.main()
        /Users/alexeitighineanu/go/src/github.com/shoutrrr/cli/main.go:9 +0x31
exit status 2

It is pretty confusing given that it is caused by a '#' symbol. I've discovered it is caused by this statement in
pkg/services/rocketchat_configure.go:

if len(path) > 3 {
		if path[3][0:1] != "@" {                          <--------HERE
			config.Channel = "#" + path[3]

Because path[3] in my case was empty. url.Parse red the path just until the '#' symbol.

Therefore, here are two possible remedies for this:
a) whether check at url.Parse() stage for escaped fragments (in pkg/router/router.go):

func (router *ServiceRouter) ExtractServiceName(rawURL string) (string, *url.URL, error) {
	serviceURL, err := url.Parse(rawURL)
	if serviceURL.EscapedFragment() != "" && strings.Contains(rawURL, "/#") {
		fmt.Println("Please, do not use \"#\" before channel name")
		serviceURL.Path += serviceURL.EscapedFragment()
	}
	if err != nil {
		return "", &url.URL{}, err
	}
	return serviceURL.Scheme, serviceURL, nil
}

b) or check for escaped fragments just in rocket chat's case (if somebody is worrying about introducing bugs affecting other services):
(in pkg/services/rocketchat_configure.go:)

if path[3] == "" && serviceURL.EscapedFragment() != "" {
  fmt.Println("Please, do not use \"#\" before channel name")
  path[3] = serviceURL.EscapedFragment()
}

Let me know what you think, thanks.

@piksel
Copy link
Member

piksel commented Nov 5, 2020

Yeah, since that is rocketchat specific, it should probably be handled inside the service (or rather, it's config parser), especially as this would prevent all other services from ever using the fragment part of the URLs.

I guess we should either explicitly allow the fragment to be used as the channel name, or throw an error when we detect it, not do both. But otherwise I totally agree, getting a segfault for using the wrong syntax is terrible feedback for the consumer!

ellisab added a commit to ellisab/shoutrrr that referenced this issue Nov 13, 2020
Signed-off-by: Alexandru Bonini <[email protected]>
simskij pushed a commit that referenced this issue Nov 15, 2020
@atighineanu
Copy link
Contributor Author

solved by #74

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