[pull] trunk from cli:trunk

api/http_client.go

@@ -17,22 +17,24 @@ type tokenGetter interface {
 }
 
 type HTTPClientOptions struct {
-	AppVersion     string
-	CacheTTL       time.Duration
-	Config         tokenGetter
-	EnableCache    bool
-	Log            io.Writer
-	LogColorize    bool
-	LogVerboseHTTP bool
+	AppVersion         string
+	CacheTTL           time.Duration
+	Config             tokenGetter
+	EnableCache        bool
+	Log                io.Writer
+	LogColorize        bool
+	LogVerboseHTTP     bool
+	SkipDefaultHeaders bool
 }
 
 func NewHTTPClient(opts HTTPClientOptions) (*http.Client, error) {
 	// Provide invalid host, and token values so gh.HTTPClient will not automatically resolve them.
 	// The real host and token are inserted at request time.
 	clientOpts := ghAPI.ClientOptions{
-		Host:         "none",
-		AuthToken:    "none",
-		LogIgnoreEnv: true,
+		Host:               "none",
+		AuthToken:          "none",
+		LogIgnoreEnv:       true,
+		SkipDefaultHeaders: opts.SkipDefaultHeaders,
 	}
 
 	debugEnabled, debugValue := utils.IsDebugEnabled()

api/http_client_test.go

@@ -18,15 +18,16 @@ import (
 
 func TestNewHTTPClient(t *testing.T) {
 	type args struct {
-		config         tokenGetter
-		appVersion     string
-		logVerboseHTTP bool
+		config             tokenGetter
+		appVersion         string
+		logVerboseHTTP     bool
+		skipDefaultHeaders bool
 	}
 	tests := []struct {
 		name       string
 		args       args
 		host       string
-		wantHeader map[string]string
+		wantHeader map[string][]string
 		wantStderr string
 	}{
 		{
@@ -37,10 +38,10 @@ func TestNewHTTPClient(t *testing.T) {
 				logVerboseHTTP: false,
 			},
 			host: "github.com",
-			wantHeader: map[string]string{
-				"authorization": "token MYTOKEN",
-				"user-agent":    "GitHub CLI v1.2.3",
-				"accept":        "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
+			wantHeader: map[string][]string{
+				"authorization": {"token MYTOKEN"},
+				"user-agent":    {"GitHub CLI v1.2.3"},
+				"accept":        {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
 			},
 			wantStderr: "",
 		},
@@ -51,10 +52,10 @@ func TestNewHTTPClient(t *testing.T) {
 				appVersion: "v1.2.3",
 			},
 			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",
+			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: "",
 		},
@@ -66,10 +67,10 @@ func TestNewHTTPClient(t *testing.T) {
 				logVerboseHTTP: false,
 			},
 			host: "github.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",
+			wantHeader: map[string][]string{
+				"authorization": nil, // should not be set
+				"user-agent":    {"GitHub CLI v1.2.3"},
+				"accept":        {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
 			},
 			wantStderr: "",
 		},
@@ -81,10 +82,10 @@ func TestNewHTTPClient(t *testing.T) {
 				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",
+			wantHeader: map[string][]string{
+				"authorization": nil, // should not be set
+				"user-agent":    {"GitHub CLI v1.2.3"},
+				"accept":        {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
 			},
 			wantStderr: "",
 		},
@@ -96,10 +97,10 @@ func TestNewHTTPClient(t *testing.T) {
 				logVerboseHTTP: true,
 			},
 			host: "github.com",
-			wantHeader: map[string]string{
-				"authorization": "token MYTOKEN",
-				"user-agent":    "GitHub CLI v1.2.3",
-				"accept":        "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview",
+			wantHeader: map[string][]string{
+				"authorization": {"token MYTOKEN"},
+				"user-agent":    {"GitHub CLI v1.2.3"},
+				"accept":        {"application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview"},
 			},
 			wantStderr: heredoc.Doc(`
 				* Request at <time>
@@ -115,6 +116,34 @@ func TestNewHTTPClient(t *testing.T) {
 				< HTTP/1.1 204 No Content
 				< Date: <time>
 
+				* Request took <duration>
+			`),
+		},
+		{
+			name: "respect skip default headers option",
+			args: args{
+				appVersion:         "v1.2.3",
+				logVerboseHTTP:     true,
+				skipDefaultHeaders: true,
+			},
+			host: "github.com",
+			wantHeader: map[string][]string{
+				"accept":        nil,
+				"authorization": nil,
+				"content-type":  nil,
+				"user-agent":    {"GitHub CLI v1.2.3"},
+			},
+			wantStderr: heredoc.Doc(`
+				* Request at <time>
+				* Request to http://<host>:<port>
+				> GET / HTTP/1.1
+				> Host: github.com
+				> Time-Zone: <timezone>
+				> User-Agent: GitHub CLI v1.2.3
+
+				< HTTP/1.1 204 No Content
+				< Date: <time>
+
 				* Request took <duration>
 			`),
 		},
@@ -131,10 +160,11 @@ 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,
-				LogVerboseHTTP: tt.args.logVerboseHTTP,
+				AppVersion:         tt.args.appVersion,
+				Config:             tt.args.config,
+				Log:                ios.ErrOut,
+				LogVerboseHTTP:     tt.args.logVerboseHTTP,
+				SkipDefaultHeaders: tt.args.skipDefaultHeaders,
 			})
 			require.NoError(t, err)
 
@@ -148,7 +178,7 @@ func TestNewHTTPClient(t *testing.T) {
 			require.NoError(t, err)
 
 			for name, value := range tt.wantHeader {
-				assert.Equal(t, value, gotReq.Header.Get(name), name)
+				assert.Equal(t, value, gotReq.Header.Values(name), name)
 			}
 
 			assert.Equal(t, 204, res.StatusCode)

go.mod

@@ -32,7 +32,6 @@ require (
 	github.com/gorilla/websocket v1.5.3
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hashicorp/go-version v1.7.0
-	github.com/henvic/httpretty v0.1.4
 	github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec
 	github.com/in-toto/attestation v1.1.2
 	github.com/joho/godotenv v1.5.1
@@ -148,6 +147,7 @@ require (
 	github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect
+	github.com/henvic/httpretty v0.1.4 // indirect
 	github.com/huandu/xstrings v1.5.0 // indirect
 	github.com/in-toto/in-toto-golang v0.9.0 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect

internal/authflow/flow.go

@@ -6,17 +6,13 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"regexp"
-	"strings"
 
 	"github.com/atotto/clipboard"
 	"github.com/cli/cli/v2/api"
 	"github.com/cli/cli/v2/internal/browser"
 	"github.com/cli/cli/v2/internal/ghinstance"
 	"github.com/cli/cli/v2/pkg/iostreams"
-	"github.com/cli/cli/v2/utils"
 	"github.com/cli/oauth"
-	"github.com/henvic/httpretty"
 
 	ghauth "github.com/cli/go-gh/v2/pkg/auth"
 )
@@ -26,21 +22,15 @@ var (
 	oauthClientID = "178c6fc778ccc68e1d6a"
 	// This value is safe to be embedded in version control
 	oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b"
-
-	jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`)
 )
 
-func AuthFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool, b browser.Browser, isCopyToClipboard bool) (string, string, error) {
+// AuthFlow initiates an OAuth device or web application flow to acquire a
+// token. The provided HTTP client should be a plain client that does not set
+// auth or other headers.
+func AuthFlow(httpClient *http.Client, oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool, b browser.Browser, isCopyToClipboard bool) (string, string, error) {
 	w := IO.ErrOut
 	cs := IO.ColorScheme()
 
-	httpClient := &http.Client{}
-	debugEnabled, debugValue := utils.IsDebugEnabled()
-	if debugEnabled {
-		logTraffic := strings.Contains(debugValue, "api")
-		httpClient.Transport = verboseLog(IO.ErrOut, logTraffic, IO.ColorEnabled())(httpClient.Transport)
-	}
-
 	minimumScopes := []string{"repo", "read:org", "gist"}
 	scopes := append(minimumScopes, additionalScopes...)
 
@@ -150,28 +140,3 @@ func waitForEnter(r io.Reader) error {
 	scanner.Scan()
 	return scanner.Err()
 }
-
-func verboseLog(out io.Writer, logTraffic bool, colorize bool) func(http.RoundTripper) http.RoundTripper {
-	logger := &httpretty.Logger{
-		Time:            true,
-		TLS:             false,
-		Colors:          colorize,
-		RequestHeader:   logTraffic,
-		RequestBody:     logTraffic,
-		ResponseHeader:  logTraffic,
-		ResponseBody:    logTraffic,
-		Formatters:      []httpretty.Formatter{&httpretty.JSONFormatter{}},
-		MaxResponseBody: 10000,
-	}
-	logger.SetOutput(out)
-	logger.SetBodyFilter(func(h http.Header) (skip bool, err error) {
-		return !inspectableMIMEType(h.Get("Content-Type")), nil
-	})
-	return logger.RoundTripper
-}
-
-func inspectableMIMEType(t string) bool {
-	return strings.HasPrefix(t, "text/") ||
-		strings.HasPrefix(t, "application/x-www-form-urlencoded") ||
-		jsonTypeRE.MatchString(t)
-}

pkg/cmd/auth/login/login.go

@@ -20,12 +20,13 @@ import (
 )
 
 type LoginOptions struct {
-	IO         *iostreams.IOStreams
-	Config     func() (gh.Config, error)
-	HttpClient func() (*http.Client, error)
-	GitClient  *git.Client
-	Prompter   shared.Prompt
-	Browser    browser.Browser
+	IO              *iostreams.IOStreams
+	Config          func() (gh.Config, error)
+	HttpClient      func() (*http.Client, error)
+	PlainHttpClient func() (*http.Client, error)
+	GitClient       *git.Client
+	Prompter        shared.Prompt
+	Browser         browser.Browser
 
 	MainExecutable string
 
@@ -43,12 +44,13 @@ type LoginOptions struct {
 
 func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command {
 	opts := &LoginOptions{
-		IO:         f.IOStreams,
-		Config:     f.Config,
-		HttpClient: f.HttpClient,
-		GitClient:  f.GitClient,
-		Prompter:   f.Prompter,
-		Browser:    f.Browser,
+		IO:              f.IOStreams,
+		Config:          f.Config,
+		HttpClient:      f.HttpClient,
+		PlainHttpClient: f.PlainHttpClient,
+		GitClient:       f.GitClient,
+		Prompter:        f.Prompter,
+		Browser:         f.Browser,
 	}
 
 	var tokenStdin bool
@@ -190,6 +192,11 @@ func loginRun(opts *LoginOptions) error {
 		return cmdutil.SilentError
 	}
 
+	plainHTTPClient, err := opts.PlainHttpClient()
+	if err != nil {
+		return err
+	}
+
 	httpClient, err := opts.HttpClient()
 	if err != nil {
 		return err
@@ -210,16 +217,17 @@ func loginRun(opts *LoginOptions) error {
 	}
 
 	return shared.Login(&shared.LoginOptions{
-		IO:          opts.IO,
-		Config:      authCfg,
-		HTTPClient:  httpClient,
-		Hostname:    hostname,
-		Interactive: opts.Interactive,
-		Web:         opts.Web,
-		Scopes:      opts.Scopes,
-		GitProtocol: opts.GitProtocol,
-		Prompter:    opts.Prompter,
-		Browser:     opts.Browser,
+		IO:              opts.IO,
+		Config:          authCfg,
+		HTTPClient:      httpClient,
+		PlainHTTPClient: plainHTTPClient,
+		Hostname:        hostname,
+		Interactive:     opts.Interactive,
+		Web:             opts.Web,
+		Scopes:          opts.Scopes,
+		GitProtocol:     opts.GitProtocol,
+		Prompter:        opts.Prompter,
+		Browser:         opts.Browser,
 		CredentialFlow: &shared.GitCredentialFlow{
 			Prompter: opts.Prompter,
 			HelperConfig: &gitcredentials.HelperConfig{

pkg/cmd/auth/login/login_test.go

@@ -483,6 +483,9 @@ func Test_loginRun_nontty(t *testing.T) {
 			tt.opts.HttpClient = func() (*http.Client, error) {
 				return &http.Client{Transport: reg}, nil
 			}
+			tt.opts.PlainHttpClient = func() (*http.Client, error) {
+				return &http.Client{Transport: reg}, nil
+			}
 			if tt.httpStubs != nil {
 				tt.httpStubs(reg)
 			}
@@ -775,6 +778,9 @@ func Test_loginRun_Survey(t *testing.T) {
 			tt.opts.HttpClient = func() (*http.Client, error) {
 				return &http.Client{Transport: reg}, nil
 			}
+			tt.opts.PlainHttpClient = func() (*http.Client, error) {
+				return &http.Client{Transport: reg}, nil
+			}
 			if tt.httpStubs != nil {
 				tt.httpStubs(reg)
 			} else {