Skip to content

Commit a4d01b4

Browse files
committed
change DownloadReleaseAsset API to additionally return a redirectURL
If a redirect to AWS occurs, then the redirect will be canceled and the redirect Location will be returned in redirectURL and the io.ReadCloser will be nil. At this point, the user would need to perform the http.Get with the redirectURL in a different http.Client (without the GitHub authentication transport). Note that this is a breaking API change. Fixes #246. Change-Id: I4fe2dfc9cd0ce81934a07f11a6296e71c7dd51a0
1 parent add6dcc commit a4d01b4

File tree

3 files changed

+32
-23
lines changed

3 files changed

+32
-23
lines changed

github/github.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ const (
6868
type Client struct {
6969
// HTTP client used to communicate with the API.
7070
client *http.Client
71+
// clientMu protects the client during calls that modify the CheckRedirect func.
72+
clientMu sync.Mutex
7173

7274
// Base URL for API requests. Defaults to the public GitHub API, but can be
7375
// set to a domain endpoint to use with GitHub Enterprise. BaseURL should

github/repos_releases.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ import (
1010
"fmt"
1111
"io"
1212
"mime"
13+
"net/http"
1314
"os"
1415
"path/filepath"
16+
"strings"
1517
)
1618

1719
// RepositoryRelease represents a GitHub release in a repository.
@@ -213,27 +215,43 @@ func (s *RepositoriesService) GetReleaseAsset(owner, repo string, id int) (*Rele
213215
return asset, resp, err
214216
}
215217

216-
// DownloadReleaseAsset downloads a release asset.
218+
// DownloadReleaseAsset downloads a release asset or returns a redirect URL.
217219
//
218220
// DownloadReleaseAsset returns an io.ReadCloser that reads the contents of the
219221
// specified release asset. It is the caller's responsibility to close the ReadCloser.
222+
// If a redirect is returned, the redirect URL will be returned as a string instead
223+
// of the io.ReadCloser. Exactly one of rc and redirectURL will be zero.
220224
//
221225
// GitHub API docs : http://developer.github.com/v3/repos/releases/#get-a-single-release-asset
222-
func (s *RepositoriesService) DownloadReleaseAsset(owner, repo string, id int) (io.ReadCloser, error) {
226+
func (s *RepositoriesService) DownloadReleaseAsset(owner, repo string, id int) (rc io.ReadCloser, redirectURL string, err error) {
223227
u := fmt.Sprintf("repos/%s/%s/releases/assets/%d", owner, repo, id)
224228

225229
req, err := s.client.NewRequest("GET", u, nil)
226230
if err != nil {
227-
return nil, err
231+
return nil, "", err
228232
}
229233
req.Header.Set("Accept", defaultMediaType)
230234

235+
s.client.clientMu.Lock()
236+
defer s.client.clientMu.Unlock()
237+
238+
var loc string
239+
saveRedirect := s.client.client.CheckRedirect
240+
s.client.client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
241+
loc = req.URL.String()
242+
return errors.New("disable redirect")
243+
}
244+
defer func() { s.client.client.CheckRedirect = saveRedirect }()
245+
231246
resp, err := s.client.client.Do(req)
232247
if err != nil {
233-
return nil, err
248+
if !strings.Contains(err.Error(), "disable redirect") {
249+
return nil, "", err
250+
}
251+
return nil, loc, nil
234252
}
235253

236-
return resp.Body, nil
254+
return resp.Body, "", nil
237255
}
238256

239257
// EditReleaseAsset edits a repository release asset.

github/repos_releases_test.go

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"net/http"
1414
"os"
1515
"reflect"
16+
"strings"
1617
"testing"
1718
)
1819

@@ -218,7 +219,7 @@ func TestRepositoriesService_DownloadReleaseAsset_Stream(t *testing.T) {
218219
fmt.Fprint(w, "Hello World")
219220
})
220221

221-
reader, err := client.Repositories.DownloadReleaseAsset("o", "r", 1)
222+
reader, _, err := client.Repositories.DownloadReleaseAsset("o", "r", 1)
222223
if err != nil {
223224
t.Errorf("Repositories.DownloadReleaseAsset returned error: %v", err)
224225
}
@@ -239,28 +240,16 @@ func TestRepositoriesService_DownloadReleaseAsset_Redirect(t *testing.T) {
239240
mux.HandleFunc("/repos/o/r/releases/assets/1", func(w http.ResponseWriter, r *http.Request) {
240241
testMethod(t, r, "GET")
241242
testHeader(t, r, "Accept", defaultMediaType)
242-
w.Header().Set("Location", server.URL+"/github-cloud/releases/1/hello-world.txt")
243-
w.WriteHeader(http.StatusFound)
243+
http.Redirect(w, r, "/yo", http.StatusFound)
244244
})
245245

246-
mux.HandleFunc("/github-cloud/releases/1/hello-world.txt", func(w http.ResponseWriter, r *http.Request) {
247-
testMethod(t, r, "GET")
248-
w.Header().Set("Content-Type", "application/octet-stream")
249-
w.Header().Set("Content-Disposition", "attachment; filename=hello-world.txt")
250-
fmt.Fprint(w, "Hello World")
251-
})
252-
253-
reader, err := client.Repositories.DownloadReleaseAsset("o", "r", 1)
246+
_, got, err := client.Repositories.DownloadReleaseAsset("o", "r", 1)
254247
if err != nil {
255248
t.Errorf("Repositories.DownloadReleaseAsset returned error: %v", err)
256249
}
257-
want := []byte("Hello World")
258-
content, err := ioutil.ReadAll(reader)
259-
if err != nil {
260-
t.Errorf("Repositories.DownloadReleaseAsset returned bad reader: %v", err)
261-
}
262-
if !bytes.Equal(want, content) {
263-
t.Errorf("Repositories.DownloadReleaseAsset returned %+v, want %+v", content, want)
250+
want := "/yo"
251+
if !strings.HasSuffix(got, want) {
252+
t.Errorf("Repositories.DownloadReleaseAsset returned %+v, want %+v", got, want)
264253
}
265254
}
266255

0 commit comments

Comments
 (0)