fix: Relax id parsing strictness for existing ids (#3129)

Signed-off-by: Steve Hipwell <[email protected]>

Author: stevehipwell

github/util.go

@@ -21,20 +21,20 @@ import (
 
 const (
 	idSeparator        = ":"
-	idSeperatorEscaped = `??`
+	idSeparatorEscaped = `??`
 )
 
 // https://developer.github.com/guides/traversing-with-pagination/#basics-of-pagination
 var maxPerPage = 100
 
 // escapeIDPart escapes any idSeparator characters in a string.
 func escapeIDPart(part string) string {
-	return strings.ReplaceAll(part, idSeparator, idSeperatorEscaped)
+	return strings.ReplaceAll(part, idSeparator, idSeparatorEscaped)
 }
 
 // unescapeIDPart unescapes any escaped idSeparator characters in a string.
 func unescapeIDPart(part string) string {
-	return strings.ReplaceAll(part, idSeperatorEscaped, idSeparator)
+	return strings.ReplaceAll(part, idSeparatorEscaped, idSeparator)
 }
 
 // buildID joins the parts with the idSeparator.
@@ -44,12 +44,13 @@ func buildID(parts ...string) (string, error) {
 		return "", fmt.Errorf("no parts provided to build id")
 	}
 
-	id := strings.Join(parts, idSeparator)
-
-	if p := strings.Split(id, idSeparator); len(p) != l {
-		return "", fmt.Errorf("unescaped seperators in id parts %v", parts)
+	for i, p := range parts {
+		if i < l-1 && strings.Contains(p, idSeparator) {
+			return "", fmt.Errorf("unescaped separator in non-final part %q", p)
+		}
 	}
 
+	id := strings.Join(parts, idSeparator)
 	return id, nil
 }
 
@@ -59,7 +60,7 @@ func parseID(id string, count int) ([]string, error) {
 		return nil, fmt.Errorf("id is empty")
 	}
 
-	parts := strings.Split(id, idSeparator)
+	parts := strings.SplitN(id, idSeparator, count)
 	if len(parts) != count {
 		return nil, fmt.Errorf("unexpected ID format (%q); expected %d parts separated by %q", id, count, idSeparator)
 	}

github/util_test.go

@@ -107,8 +107,14 @@ func Test_buildID(t *testing.T) {
 			hasError: false,
 		},
 		{
-			testName: "part_with_unescaped_separator",
-			parts:    []string{"part1", "part:2", "part3"},
+			testName: "part_with_unescaped_separator_at_end",
+			parts:    []string{"part1", "part2", "part3:extra"},
+			expect:   "part1:part2:part3:extra",
+			hasError: false,
+		},
+		{
+			testName: "part_with_unescaped_separator_in_middle",
+			parts:    []string{"part1", "part2:extra", "part3"},
 			expect:   "",
 			hasError: true,
 		},
@@ -148,14 +154,14 @@ func Test_parseID(t *testing.T) {
 		hasError bool
 	}{
 		{
-			testName: "two_parts_expected_two",
+			testName: "two_parts",
 			id:       "part1:part2",
 			count:    2,
 			expect:   []string{"part1", "part2"},
 			hasError: false,
 		},
 		{
-			testName: "three_parts_expected_three",
+			testName: "three_parts",
 			id:       "part1:part2:part3",
 			count:    3,
 			expect:   []string{"part1", "part2", "part3"},
@@ -169,11 +175,11 @@ func Test_parseID(t *testing.T) {
 			hasError: true,
 		},
 		{
-			testName: "three_parts_expected_two",
-			id:       "part1:part2:part3",
+			testName: "two_parts_with_extra",
+			id:       "part1:part2:extra",
 			count:    2,
-			expect:   nil,
-			hasError: true,
+			expect:   []string{"part1", "part2:extra"},
+			hasError: false,
 		},
 		{
 			testName: "empty_id",
@@ -223,11 +229,11 @@ func Test_parseID2(t *testing.T) {
 			hasError: false,
 		},
 		{
-			testName: "invalid_three_parts",
-			id:       "part1:part2:part3",
-			expect1:  "",
-			expect2:  "",
-			hasError: true,
+			testName: "valid_two_parts_with_extra",
+			id:       "part1:part2:extra",
+			expect1:  "part1",
+			expect2:  "part2:extra",
+			hasError: false,
 		},
 		{
 			testName: "invalid_one_part",
@@ -280,16 +286,16 @@ func Test_parseID3(t *testing.T) {
 			hasError: false,
 		},
 		{
-			testName: "invalid_two_parts",
-			id:       "part1:part2",
-			expect1:  "",
-			expect2:  "",
-			expect3:  "",
-			hasError: true,
+			testName: "valid_three_parts_with_extra",
+			id:       "part1:part2:part3:extra",
+			expect1:  "part1",
+			expect2:  "part2",
+			expect3:  "part3:extra",
+			hasError: false,
 		},
 		{
-			testName: "invalid_four_parts",
-			id:       "part1:part2:part3:part4",
+			testName: "invalid_two_parts",
+			id:       "part1:part2",
 			expect1:  "",
 			expect2:  "",
 			expect3:  "",