From 8a221bb766e5841dd733167c1395e3ff6accfed8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav@github.com>
Date: Tue, 8 Jun 2021 19:21:48 +0200
Subject: [PATCH] Add tests for our default HTTP client

---
 pkg/cmd/factory/http.go      |  18 +++-
 pkg/cmd/factory/http_test.go | 158 +++++++++++++++++++++++++++++++++++
 2 files changed, 172 insertions(+), 4 deletions(-)
 create mode 100644 pkg/cmd/factory/http_test.go

diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go
index 2e7619540..0005c6022 100644
--- a/pkg/cmd/factory/http.go
+++ b/pkg/cmd/factory/http.go
@@ -8,7 +8,6 @@ import (
 	"time"
 
 	"github.com/cli/cli/api"
-	"github.com/cli/cli/internal/config"
 	"github.com/cli/cli/internal/ghinstance"
 	"github.com/cli/cli/pkg/iostreams"
 )
@@ -53,8 +52,12 @@ var timezoneNames = map[int]string{
 	50400:  "Pacific/Kiritimati",
 }
 
+type configGetter interface {
+	Get(string, string) (string, error)
+}
+
 // generic authenticated HTTP client for commands
-func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, setAccept bool) *http.Client {
+func NewHTTPClient(io *iostreams.IOStreams, cfg configGetter, appVersion string, setAccept bool) *http.Client {
 	var opts []api.ClientOption
 	if verbose := os.Getenv("DEBUG"); verbose != "" {
 		logTraffic := strings.Contains(verbose, "api")
@@ -64,7 +67,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string
 	opts = append(opts,
 		api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", appVersion)),
 		api.AddHeaderFunc("Authorization", func(req *http.Request) (string, error) {
-			hostname := ghinstance.NormalizeHostname(req.URL.Hostname())
+			hostname := ghinstance.NormalizeHostname(getHost(req))
 			if token, err := cfg.Get(hostname, "oauth_token"); err == nil && token != "" {
 				return fmt.Sprintf("token %s", token), nil
 			}
@@ -89,7 +92,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string
 				accept := "application/vnd.github.antiope-preview+json"
 				// introduced for #2952: pr branch up to date status
 				accept += ", application/vnd.github.merge-info-preview+json"
-				if ghinstance.IsEnterprise(req.URL.Hostname()) {
+				if ghinstance.IsEnterprise(getHost(req)) {
 					// shadow-cat-preview: Draft pull requests
 					accept += ", application/vnd.github.shadow-cat-preview"
 				}
@@ -100,3 +103,10 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string
 
 	return api.NewHTTPClient(opts...)
 }
+
+func getHost(r *http.Request) string {
+	if r.Host != "" {
+		return r.Host
+	}
+	return r.URL.Hostname()
+}
diff --git a/pkg/cmd/factory/http_test.go b/pkg/cmd/factory/http_test.go
new file mode 100644
index 000000000..f52a8704f
--- /dev/null
+++ b/pkg/cmd/factory/http_test.go
@@ -0,0 +1,158 @@
+package factory
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"regexp"
+	"testing"
+
+	"github.com/MakeNowJust/heredoc"
+	"github.com/cli/cli/pkg/iostreams"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewHTTPClient(t *testing.T) {
+	type args struct {
+		config     configGetter
+		appVersion string
+		setAccept  bool
+	}
+	tests := []struct {
+		name       string
+		args       args
+		envDebug   string
+		host       string
+		wantHeader map[string]string
+		wantStderr string
+	}{
+		{
+			name: "github.com with Accept header",
+			args: args{
+				config:     tinyConfig{"github.com:oauth_token": "MYTOKEN"},
+				appVersion: "v1.2.3",
+				setAccept:  true,
+			},
+			host: "github.com",
+			wantHeader: map[string]string{
+				"authorization": "token MYTOKEN",
+				"user-agent":    "GitHub CLI v1.2.3",
+				"accept":        "application/vnd.github.antiope-preview+json, application/vnd.github.merge-info-preview+json",
+			},
+			wantStderr: "",
+		},
+		{
+			name: "github.com no Accept header",
+			args: args{
+				config:     tinyConfig{"example.com:oauth_token": "MYTOKEN"},
+				appVersion: "v1.2.3",
+				setAccept:  false,
+			},
+			host: "github.com",
+			wantHeader: map[string]string{
+				"authorization": "",
+				"user-agent":    "GitHub CLI v1.2.3",
+				"accept":        "",
+			},
+			wantStderr: "",
+		},
+		{
+			name: "github.com in verbose mode",
+			args: args{
+				config:     tinyConfig{"github.com:oauth_token": "MYTOKEN"},
+				appVersion: "v1.2.3",
+				setAccept:  false,
+			},
+			host:     "github.com",
+			envDebug: "api",
+			wantHeader: map[string]string{
+				"authorization": "token MYTOKEN",
+				"user-agent":    "GitHub CLI v1.2.3",
+				"accept":        "",
+			},
+			wantStderr: heredoc.Doc(`
+				* Request at <time>
+				* Request to http://<host>:<port>
+				> GET / HTTP/1.1
+				> Host: github.com
+				> Authorization: token ████████████████████
+				> User-Agent: GitHub CLI v1.2.3
+				
+				< HTTP/1.1 204 No Content
+				< Date: <time>
+				
+				* 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.antiope-preview+json, application/vnd.github.merge-info-preview+json, application/vnd.github.shadow-cat-preview",
+			},
+			wantStderr: "",
+		},
+	}
+
+	var gotReq *http.Request
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotReq = r
+		w.WriteHeader(http.StatusNoContent)
+	}))
+	defer ts.Close()
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			oldDebug := os.Getenv("DEBUG")
+			os.Setenv("DEBUG", tt.envDebug)
+			t.Cleanup(func() {
+				os.Setenv("DEBUG", oldDebug)
+			})
+
+			io, _, _, stderr := iostreams.Test()
+			client := NewHTTPClient(io, tt.args.config, tt.args.appVersion, tt.args.setAccept)
+
+			req, err := http.NewRequest("GET", ts.URL, nil)
+			req.Host = tt.host
+			require.NoError(t, err)
+
+			res, err := client.Do(req)
+			require.NoError(t, err)
+
+			for name, value := range tt.wantHeader {
+				assert.Equal(t, value, gotReq.Header.Get(name), name)
+			}
+
+			assert.Equal(t, 204, res.StatusCode)
+			assert.Equal(t, tt.wantStderr, normalizeVerboseLog(stderr.String()))
+		})
+	}
+}
+
+type tinyConfig map[string]string
+
+func (c tinyConfig) Get(host, key string) (string, error) {
+	return c[fmt.Sprintf("%s:%s", host, key)], nil
+}
+
+var requestAtRE = regexp.MustCompile(`(?m)^\* Request at .+`)
+var dateRE = regexp.MustCompile(`(?m)^< Date: .+`)
+var hostWithPortRE = regexp.MustCompile(`127\.0\.0\.1:\d+`)
+var durationRE = regexp.MustCompile(`(?m)^\* Request took .+`)
+
+func normalizeVerboseLog(t string) string {
+	t = requestAtRE.ReplaceAllString(t, "* Request at <time>")
+	t = hostWithPortRE.ReplaceAllString(t, "<host>:<port>")
+	t = dateRE.ReplaceAllString(t, "< Date: <time>")
+	t = durationRE.ReplaceAllString(t, "* Request took <duration>")
+	return t
+}