Skip to content

Commit d63ab8d

Browse files
babakksCopilot
andcommitted
fix(discussion/shared): error on out-of-range discussion number in URL
The discussion URL pattern captures one or more digits, which can exceed the int32 range. The parse error from strconv.ParseInt was previously discarded, silently clamping the number. Return an explicit error instead, and cover it with tests in both URL parsing functions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5d77247 commit d63ab8d

2 files changed

Lines changed: 19 additions & 2 deletions

File tree

pkg/cmd/discussion/shared/lookup.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,11 @@ func ParseDiscussionArg(arg string) (int32, ghrepo.Interface, error) {
3838
return 0, nil, fmt.Errorf("invalid discussion URL: %q", arg)
3939
}
4040

41-
num, _ := strconv.ParseInt(m[3], 10, 32)
41+
num, err := strconv.ParseInt(m[3], 10, 32)
42+
if err != nil {
43+
return 0, nil, fmt.Errorf("invalid discussion number in URL: %q", m[3])
44+
}
45+
4246
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
4347
return int32(num), repo, nil
4448
}
@@ -84,7 +88,10 @@ func ParseDiscussionOrCommentArg(arg string) (*ParsedDiscussionOrCommentArg, err
8488
return nil, fmt.Errorf("invalid discussion URL: %q", arg)
8589
}
8690

87-
num, _ := strconv.ParseInt(m[3], 10, 32)
91+
num, err := strconv.ParseInt(m[3], 10, 32)
92+
if err != nil {
93+
return nil, fmt.Errorf("invalid discussion number in URL: %q", m[3])
94+
}
8895
repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname())
8996

9097
if fragment := u.Fragment; strings.HasPrefix(fragment, "discussioncomment-") {

pkg/cmd/discussion/shared/lookup_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ func TestParseDiscussionArg(t *testing.T) {
5252
arg: "https://github.com/owner/repo/discussions/",
5353
wantErr: `invalid discussion URL: "https://github.com/owner/repo/discussions/"`,
5454
},
55+
{
56+
name: "URL with overflowing number",
57+
arg: "https://github.com/owner/repo/discussions/99999999999999999999",
58+
wantErr: `invalid discussion number in URL: "99999999999999999999"`,
59+
},
5560
{
5661
name: "zero",
5762
arg: "0",
@@ -166,6 +171,11 @@ func TestParseDiscussionOrCommentArg(t *testing.T) {
166171
arg: "https://github.com/owner/repo/discussions/",
167172
wantErr: `invalid discussion URL: "https://github.com/owner/repo/discussions/"`,
168173
},
174+
{
175+
name: "URL with overflowing number",
176+
arg: "https://github.com/owner/repo/discussions/99999999999999999999",
177+
wantErr: `invalid discussion number in URL: "99999999999999999999"`,
178+
},
169179
{
170180
name: "comment URL with invalid fragment",
171181
arg: "https://github.com/owner/repo/discussions/5#discussioncomment-abc",

0 commit comments

Comments
 (0)