Skip to content

Commit

Permalink
Fix leaked client transport on copy (#3051)
Browse files Browse the repository at this point in the history
Fixes: #3043.
  • Loading branch information
gmlewis committed Jan 28, 2024
1 parent d26bcb8 commit 536966d
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
9 changes: 6 additions & 3 deletions github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,15 +450,18 @@ func (c *Client) copy() *Client {
c.clientMu.Lock()
// can't use *c here because that would copy mutexes by value.
clone := Client{
client: c.client,
client: &http.Client{},
UserAgent: c.UserAgent,
BaseURL: c.BaseURL,
UploadURL: c.UploadURL,
secondaryRateLimitReset: c.secondaryRateLimitReset,
}
c.clientMu.Unlock()
if clone.client == nil {
clone.client = &http.Client{}
if c.client != nil {
clone.client.Transport = c.client.Transport
clone.client.CheckRedirect = c.client.CheckRedirect
clone.client.Jar = c.client.Jar
clone.client.Timeout = c.client.Timeout
}
c.rateMu.Lock()
copy(clone.rateLimits[:], c.rateLimits[:])
Expand Down
29 changes: 29 additions & 0 deletions github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2599,3 +2599,32 @@ func TestParseTokenExpiration(t *testing.T) {
}
}
}

func TestClientCopy_leak_transport(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
accessToken := r.Header.Get("Authorization")
_, _ = fmt.Fprintf(w, `{"login": "%s"}`, accessToken)
}))
clientPreconfiguredWithURLs, err := NewClient(nil).WithEnterpriseURLs(srv.URL, srv.URL)
if err != nil {
t.Fatal(err)
}

aliceClient := clientPreconfiguredWithURLs.WithAuthToken("alice")
bobClient := clientPreconfiguredWithURLs.WithAuthToken("bob")

alice, _, err := aliceClient.Users.Get(context.Background(), "")
if err != nil {
t.Fatal(err)
}

assertNoDiff(t, "Bearer alice", alice.GetLogin())

bob, _, err := bobClient.Users.Get(context.Background(), "")
if err != nil {
t.Fatal(err)
}

assertNoDiff(t, "Bearer bob", bob.GetLogin())
}

0 comments on commit 536966d

Please sign in to comment.