DownloadReleaseAsset breaks with renamed repository

[mterwill]

DownloadReleaseAsset breaks when a repository is renamed. You can see a reproduction of this here on a repository that I renamed from go-github-issue-demo to go-github-issue-demo-1.

    main_test.go:57: data differs (-got +want):
          string(
        -       `{"url":"https://api.github.com/repos/mterwill/go-github-issue-demo-1/releases/assets/151970555","id":151970555,"node_id":"RA_kwDOLThgfM4JDuL7","name":"foo.txt","label":null,"uploader":{"login":"mterwill","id":5882053,"node_id":"MDQ6VXNlcjU4ODIwNTM=","avata`...,
        +       "Hello, world!\n",
          )

The code expects to receive exactly 1 redirect, which is to download the asset from the media server. However, if the repository is renamed, GitHub redirects once more to the new repository name. On following the redirect, downloadReleaseAssetFromURL code below sets a different accept header which causes the API server to respond with the release metadata rather than contents (docs):

To download the asset's binary content, set the Accept header of the request to application/octet-stream. The API will either redirect the client to the location, or stream it directly if possible. API clients should handle both a 200 or 302 response.

https://github.com/google/go-github/blob/454c1ddaeb85aa3caba66b7cd21ca24b6a2e311d/github/repos_releases.go#L390

The test I linked above is a minimal reproduction but it's worth noting that we actually discovered this in a different way: attempting to download a release for a renamed private repository actually returns a 401 Unauthorized, as when the redirect is followed the client also omits authentication (for actual release asset downloads, GitHub puts a token in the query params). We were following the function documentation and passing http.DefaultClient, rather than our authenticating HTTP client that was used to originally construct the GHE API client.

[gmlewis]

Could this be related to #3043 ?

[dnephin]

I don't think so, no. The two problems, as I understand them, are:

One:

https://github.com/google/go-github/blob/454c1ddaeb85aa3caba66b7cd21ca24b6a2e311d/github/repos_releases.go#L390

because this API requires application/octet-stream to download the asset. This line should use that as the Accept header, not */*.

Two:

https://github.com/google/go-github/blob/454c1ddaeb85aa3caba66b7cd21ca24b6a2e311d/github/repos_releases.go#L370

uses followRedirectsClient instead of s.client.client. The documentation could be more clear about when it's not appropriate to use http.DefaultClient (when you're using private repositories).

#3051 (the PR that closed #3043) could be a problem if DownloadReleaseAsset used s.client.client to follow redirects.

[gmlewis]

@dnephin - thanks for the explanation. Since you seem to have a better grasp of this than myself, would you like to put together a PR to solve these issues?