Set default Accept header for api command when one is not specified (#8303)

api/client_test.go

@@ -234,10 +234,9 @@ func TestHTTPHeaders(t *testing.T) {
 
 	ios, _, _, stderr := iostreams.Test()
 	httpClient, err := NewHTTPClient(HTTPClientOptions{
-		AppVersion:        "v1.2.3",
-		Config:            tinyConfig{ts.URL[7:] + ":oauth_token": "MYTOKEN"},
-		Log:               ios.ErrOut,
-		SkipAcceptHeaders: false,
+		AppVersion: "v1.2.3",
+		Config:     tinyConfig{ts.URL[7:] + ":oauth_token": "MYTOKEN"},
+		Log:        ios.ErrOut,
 	})
 	assert.NoError(t, err)
 	client := NewClientFromHTTP(httpClient)

api/http_client.go

@@ -17,14 +17,13 @@ type tokenGetter interface {
 }
 
 type HTTPClientOptions struct {
-	AppVersion        string
-	CacheTTL          time.Duration
-	Config            tokenGetter
-	EnableCache       bool
-	Log               io.Writer
-	LogColorize       bool
-	LogVerboseHTTP    bool
-	SkipAcceptHeaders bool
+	AppVersion     string
+	CacheTTL       time.Duration
+	Config         tokenGetter
+	EnableCache    bool
+	Log            io.Writer
+	LogColorize    bool
+	LogVerboseHTTP bool
 }
 
 func NewHTTPClient(opts HTTPClientOptions) (*http.Client, error) {
@@ -50,9 +49,6 @@ func NewHTTPClient(opts HTTPClientOptions) (*http.Client, error) {
 	headers := map[string]string{
 		userAgent: fmt.Sprintf("GitHub CLI %s", opts.AppVersion),
 	}
-	if opts.SkipAcceptHeaders {
-		headers[accept] = ""
-	}
 	clientOpts.Headers = headers
 
 	if opts.EnableCache {

api/http_client_test.go

@@ -20,7 +20,6 @@ func TestNewHTTPClient(t *testing.T) {
 	type args struct {
 		config         tokenGetter
 		appVersion     string
-		setAccept      bool
 		logVerboseHTTP bool
 	}
 	tests := []struct {
@@ -31,11 +30,10 @@ func TestNewHTTPClient(t *testing.T) {
 		wantStderr string
 	}{
 		{
-			name: "github.com with Accept header",
+			name: "github.com",
 			args: args{
 				config:         tinyConfig{"github.com:oauth_token": "MYTOKEN"},
 				appVersion:     "v1.2.3",
-				setAccept:      true,
 				logVerboseHTTP: false,
 			},
 			host: "github.com",
@@ -47,18 +45,16 @@ func TestNewHTTPClient(t *testing.T) {
 			wantStderr: "",
 		},
 		{
-			name: "github.com no Accept header",
+			name: "GHES",
 			args: args{
-				config:         tinyConfig{"github.com:oauth_token": "MYTOKEN"},
-				appVersion:     "v1.2.3",
-				setAccept:      false,
-				logVerboseHTTP: false,
+				config:     tinyConfig{"example.com:oauth_token": "GHETOKEN"},
+				appVersion: "v1.2.3",
 			},
-			host: "github.com",
+			host: "example.com",
 			wantHeader: map[string]string{
-				"authorization": "token MYTOKEN",
+				"authorization": "token GHETOKEN",
 				"user-agent":    "GitHub CLI v1.2.3",
-				"accept":        "",
+				"accept":        "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
 			},
 			wantStderr: "",
 		},
@@ -67,7 +63,6 @@ func TestNewHTTPClient(t *testing.T) {
 			args: args{
 				config:         tinyConfig{"example.com:oauth_token": "MYTOKEN"},
 				appVersion:     "v1.2.3",
-				setAccept:      true,
 				logVerboseHTTP: false,
 			},
 			host: "github.com",
@@ -78,12 +73,26 @@ func TestNewHTTPClient(t *testing.T) {
 			},
 			wantStderr: "",
 		},
+		{
+			name: "GHES no authentication token",
+			args: args{
+				config:         tinyConfig{"github.com:oauth_token": "MYTOKEN"},
+				appVersion:     "v1.2.3",
+				logVerboseHTTP: false,
+			},
+			host: "example.com",
+			wantHeader: map[string]string{
+				"authorization": "",
+				"user-agent":    "GitHub CLI v1.2.3",
+				"accept":        "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
+			},
+			wantStderr: "",
+		},
 		{
 			name: "github.com in verbose mode",
 			args: args{
 				config:         tinyConfig{"github.com:oauth_token": "MYTOKEN"},
 				appVersion:     "v1.2.3",
-				setAccept:      true,
 				logVerboseHTTP: true,
 			},
 			host: "github.com",
@@ -109,21 +118,6 @@ func TestNewHTTPClient(t *testing.T) {
 				* Request took <duration>
 			`),
 		},
-		{
-			name: "GHES Accept header",
-			args: args{
-				config:     tinyConfig{"example.com:oauth_token": "GHETOKEN"},
-				appVersion: "v1.2.3",
-				setAccept:  true,
-			},
-			host: "example.com",
-			wantHeader: map[string]string{
-				"authorization": "token GHETOKEN",
-				"user-agent":    "GitHub CLI v1.2.3",
-				"accept":        "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
-			},
-			wantStderr: "",
-		},
 	}
 
 	var gotReq *http.Request
@@ -137,11 +131,10 @@ func TestNewHTTPClient(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			ios, _, _, stderr := iostreams.Test()
 			client, err := NewHTTPClient(HTTPClientOptions{
-				AppVersion:        tt.args.appVersion,
-				Config:            tt.args.config,
-				Log:               ios.ErrOut,
-				SkipAcceptHeaders: !tt.args.setAccept,
-				LogVerboseHTTP:    tt.args.logVerboseHTTP,
+				AppVersion:     tt.args.appVersion,
+				Config:         tt.args.config,
+				Log:            ios.ErrOut,
+				LogVerboseHTTP: tt.args.logVerboseHTTP,
 			})
 			require.NoError(t, err)
 

pkg/cmd/api/api.go

@@ -298,14 +298,13 @@ func apiRun(opts *ApiOptions) error {
 				log = opts.IO.Out
 			}
 			opts := api.HTTPClientOptions{
-				AppVersion:        opts.AppVersion,
-				CacheTTL:          opts.CacheTTL,
-				Config:            cfg.Authentication(),
-				EnableCache:       opts.CacheTTL > 0,
-				Log:               log,
-				LogColorize:       opts.IO.ColorEnabled(),
-				LogVerboseHTTP:    opts.Verbose,
-				SkipAcceptHeaders: true,
+				AppVersion:     opts.AppVersion,
+				CacheTTL:       opts.CacheTTL,
+				Config:         cfg.Authentication(),
+				EnableCache:    opts.CacheTTL > 0,
+				Log:            log,
+				LogColorize:    opts.IO.ColorEnabled(),
+				LogVerboseHTTP: opts.Verbose,
 			}
 			return api.NewHTTPClient(opts)
 		}

pkg/cmd/api/api_test.go

@@ -1455,3 +1455,58 @@ func Test_parseErrorResponse(t *testing.T) {
 		})
 	}
 }
+
+func Test_apiRun_acceptHeader(t *testing.T) {
+	tests := []struct {
+		name             string
+		options          ApiOptions
+		wantAcceptHeader string
+	}{
+		{
+			name:             "sets default accept header",
+			options:          ApiOptions{},
+			wantAcceptHeader: "*/*",
+		},
+		{
+			name: "does not override user accept header",
+			options: ApiOptions{
+				RequestHeaders: []string{"Accept: testing"},
+			},
+			wantAcceptHeader: "testing",
+		},
+		{
+			name: "does not override preview names",
+			options: ApiOptions{
+				Previews: []string{"nebula"},
+			},
+			wantAcceptHeader: "application/vnd.github.nebula-preview+json",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ios, _, _, _ := iostreams.Test()
+			tt.options.IO = ios
+
+			tt.options.Config = func() (config.Config, error) {
+				return config.NewBlankConfig(), nil
+			}
+
+			var gotReq *http.Request
+			tt.options.HttpClient = func() (*http.Client, error) {
+				var tr roundTripper = func(req *http.Request) (*http.Response, error) {
+					gotReq = req
+					resp := &http.Response{
+						StatusCode: 200,
+						Request:    req,
+						Body:       io.NopCloser(bytes.NewBufferString("")),
+					}
+					return resp, nil
+				}
+				return &http.Client{Transport: tr}, nil
+			}
+
+			assert.NoError(t, apiRun(&tt.options))
+			assert.Equal(t, tt.wantAcceptHeader, gotReq.Header.Get("Accept"))
+		})
+	}
+}

pkg/cmd/api/http.go

@@ -74,6 +74,9 @@ func httpRequest(client *http.Client, hostname string, method string, p string,
 	if bodyIsJSON && req.Header.Get("Content-Type") == "" {
 		req.Header.Set("Content-Type", "application/json; charset=utf-8")
 	}
+	if req.Header.Get("Accept") == "" {
+		req.Header.Set("Accept", "*/*")
+	}
 
 	return client.Do(req)
 }

pkg/cmd/api/http_test.go

@@ -126,7 +126,25 @@ func Test_httpRequest(t *testing.T) {
 				method:  "GET",
 				u:       "https://api.github.com/repos/octocat/spoon-knife",
 				body:    "",
-				headers: "",
+				headers: "Accept: */*\r\n",
+			},
+		},
+		{
+			name: "GET with accept header",
+			args: args{
+				client:  &httpClient,
+				host:    "github.com",
+				method:  "GET",
+				p:       "repos/octocat/spoon-knife",
+				params:  nil,
+				headers: []string{"Accept: testing"},
+			},
+			wantErr: false,
+			want: expects{
+				method:  "GET",
+				u:       "https://api.github.com/repos/octocat/spoon-knife",
+				body:    "",
+				headers: "Accept: testing\r\n",
 			},
 		},
 		{
@@ -144,7 +162,7 @@ func Test_httpRequest(t *testing.T) {
 				method:  "GET",
 				u:       "https://api.github.com/repos/octocat/spoon-knife",
 				body:    "",
-				headers: "",
+				headers: "Accept: */*\r\n",
 			},
 		},
 		{
@@ -162,7 +180,7 @@ func Test_httpRequest(t *testing.T) {
 				method:  "GET",
 				u:       "https://api.github.com/repos/octocat/spoon-knife",
 				body:    "",
-				headers: "",
+				headers: "Accept: */*\r\n",
 			},
 		},
 		{
@@ -180,7 +198,7 @@ func Test_httpRequest(t *testing.T) {
 				method:  "GET",
 				u:       "https://example.org/api/v3/repos/octocat/spoon-knife",
 				body:    "",
-				headers: "",
+				headers: "Accept: */*\r\n",
 			},
 		},
 		{
@@ -200,7 +218,7 @@ func Test_httpRequest(t *testing.T) {
 				method:  "GET",
 				u:       "https://api.github.com/repos/octocat/spoon-knife?a=b",
 				body:    "",
-				headers: "",
+				headers: "Accept: */*\r\n",
 			},
 		},
 		{
@@ -220,7 +238,7 @@ func Test_httpRequest(t *testing.T) {
 				method:  "POST",
 				u:       "https://api.github.com/repos",
 				body:    `{"a":"b"}`,
-				headers: "Content-Type: application/json; charset=utf-8\r\n",
+				headers: "Accept: */*\r\nContent-Type: application/json; charset=utf-8\r\n",
 			},
 		},
 		{
@@ -240,7 +258,7 @@ func Test_httpRequest(t *testing.T) {
 				method:  "POST",
 				u:       "https://api.github.com/graphql",
 				body:    `{"variables":{"a":"b"}}`,
-				headers: "Content-Type: application/json; charset=utf-8\r\n",
+				headers: "Accept: */*\r\nContent-Type: application/json; charset=utf-8\r\n",
 			},
 		},
 		{
@@ -258,7 +276,7 @@ func Test_httpRequest(t *testing.T) {
 				method:  "POST",
 				u:       "https://example.org/api/graphql",
 				body:    `{}`,
-				headers: "Content-Type: application/json; charset=utf-8\r\n",
+				headers: "Accept: */*\r\nContent-Type: application/json; charset=utf-8\r\n",
 			},
 		},
 		{