Skip to content

Commit 536966d

Browse files
authored
Fix leaked client transport on copy (#3051)
Fixes: #3043.
1 parent d26bcb8 commit 536966d

2 files changed

Lines changed: 35 additions & 3 deletions

File tree

‎github/github.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -450,15 +450,18 @@ func (c *Client) copy() *Client {
450450
c.clientMu.Lock()
451451
// can't use *c here because that would copy mutexes by value.
452452
clone := Client{
453-
client: c.client,
453+
client: &http.Client{},
454454
UserAgent: c.UserAgent,
455455
BaseURL: c.BaseURL,
456456
UploadURL: c.UploadURL,
457457
secondaryRateLimitReset: c.secondaryRateLimitReset,
458458
}
459459
c.clientMu.Unlock()
460-
if clone.client == nil {
461-
clone.client = &http.Client{}
460+
if c.client != nil {
461+
clone.client.Transport = c.client.Transport
462+
clone.client.CheckRedirect = c.client.CheckRedirect
463+
clone.client.Jar = c.client.Jar
464+
clone.client.Timeout = c.client.Timeout
462465
}
463466
c.rateMu.Lock()
464467
copy(clone.rateLimits[:], c.rateLimits[:])

‎github/github_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2599,3 +2599,32 @@ func TestParseTokenExpiration(t *testing.T) {
25992599
}
26002600
}
26012601
}
2602+
2603+
func TestClientCopy_leak_transport(t *testing.T) {
2604+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
2605+
w.Header().Set("Content-Type", "application/json")
2606+
accessToken := r.Header.Get("Authorization")
2607+
_, _ = fmt.Fprintf(w, `{"login": "%s"}`, accessToken)
2608+
}))
2609+
clientPreconfiguredWithURLs, err := NewClient(nil).WithEnterpriseURLs(srv.URL, srv.URL)
2610+
if err != nil {
2611+
t.Fatal(err)
2612+
}
2613+
2614+
aliceClient := clientPreconfiguredWithURLs.WithAuthToken("alice")
2615+
bobClient := clientPreconfiguredWithURLs.WithAuthToken("bob")
2616+
2617+
alice, _, err := aliceClient.Users.Get(context.Background(), "")
2618+
if err != nil {
2619+
t.Fatal(err)
2620+
}
2621+
2622+
assertNoDiff(t, "Bearer alice", alice.GetLogin())
2623+
2624+
bob, _, err := bobClient.Users.Get(context.Background(), "")
2625+
if err != nil {
2626+
t.Fatal(err)
2627+
}
2628+
2629+
assertNoDiff(t, "Bearer bob", bob.GetLogin())
2630+
}

0 commit comments

Comments
 (0)