diff --git a/github/util.go b/github/util.go index 736615a9cc..97d4dfcb0b 100644 --- a/github/util.go +++ b/github/util.go @@ -21,7 +21,7 @@ import ( const ( idSeparator = ":" - idSeperatorEscaped = `??` + idSeparatorEscaped = `??` ) // https://developer.github.com/guides/traversing-with-pagination/#basics-of-pagination @@ -29,12 +29,12 @@ 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) } diff --git a/github/util_test.go b/github/util_test.go index bd2b4fb9c9..e18bbb5a4a 100644 --- a/github/util_test.go +++ b/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: "",