Skip to content

Commit d759cc4

Browse files
authored
Merge pull request #41 from luna-veil-8080/main
Fix regression in github enterprise url handling #40
2 parents c454a71 + 7b20ea4 commit d759cc4

File tree

2 files changed

+76
-14
lines changed

2 files changed

+76
-14
lines changed

github.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"io"
99
"net/http"
1010
"net/url"
11+
"strings"
1112
"time"
1213
)
1314

@@ -101,6 +102,16 @@ func (c *githubClient) withEnterpriseURL(baseURL string) (*githubClient, error)
101102
return nil, fmt.Errorf("failed to parse base URL: %w", err)
102103
}
103104

105+
if !strings.HasSuffix(base.Path, "/") {
106+
base.Path += "/"
107+
}
108+
109+
if !strings.HasSuffix(base.Path, "/api/v3/") &&
110+
!strings.HasPrefix(base.Host, "api.") &&
111+
!strings.Contains(base.Host, ".api.") {
112+
base.Path += "api/v3/"
113+
}
114+
104115
c.baseURL = base
105116

106117
return c, nil

github_test.go

Lines changed: 65 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,33 +12,84 @@ import (
1212

1313
func Test_githubClient_withEnterpriseURL(t *testing.T) {
1414
tests := []struct {
15-
name string
16-
baseURL string
17-
wantErr bool
15+
name string
16+
baseURL string
17+
wantErr bool
18+
expectedBaseURL string
1819
}{
1920
{
20-
name: "valid URL",
21-
baseURL: "https://github.example.com",
22-
wantErr: false,
21+
name: "valid URL with first subdomain",
22+
baseURL: "https://api.github.example.com",
23+
wantErr: false,
24+
expectedBaseURL: "https://api.github.example.com/",
25+
},
26+
{
27+
name: "valid URL with first subdomain and trailing slash",
28+
baseURL: "https://api.github.example.com/",
29+
wantErr: false,
30+
expectedBaseURL: "https://api.github.example.com/",
31+
},
32+
{
33+
name: "valid URL with second subdomain",
34+
baseURL: "https://ghes.api.example.com",
35+
wantErr: false,
36+
expectedBaseURL: "https://ghes.api.example.com/",
37+
},
38+
{
39+
name: "valid URL with second subdomain and trailing slash",
40+
baseURL: "https://ghes.api.example.com/",
41+
wantErr: false,
42+
expectedBaseURL: "https://ghes.api.example.com/",
43+
},
44+
{
45+
name: "valid URL with path",
46+
baseURL: "https://github.example.com/api/v3",
47+
wantErr: false,
48+
expectedBaseURL: "https://github.example.com/api/v3/",
2349
},
2450
{
25-
name: "invalid URL with control characters",
26-
baseURL: "ht\ntp://invalid",
27-
wantErr: true,
51+
name: "valid URL with path and trailing slash",
52+
baseURL: "https://github.example.com/api/v3/",
53+
wantErr: false,
54+
expectedBaseURL: "https://github.example.com/api/v3/",
2855
},
2956
{
30-
name: "URL with spaces",
31-
baseURL: "http://invalid url with spaces",
32-
wantErr: true,
57+
name: "valid URL without path",
58+
baseURL: "https://github.example.com",
59+
wantErr: false,
60+
expectedBaseURL: "https://github.example.com/api/v3/",
61+
},
62+
{
63+
name: "valid URL without path but with trailing slash",
64+
baseURL: "https://github.example.com/",
65+
wantErr: false,
66+
expectedBaseURL: "https://github.example.com/api/v3/",
67+
},
68+
{
69+
name: "invalid URL with control characters",
70+
baseURL: "ht\ntp://invalid",
71+
wantErr: true,
72+
expectedBaseURL: "",
73+
},
74+
{
75+
name: "URL with spaces",
76+
baseURL: "http://invalid url with spaces",
77+
wantErr: true,
78+
expectedBaseURL: "",
3379
},
3480
}
3581

3682
for _, tt := range tests {
3783
t.Run(tt.name, func(t *testing.T) {
3884
client := newGitHubClient(&http.Client{})
39-
_, err := client.withEnterpriseURL(tt.baseURL)
85+
githubClient, err := client.withEnterpriseURL(tt.baseURL)
86+
4087
if (err != nil) != tt.wantErr {
41-
t.Errorf("withEnterpriseURL() error = %v, wantErr %v", err, tt.wantErr)
88+
t.Errorf("withEnterpriseURL(%v) error = %v", tt.baseURL, err)
89+
}
90+
91+
if err == nil && githubClient.baseURL.String() != tt.expectedBaseURL {
92+
t.Errorf("withEnterpriseURL(%v) expected = %v, received = %v", tt.baseURL, tt.expectedBaseURL, githubClient.baseURL)
4293
}
4394
})
4495
}

0 commit comments

Comments
 (0)