[php-fpm] Possibility of index out of range [0]

[zak-pawel]

Found possible problem with index out of range in plugins/inputs/phpfpm/phpfpm.go:

func globUnixSocket(url string) ([]string, error) {
	pattern, status := unixSocketPaths(url)
	glob, err := globpath.Compile(pattern)
	if err != nil {
		return nil, fmt.Errorf("could not compile glob %q: %v", pattern, err)
	}
	paths := glob.Match()
	if len(paths) == 0 {
		if _, err := os.Stat(paths[0]); err != nil {
			if os.IsNotExist(err) {
				return nil, fmt.Errorf("Socket doesn't exist  '%s': %s", pattern, err)
			}
			return nil, err
		}
		return nil, nil
	}

For this condition if len(paths) == 0 (empty array) this code will be executed: os.Stat(paths[0]).
Probably will end with panic: runtime error: index out of range [0] with length 0

It was introduced in #7089 (exactly in this commit: 2a7bf89) after discussion between @andrenth and @danielnelson.
I can fix this but I need a guidance about desired behavior.

[zak-pawel]

Testing if panic appears is quite simple:

func TestPhpFpmGeneratesMetrics_Should_Panic_When_Socket_Path_Containing_Meta_Is_Invalid(t *testing.T) {
	r := &phpfpm{
		Urls: []string{"/tmp/*/invalid.sock"},
	}

	err := r.Init()
	require.NoError(t, err)

	var acc testutil.Accumulator

	defer func() { recover() }()
	err = acc.GatherError(r.Gather)
	t.Errorf("r.Gather should have panicked")
}

[ssoroka]

that whole stat block makes no sense. if they really wanted it to error on empty glob matches, it should just return a new error.

[andrenth]

I think this is a leftover from the previous commit which tried to allow an empty match on the glob but forbid a nonexistent static path. It seems to me the fix is to simply remove the os.Stat call and just return the error as @ssoroka mentioned, ie.

if len(paths) == 0 {
    return nil, fmt.Errorf("Socket doesn't exist  '%s': %s", pattern, err)
}