Email Notifier panics when unmarshalling headers from email config

[joeblubaugh]

What did you do?
Updated the github.com/grafana/mimir dependency on Prometheus to v0.24.1-0.20220805150254-c732372d7d3b

Unmarshalled some email notifier configuration yaml that unmarshalled successfully in v0.24.0

What did you expect to see?
The configuration would continue to unmarshal successfully.

What did you see instead? Under which circumstances?
A panic while title-casing HTTP header values.

Environment

• System information:

  insert output of uname -srm here

• Alertmanager version:
  v0.24.1-0.20220805150254-c732372d7d3b

• Alertmanager configuration file:
  This is a tenant-private configuration. We will update this issue with a reproducible string once we find one.

• Logs:

/vendor/gopkg.in/yaml.v2/decode.go:270 +0xa7
gopkg.in/yaml%2ev2.(*decoder).callUnmarshaler(0xc20be3f980, 0xc20c116e70, {0x7f170dda42b8, 0xc0430126c0})
/vendor/github.com/prometheus/alertmanager/config/notifiers.go:196 +0x1b9
github.com/prometheus/alertmanager/config.(*EmailConfig).UnmarshalYAML(0xc0430126c0, 0x2340b20?)
	/vendor/golang.org/x/text/cases/cases.go:51
golang.org/x/text/cases.Caser.String(...)
	/vendor/golang.org/x/text/transform/transform.go:650 +0xbe5
golang.org/x/text/transform.String({0x7f170e30ea18, 0xc0002d2a00}, {0xc20fd3aeb8, 0x2})
...
runtime error: slice bounds out of range [11:7]

This was introduced in #3017, where Title casing switched from strings.Title (deprecated) to golang.org/x/text/case title casing.

One fix would be to revert to string.Title, which does not panic on these inputs, and filing a bug with x/text/case when we can determine the input that panics.

Another possible way of addressing this issue is to avoid title casing of header names. Email header names are case insensitive: https://www.rfc-editor.org/rfc/rfc5322#section-1.2.2 (see also: https://stackoverflow.com/questions/6143549/are-email-headers-case-sensitive), so lower-casing every header with strings.ToLower() should be sufficient to check for collisions.

[simonpasquier]

Have you already filed a bug against golang.org/x/text/cases? I agree that using strings.ToLower() is probably safe but would need to check in the Git history if there was any reason for using Title().

[joeblubaugh]

I haven't yet. I ran a fuzz test this morning but wasn't able to trigger a panic. I'll file one with x/text/cases in the next 24h.

[simonpasquier]

The use of strings.Title() has been added in #147 hence 1) it's been there forever and 2) I see no concrete argument in the original PR for using Title() rather than ToLower() or ToUpper().

[joeblubaugh]

I've filed: golang/go#55032

There's currently a maintainer handoff in process: golang/go#54481

[joeblubaugh]

What do you think about converting to strings.ToLower? Given case-insensitivity, it's a much simpler transformation.

[roidelapluie]

We could use prometheus/blackbox_exporter#937

[simonpasquier]

We could use prometheus/blackbox_exporter#937

sounds good to me too.

[joeblubaugh]

I read through CanonicalMimeHeaderKey and it doesn't do any fancy stuff with text/transform, despite the indirect dependency elsewhere in the package. It looks solid to me.