Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Robust Header Check #13

Closed
wants to merge 12 commits into from
2 changes: 1 addition & 1 deletion appsTransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (t *AppsTransport) RoundTrip(req *http.Request) (*http.Response, error) {
}

req.Header.Set("Authorization", "Bearer "+ss)
req.Header.Set("Accept", acceptHeader)
addAcceptHeader(req.Header)

resp, err := t.tr.RoundTrip(req)
return resp, err
Expand Down
22 changes: 21 additions & 1 deletion transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"io/ioutil"
"net/http"
"strings"
"sync"
"time"
)
Expand Down Expand Up @@ -90,7 +91,7 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
}

req.Header.Set("Authorization", "token "+token)
req.Header.Add("Accept", acceptHeader) // We add to "Accept" header to avoid overwriting existing req headers.
addAcceptHeader(req.Header)
resp, err := t.tr.RoundTrip(req)
return resp, err
}
Expand Down Expand Up @@ -134,3 +135,22 @@ func (t *Transport) refreshToken() error {

return nil
}

// addAcceptHeader adds acceptHeader value to "Accept" header, but only
// if the current "Accept" header is not set, or if it already accepts JSON.
func addAcceptHeader(headers http.Header) {
if headers.Get("Accept") == "" {
headers.Set("Accept", acceptHeader)
return
}

// Need to loop through all Accept headers in case there is more than one.
for _, header := range headers["Accept"] {
// Looks as though all media types (https://developer.github.com/v3/media/) that can accept json end with "json".
// Only doing a suffix check to see if a json header already exists.
if strings.HasSuffix(header, "json") {
headers.Add("Accept", acceptHeader) // We add to "Accept" header to avoid overwriting existing req headers.
return
}
}
}
76 changes: 76 additions & 0 deletions transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"time"
)
Expand Down Expand Up @@ -173,3 +174,78 @@ func TestNew_appendHeader(t *testing.T) {
t.Errorf("could not find %v in request's accept headers: %v", myheader, headers["Accept"])
}
}

func TestSingleAssetBinary_AddAcceptHeader(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, "https://api.github.com/repos/bradleyfalzon/ghinstallation/releases/assets/1", http.NoBody)
if err != nil {
t.Fatalf("Failed to create test request: %s", err.Error())
}

req.Header.Set("Accept", "application/octet-stream")

addAcceptHeader(req.Header)

for headerName, headers := range req.Header {
if strings.ToLower(headerName) == "accept" {
for _, header := range headers {
if header == acceptHeader {
fmt.Printf("Header Name: %s, Header: %s", headerName, header)
t.Error("Set Accept header improperly for single asset call")
}
}
}
}
}

func TestNormal_AddAcceptHeader(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, "https://api.github.com/repos", http.NoBody)
if err != nil {
t.Fatalf("Failed to create test request: %s", err.Error())
}

addAcceptHeader(req.Header)

if req.Header.Get("Accept") != acceptHeader {
t.Error("Did not correctly set 'Accept' header")
}
}

func TestAlreadyExists_AddAcceptHeader(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, "https://api.github.com/repos", http.NoBody)
if err != nil {
t.Fatalf("Failed to create test request: %s", err.Error())
}

req.Header.Set("accept", "application/vnd.github.v3+json")

addAcceptHeader(req.Header)

found := false
for headerName, headers := range req.Header {
if strings.ToLower(headerName) == "accept" {
for _, header := range headers {
if header == acceptHeader {
found = true
break
}
}
}
}

if !found {
t.Errorf("Did not correctly append %s header", acceptHeader)
}
}

func TestSingleAssetJSON_AddAcceptHeader(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, "https://api.github.com/repos/bradleyfalzon/ghinstallation/releases/assets/1", http.NoBody)
if err != nil {
t.Fatalf("Failed to create test request: %s", err.Error())
}

addAcceptHeader(req.Header)

if req.Header.Get("Accept") != acceptHeader {
t.Error("Did not correctly set 'Accept' header")
}
}