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

prometheus.InvalidDesc.String() is formatted incorrectly #1684

Open
SealOfTime opened this issue Nov 12, 2024 · 1 comment · May be fixed by #1687
Open

prometheus.InvalidDesc.String() is formatted incorrectly #1684

SealOfTime opened this issue Nov 12, 2024 · 1 comment · May be fixed by #1687
Labels
Milestone

Comments

@SealOfTime
Copy link

On v1.20.5, if String() method of a prometheus.Desc returned from NewInvalidDesc called, it will cause a nil pointer derefence, which will result in an incorrectly formatted string

type Desc struct {
	//...
	variableLabels *compiledLabels
        //...
}

func NewInvalidDesc(err error) *Desc {
	return &Desc{
		err: err,
	}
}

func (d *Desc) String() string {
	//...
	vlStrings := make([]string, 0, len(d.variableLabels.names))
	for _, vl := range d.variableLabels.names {
        //...
}

NewInvalidDesc will create a Desc with variableLabels equal to nil, therefore accessing its field names will cause a nil-pointer dereference.

The result is: descriptor %!s(PANIC=String method: runtime error: invalid memory address or nil pointer dereference) is invalid: test

The consequence of that is an unpleasant error message in prometheus.Register

Copy link

dosubot bot commented Nov 12, 2024

Hey there, @SealOfTime! I'm here to assist you with any bugs, questions, or contributions while we wait for a human maintainer. Let's squash those bugs together! 🤖

To prevent a nil pointer dereference in the String() method of a prometheus.Desc when NewInvalidDesc creates a Desc with variableLabels set to nil, you can add a nil check for d.variableLabels before accessing its fields. Here's how you can modify the String() method:

func (d *Desc) String() string {
	lpStrings := make([]string, 0, len(d.constLabelPairs))
	for _, lp := range d.constLabelPairs {
		lpStrings = append(
			lpStrings,
			fmt.Sprintf("%s=%q", lp.GetName(), lp.GetValue()),
		)
	}
	
	vlStrings := []string{}
	if d.variableLabels != nil {
		vlStrings = make([]string, 0, len(d.variableLabels.names))
		for _, vl := range d.variableLabels.names {
			if fn, ok := d.variableLabels.labelConstraints[vl]; ok && fn != nil {
				vlStrings = append(vlStrings, fmt.Sprintf("c(%s)", vl))
			} else {
				vlStrings = append(vlStrings, vl)
			}
		}
	}
	
	return fmt.Sprintf(
		"Desc{fqName: %q, help: %q, constLabels: {%s}, variableLabels: {%s}}",
		d.fqName,
		d.help,
		strings.Join(lpStrings, ","),
		strings.Join(vlStrings, ","),
	)
}

This modification ensures that the code checks if d.variableLabels is nil before attempting to access its names field, thus preventing a nil pointer dereference [1].

To continue talking to Dosu, mention @dosu.

@dosubot dosubot bot added the bug label Nov 12, 2024
@kakkoyun kakkoyun added this to the v1.21.0 milestone Nov 19, 2024
kakkoyun added a commit that referenced this issue Nov 19, 2024
…for nil label values

Fixes #1684

Signed-off-by: Kemal Akkoyun <[email protected]>
kakkoyun added a commit that referenced this issue Nov 19, 2024
…for nil label values

Fixes #1684

Signed-off-by: Kemal Akkoyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants