Skip to content

Commit ecee52e

Browse files
committed
fix: Relax id parsing strictness for existing ids
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
1 parent cf15eae commit ecee52e

File tree

2 files changed

+36
-29
lines changed

2 files changed

+36
-29
lines changed

github/util.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,20 @@ import (
2121

2222
const (
2323
idSeparator = ":"
24-
idSeperatorEscaped = `??`
24+
idSeparatorEscaped = `??`
2525
)
2626

2727
// https://developer.github.com/guides/traversing-with-pagination/#basics-of-pagination
2828
var maxPerPage = 100
2929

3030
// escapeIDPart escapes any idSeparator characters in a string.
3131
func escapeIDPart(part string) string {
32-
return strings.ReplaceAll(part, idSeparator, idSeperatorEscaped)
32+
return strings.ReplaceAll(part, idSeparator, idSeparatorEscaped)
3333
}
3434

3535
// unescapeIDPart unescapes any escaped idSeparator characters in a string.
3636
func unescapeIDPart(part string) string {
37-
return strings.ReplaceAll(part, idSeperatorEscaped, idSeparator)
37+
return strings.ReplaceAll(part, idSeparatorEscaped, idSeparator)
3838
}
3939

4040
// buildID joins the parts with the idSeparator.
@@ -44,12 +44,13 @@ func buildID(parts ...string) (string, error) {
4444
return "", fmt.Errorf("no parts provided to build id")
4545
}
4646

47-
id := strings.Join(parts, idSeparator)
48-
49-
if p := strings.Split(id, idSeparator); len(p) != l {
50-
return "", fmt.Errorf("unescaped seperators in id parts %v", parts)
47+
for i, p := range parts {
48+
if i < l-1 && strings.Contains(p, idSeparator) {
49+
return "", fmt.Errorf("unescaped separator in non-final part %q", p)
50+
}
5151
}
5252

53+
id := strings.Join(parts, idSeparator)
5354
return id, nil
5455
}
5556

@@ -59,7 +60,7 @@ func parseID(id string, count int) ([]string, error) {
5960
return nil, fmt.Errorf("id is empty")
6061
}
6162

62-
parts := strings.Split(id, idSeparator)
63+
parts := strings.SplitN(id, idSeparator, count)
6364
if len(parts) != count {
6465
return nil, fmt.Errorf("unexpected ID format (%q); expected %d parts separated by %q", id, count, idSeparator)
6566
}

github/util_test.go

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,14 @@ func Test_buildID(t *testing.T) {
107107
hasError: false,
108108
},
109109
{
110-
testName: "part_with_unescaped_separator",
111-
parts: []string{"part1", "part:2", "part3"},
110+
testName: "part_with_unescaped_separator_at_end",
111+
parts: []string{"part1", "part2", "part3:extra"},
112+
expect: "part1:part2:part3:extra",
113+
hasError: false,
114+
},
115+
{
116+
testName: "part_with_unescaped_separator_in_middle",
117+
parts: []string{"part1", "part2:extra", "part3"},
112118
expect: "",
113119
hasError: true,
114120
},
@@ -148,14 +154,14 @@ func Test_parseID(t *testing.T) {
148154
hasError bool
149155
}{
150156
{
151-
testName: "two_parts_expected_two",
157+
testName: "two_parts",
152158
id: "part1:part2",
153159
count: 2,
154160
expect: []string{"part1", "part2"},
155161
hasError: false,
156162
},
157163
{
158-
testName: "three_parts_expected_three",
164+
testName: "three_parts",
159165
id: "part1:part2:part3",
160166
count: 3,
161167
expect: []string{"part1", "part2", "part3"},
@@ -169,11 +175,11 @@ func Test_parseID(t *testing.T) {
169175
hasError: true,
170176
},
171177
{
172-
testName: "three_parts_expected_two",
173-
id: "part1:part2:part3",
178+
testName: "two_parts_with_extra",
179+
id: "part1:part2:extra",
174180
count: 2,
175-
expect: nil,
176-
hasError: true,
181+
expect: []string{"part1", "part2:extra"},
182+
hasError: false,
177183
},
178184
{
179185
testName: "empty_id",
@@ -223,11 +229,11 @@ func Test_parseID2(t *testing.T) {
223229
hasError: false,
224230
},
225231
{
226-
testName: "invalid_three_parts",
227-
id: "part1:part2:part3",
228-
expect1: "",
229-
expect2: "",
230-
hasError: true,
232+
testName: "valid_two_parts_with_extra",
233+
id: "part1:part2:extra",
234+
expect1: "part1",
235+
expect2: "part2:extra",
236+
hasError: false,
231237
},
232238
{
233239
testName: "invalid_one_part",
@@ -280,16 +286,16 @@ func Test_parseID3(t *testing.T) {
280286
hasError: false,
281287
},
282288
{
283-
testName: "invalid_two_parts",
284-
id: "part1:part2",
285-
expect1: "",
286-
expect2: "",
287-
expect3: "",
288-
hasError: true,
289+
testName: "valid_three_parts_with_extra",
290+
id: "part1:part2:part3:extra",
291+
expect1: "part1",
292+
expect2: "part2",
293+
expect3: "part3:extra",
294+
hasError: false,
289295
},
290296
{
291-
testName: "invalid_four_parts",
292-
id: "part1:part2:part3:part4",
297+
testName: "invalid_two_parts",
298+
id: "part1:part2",
293299
expect1: "",
294300
expect2: "",
295301
expect3: "",

0 commit comments

Comments
 (0)