fix: DownloadReleaseAsset handles renamed repository by mdb · Pull Request #3392 · google/go-github

mdb · 2024-12-17T10:57:47Z

This fixes issue #3081, such that Repositories.DownloadReleaseAsset correctly handles scenarios where a non-nil followRedirectsClient argument has been passed, and the specified repository has been renamed to a new name.

Previously, in such scenarios...

The initial GET /repos/<OWNER>/<REPO>/releases/assets/<ASSET ID> responds 301 w/ a Location: <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>.
The followRedirectsClient issues a GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID> with a Accept: */* header.
GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID> responds 200 with a JSON response body (rather than 302 w/ a Location: <MEDIA SERVER ASSET URL> header) due to the presence of the Accept: */* header (and the absence of a Accept: application/octet-stream header) from step 2.

The following curl offers an example of correct redirect follows & correct Accept: application/octet-stream-preservation in this scenario:

curl \
  --verbose \
  --location \
  --header "Accept: application/octet-stream" \
  https://api.github.com/repos/mterwill/go-github-issue-demo/releases/assets/151970555

Now, as per this change:

The initial GET /repos/<OWNER>/<REPO>/releases/assets/<ASSET ID> responds 301 w/ a Location: <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>.
The followRedirectsClient issues a GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID> with a Accept: application/octet-stream header as required by the GH API.
GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID> responds 302 w/ a Location: <MEDIA SERVER ASSET URL>.
The followRedirectsClient follows the redirects to GET <MEDIA SERVER ASSET URL> w/ a Accept: application/octet-stream header, and GET <MEDIA SERVER ASSET URL> responds 200 w/ the asset contents.

This fixes issue google#3081, such that `Repositories.DownloadReleaseAsset` correctly handles scenarios where a non-`nil` `followRedirectsClient` argument has been passed, but the specified repository has been renamed to a new name. Previously, in such scenarios... 1. The initial `GET /repos/<OWNER>/<REPO>/releases/assets/<ASSET ID>` responds 301 w/ a `Location: <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>`. 2. The `followRedirectsClient` issues a `GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>` with a `Accept: */*` header. 3. `GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>` responds 200 with a JSON response body (rather than 302 w/ a `Location: <MEDIA SERVER ASSET URL>` header) due to the presence of the `Accept: */*` header (and the absence of a `Accept: application/octet-stream` header)`from step 2. The following `curl` offers an example of correct redirect follows & correct `Accept: application/octet-stream`-preservation in this scenario: ``` curl \ --verbose \ --location \ --header "Accept: application/octet-stream" \ https://api.github.com/repos/mterwill/go-github-issue-demo/releases/assets/151970555 ``` Now, as per this change: 1. The initial `GET /repos/<OWNER>/<REPO>/releases/assets/<ASSET ID>` responds 301 w/ a `Location: <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>`. 2. The `followRedirectsClient` issues a `GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>` with a `Accept: application/octet-stream` header [as required by the GH API](https://docs.github.com/en/enterprise-cloud@latest/rest/releases/assets?apiVersion=2022-11-28) 3. `GET <GH API HOST>/repositories/<REPO ID>/releases/assets/<ASSET ID>` responds 302 w/ a `Location: <MEDIA SERVER ASSET URL>`. 4. The `followRedirectsClient` follows the redirects to `GET <MEDIA SERVER ASSET URL>` w/ a `Accept: application/octet-stream` header, and `GET <MEDIA SERVER ASSET URL>` responds 200 w/ the asset contents. Signed-off-by: Mike Ball <mikedball@gmail.com>

google-cla · 2024-12-17T10:57:53Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

…ed-repo

codecov · 2024-12-17T11:15:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.21%. Comparing base (2b8c7fa) to head (94578ef).
Report is 203 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3392      +/-   ##
==========================================
- Coverage   97.72%   92.21%   -5.51%     
==========================================
  Files         153      173      +20     
  Lines       13390    14770    +1380     
==========================================
+ Hits        13085    13620     +535     
- Misses        215     1060     +845     
  Partials       90       90

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gmlewis

Thank you, @mdb !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

This improves `RepositoriesService.DownloadReleaseAsset` documentation to be a bit more specific and accurate, as per: google#3081 (comment) Signed-off-by: Mike Ball <mikedball@gmail.com>

gmlewis

LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

tomfeigin

LGTM

gmlewis · 2024-12-22T15:14:48Z

Thank you, @tomfeigin !
Merging.

@deiga