Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions collector/filesystem_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,17 @@ func parseFilesystemLabels(mountInfo []*procfs.MountInfo) ([]filesystemLabels, e
mount.MountPoint = strings.ReplaceAll(mount.MountPoint, "\\040", " ")
mount.MountPoint = strings.ReplaceAll(mount.MountPoint, "\\011", "\t")

// Ensure label values are valid UTF-8 to avoid panics in
// prometheus.MustNewConstMetric. Linux paths are byte strings
// and can contain non-UTF-8 sequences.
mountPoint := strings.ToValidUTF8(rootfsStripPrefix(mount.MountPoint), "\uFFFD")
device := strings.ToValidUTF8(mount.Source, "\uFFFD")
fsType := strings.ToValidUTF8(mount.FSType, "\uFFFD")

filesystems = append(filesystems, filesystemLabels{
device: mount.Source,
mountPoint: rootfsStripPrefix(mount.MountPoint),
fsType: mount.FSType,
device: device,
mountPoint: mountPoint,
fsType: fsType,
mountOptions: mountOptionsString(mount.Options),
superOptions: mountOptionsString(mount.SuperOptions),
major: strconv.Itoa(major),
Expand Down
122 changes: 121 additions & 1 deletion collector/filesystem_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,133 @@
import (
"io"
"log/slog"
"strings"
"testing"
"unicode/utf8"

"github.com/alecthomas/kingpin/v2"

"github.com/prometheus/procfs"
)

func Test_parseFilesystemLabelsNonUTF8(t *testing.T) {
tests := []struct {
name string
in []*procfs.MountInfo
wantMountPoint string
wantDevice string
}{
{
name: "non-utf8 mount point is sanitized",
in: []*procfs.MountInfo{
{
MajorMinorVer: "8:1",
MountPoint: "/mnt/cass\xe9",
Source: "/dev/sda1",
FSType: "ext4",
Options: map[string]string{},
SuperOptions: map[string]string{},
},
},
wantMountPoint: "/mnt/cass\ufffd",
wantDevice: "/dev/sda1",
},
{
name: "non-utf8 device is sanitized",
in: []*procfs.MountInfo{
{
MajorMinorVer: "8:1",
MountPoint: "/mnt/data",
Source: "/dev/sd\xe9",
FSType: "ext4",
Options: map[string]string{},
SuperOptions: map[string]string{},
},
},
wantMountPoint: "/mnt/data",
wantDevice: "/dev/sd\ufffd",
},
{
name: "valid utf8 is preserved",
in: []*procfs.MountInfo{
{
MajorMinorVer: "8:1",
MountPoint: "/mnt/caf\u00e9",
Source: "/dev/sda1",
FSType: "ext4",
Options: map[string]string{},
SuperOptions: map[string]string{},
},
},
wantMountPoint: "/mnt/caf\u00e9",
wantDevice: "/dev/sda1",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
filesystems, err := parseFilesystemLabels(tt.in)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(filesystems) != 1 {
t.Fatalf("expected 1 filesystem, got %d", len(filesystems))
}
fs := filesystems[0]
if fs.mountPoint != tt.wantMountPoint {
t.Errorf("mountPoint = %q, want %q", fs.mountPoint, tt.wantMountPoint)
}
if fs.device != tt.wantDevice {
t.Errorf("device = %q, want %q", fs.device, tt.wantDevice)
}
// Verify all label values are valid UTF-8.
for _, v := range []string{fs.device, fs.mountPoint, fs.fsType} {
if !utf8.ValidString(v) {
t.Errorf("label value %q is not valid UTF-8", v)
}
}
})
}
}

func Test_parseFilesystemLabelsNonUTF8DoesNotPanic(t *testing.T) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test largely overlaps the table test, both guard the same revert. Its only unique coverage is an invalid fsType; consider folding that into the table test and dropping this one. If you'd rather keep a named regression test for #3662, make it exercise MustNewConstMetric so it actually reproduces the panic, right now neither test hits that path.

// Verify that non-UTF-8 data does not cause a panic when
// constructing Prometheus metrics (the root cause of #3662).
input := []*procfs.MountInfo{
{
MajorMinorVer: "8:1",
MountPoint: "/mnt/bad\xffpath",
Source: "/dev/sd\x00a1",
FSType: "ext\xfe4",
Options: map[string]string{},
SuperOptions: map[string]string{},
},
}

filesystems, err := parseFilesystemLabels(input)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(filesystems) != 1 {
t.Fatalf("expected 1 filesystem, got %d", len(filesystems))
}

// All label values must be valid UTF-8.
for _, v := range []string{
filesystems[0].device,
filesystems[0].mountPoint,
filesystems[0].fsType,
} {
if !utf8.ValidString(v) {
t.Errorf("label value %q is not valid UTF-8", v)
}
// Must not contain any raw invalid bytes (replacement char is OK).
if strings.ContainsRune(v, 0xFFFD) {
t.Logf("label value contains replacement character (expected for invalid input): %q", v)
}
}
}

func Test_parseFilesystemLabelsError(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -122,8 +242,8 @@
"/run/rpc_pipefs": "",
"/run/user/1000": "",
"/run/user/1000/gvfs": "",
"/var/lib/kubelet/plugins/kubernetes.io/vsphere-volume/mounts/[vsanDatastore] bafb9e5a-8856-7e6c-699c-801844e77a4a/kubernetes-dynamic-pvc-3eba5bba-48a3-11e8-89ab-005056b92113.vmdk": "",

Check failure on line 245 in collector/filesystem_linux_test.go

View workflow job for this annotation

GitHub Actions / lint

File is not properly formatted (goimports)
"/var/lib/kubelet/plugins/kubernetes.io/vsphere-volume/mounts/[vsanDatastore] bafb9e5a-8856-7e6c-699c-801844e77a4a/kubernetes-dynamic-pvc-3eba5bba-48a3-11e8-89ab-005056b92113.vmdk": "",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required by the Non-UTF8 bug fix?

"/var/lib/kubelet/plugins/kubernetes.io/vsphere-volume/mounts/[vsanDatastore]\tbafb9e5a-8856-7e6c-699c-801844e77a4a/kubernetes-dynamic-pvc-3eba5bba-48a3-11e8-89ab-005056b92113.vmdk": "",
"/var/lib/containers/storage/overlay": "",
}

Expand Down
Loading