bradleyfalzon / ghinstallation

HTTP Round Tripper for GitHub Apps - Authenticate as an Installation Workflow
Apache License 2.0
317 stars 98 forks source link

Accept Header Issue #12

Open cpheps opened 6 years ago

cpheps commented 6 years ago

I've run into an issue when using the DownloadReleaseAsset function in the github.com/google/go-github library. It appears the GitHub API will not correctly download a release asset binary when two Accept headers are present.

Transport.RoundTrip adds it's own Accept header which causes the download to fail as the API is looking for a single header with the value application/octet-stream.

Here is the link the the GitHub API documentation https://developer.github.com/v3/repos/releases/#get-a-single-release-asset

lyonlai commented 3 years ago

@cpheps yo. I saw you had the PR and it's closed eventually. how did you solve it at the end? using your own folk?

cpheps commented 3 years ago

@lyonlai That's was a few years and jobs. We didn't fork it but I seem to remember somehow rebuilding the headers from scratch and deleting the extra accept header.

lyonlai commented 3 years ago

sigh. this should be merged. we ended up feeding a custom Transport to overwritre the header just for the download URL, which fits our use case but still not ideal as could be fixed in this library. Thanks for answering, it's just for curiosity. @cpheps

elgohr commented 2 years ago

Using something like

type downloadingRoundTripper struct {
    roundTripper http.RoundTripper
}

func NewDownloadingRoundTripper() *downloadingRoundTripper {
    return &downloadingRoundTripper{roundTripper: http.DefaultTransport}
}

func (r *downloadingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
    if req.Method == http.MethodGet && strings.Contains(req.URL.Path, "/releases/assets/") {
        req.Header.Set("Accept", "application/octet-stream")
    }
    return r.roundTripper.RoundTrip(req)
}

as the default roundtripper could be a workaround/ugly fix.