From a21179ae69866d1c1d391441f3e380a362b44af5 Mon Sep 17 00:00:00 2001
From: Jim Ramsay <jramsay@redhat.com>
Date: Thu, 25 Mar 2021 15:03:08 -0400
Subject: [PATCH] cpuset.Parse: Fix edge cases and add negative tests

The cpuset.Parse function missed a couple bad input cases, specifically
"1--3" and "10-6".  These were silently ignored when they should instead
be flagged as invalid.

This now catches these cases and expands the unit tests for cpuset to
cover them (and other negative test cases as well).

Signed-off-by: Jim Ramsay <jramsay@redhat.com>
---
 pkg/kubelet/cm/cpuset/cpuset.go      |  7 ++++++-
 pkg/kubelet/cm/cpuset/cpuset_test.go | 23 +++++++++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/pkg/kubelet/cm/cpuset/cpuset.go b/pkg/kubelet/cm/cpuset/cpuset.go
index de72ba25756..24330e5a9e3 100644
--- a/pkg/kubelet/cm/cpuset/cpuset.go
+++ b/pkg/kubelet/cm/cpuset/cpuset.go
@@ -301,7 +301,7 @@ func Parse(s string) (CPUSet, error) {
 	ranges := strings.Split(s, ",")
 
 	for _, r := range ranges {
-		boundaries := strings.Split(r, "-")
+		boundaries := strings.SplitN(r, "-", 2)
 		if len(boundaries) == 1 {
 			// Handle ranges that consist of only one element like "34".
 			elem, err := strconv.Atoi(boundaries[0])
@@ -319,6 +319,11 @@ func Parse(s string) (CPUSet, error) {
 			if err != nil {
 				return NewCPUSet(), err
 			}
+			if start > end {
+				return NewCPUSet(), fmt.Errorf("invalid range %q (%d >= %d)", r, start, end)
+			}
+			// start == end is acceptable (1-1 -> 1)
+
 			// Add all elements to the result.
 			// e.g. "0-5", "46-48" => [0, 1, 2, 3, 4, 5, 46, 47, 48].
 			for e := start; e <= end; e++ {
diff --git a/pkg/kubelet/cm/cpuset/cpuset_test.go b/pkg/kubelet/cm/cpuset/cpuset_test.go
index c77ab8b7543..d79748df506 100644
--- a/pkg/kubelet/cm/cpuset/cpuset_test.go
+++ b/pkg/kubelet/cm/cpuset/cpuset_test.go
@@ -323,7 +323,7 @@ func TestCPUSetString(t *testing.T) {
 }
 
 func TestParse(t *testing.T) {
-	testCases := []struct {
+	positiveTestCases := []struct {
 		cpusetString string
 		expected     CPUSet
 	}{
@@ -332,9 +332,12 @@ func TestParse(t *testing.T) {
 		{"1,2,3,4,5", NewCPUSet(1, 2, 3, 4, 5)},
 		{"1-5", NewCPUSet(1, 2, 3, 4, 5)},
 		{"1-2,3-5", NewCPUSet(1, 2, 3, 4, 5)},
+		{"5,4,3,2,1", NewCPUSet(1, 2, 3, 4, 5)},  // Range ordering
+		{"3-6,1-5", NewCPUSet(1, 2, 3, 4, 5, 6)}, // Overlapping ranges
+		{"3-3,5-5", NewCPUSet(3, 5)},             // Very short ranges
 	}
 
-	for _, c := range testCases {
+	for _, c := range positiveTestCases {
 		result, err := Parse(c.cpusetString)
 		if err != nil {
 			t.Fatalf("expected error not to have occurred: %v", err)
@@ -343,4 +346,20 @@ func TestParse(t *testing.T) {
 			t.Fatalf("expected string \"%s\" to parse as [%v] (got [%v])", c.cpusetString, c.expected, result)
 		}
 	}
+
+	negativeTestCases := []string{
+		// Non-numeric entries
+		"nonnumeric", "non-numeric", "no,numbers", "0-a", "a-0", "0,a", "a,0", "1-2,a,3-5",
+		// Incomplete sequences
+		"0,", "0,,", ",3", ",,3", "0,,3",
+		// Incomplete ranges and/or negative numbers
+		"-1", "1-", "1,2-,3", "1,-2,3", "-1--2", "--1", "1--",
+		// Reversed ranges
+		"3-0", "0--3"}
+	for _, c := range negativeTestCases {
+		result, err := Parse(c)
+		if err == nil {
+			t.Fatalf("expected parse failure of \"%s\", but it succeeded as \"%s\"", c, result.String())
+		}
+	}
 }