Skip to content

Commit af90f72

Browse files
mislavmislav
authored
Merge pull request cli#3803 from cli/http-accept-header
Update "Accept" header for github.com requests
2 parents 7dbaaf2 + 4debbb1 commit af90f72

File tree

2 files changed

+191
-10
lines changed

2 files changed

+191
-10
lines changed

‎pkg/cmd/factory/http.go‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"time"
99

1010
"github.com/cli/cli/api"
11-
"github.com/cli/cli/internal/config"
1211
"github.com/cli/cli/internal/ghinstance"
1312
"github.com/cli/cli/pkg/iostreams"
1413
)
@@ -53,8 +52,12 @@ var timezoneNames = map[int]string{
5352
50400: "Pacific/Kiritimati",
5453
}
5554

55+
type configGetter interface {
56+
Get(string, string) (string, error)
57+
}
58+
5659
// generic authenticated HTTP client for commands
57-
func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, setAccept bool) *http.Client {
60+
func NewHTTPClient(io *iostreams.IOStreams, cfg configGetter, appVersion string, setAccept bool) *http.Client {
5861
var opts []api.ClientOption
5962
if verbose := os.Getenv("DEBUG"); verbose != "" {
6063
logTraffic := strings.Contains(verbose, "api")
@@ -64,7 +67,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string
6467
opts = append(opts,
6568
api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", appVersion)),
6669
api.AddHeaderFunc("Authorization", func(req *http.Request) (string, error) {
67-
hostname := ghinstance.NormalizeHostname(req.URL.Hostname())
70+
hostname := ghinstance.NormalizeHostname(getHost(req))
6871
if token, err := cfg.Get(hostname, "oauth_token"); err == nil && token != "" {
6972
return fmt.Sprintf("token %s", token), nil
7073
}
@@ -85,13 +88,10 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string
8588
if setAccept {
8689
opts = append(opts,
8790
api.AddHeaderFunc("Accept", func(req *http.Request) (string, error) {
88-
// antiope-preview: Checks
89-
accept := "application/vnd.github.antiope-preview+json"
90-
// introduced for #2952: pr branch up to date status
91-
accept += ", application/vnd.github.merge-info-preview+json"
92-
if ghinstance.IsEnterprise(req.URL.Hostname()) {
93-
// shadow-cat-preview: Draft pull requests
94-
accept += ", application/vnd.github.shadow-cat-preview"
91+
accept := "application/vnd.github.merge-info-preview+json" // PullRequest.mergeStateStatus
92+
if ghinstance.IsEnterprise(getHost(req)) {
93+
accept += ", application/vnd.github.antiope-preview" // Commit.statusCheckRollup
94+
accept += ", application/vnd.github.shadow-cat-preview" // PullRequest.isDraft
9595
}
9696
return accept, nil
9797
}),
@@ -100,3 +100,10 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string
100100

101101
return api.NewHTTPClient(opts...)
102102
}
103+
104+
func getHost(r *http.Request) string {
105+
if r.Host != "" {
106+
return r.Host
107+
}
108+
return r.URL.Hostname()
109+
}

‎pkg/cmd/factory/http_test.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
package factory
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"net/http/httptest"
7+
"os"
8+
"regexp"
9+
"testing"
10+
11+
"github.com/MakeNowJust/heredoc"
12+
"github.com/cli/cli/pkg/iostreams"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestNewHTTPClient(t *testing.T) {
18+
type args struct {
19+
config configGetter
20+
appVersion string
21+
setAccept bool
22+
}
23+
tests := []struct {
24+
name string
25+
args args
26+
envDebug string
27+
host string
28+
wantHeader map[string]string
29+
wantStderr string
30+
}{
31+
{
32+
name: "github.com with Accept header",
33+
args: args{
34+
config: tinyConfig{"github.com:oauth_token": "MYTOKEN"},
35+
appVersion: "v1.2.3",
36+
setAccept: true,
37+
},
38+
host: "github.com",
39+
wantHeader: map[string]string{
40+
"authorization": "token MYTOKEN",
41+
"user-agent": "GitHub CLI v1.2.3",
42+
"accept": "application/vnd.github.merge-info-preview+json",
43+
},
44+
wantStderr: "",
45+
},
46+
{
47+
name: "github.com no Accept header",
48+
args: args{
49+
config: tinyConfig{"github.com:oauth_token": "MYTOKEN"},
50+
appVersion: "v1.2.3",
51+
setAccept: false,
52+
},
53+
host: "github.com",
54+
wantHeader: map[string]string{
55+
"authorization": "token MYTOKEN",
56+
"user-agent": "GitHub CLI v1.2.3",
57+
"accept": "",
58+
},
59+
wantStderr: "",
60+
},
61+
{
62+
name: "github.com no authentication token",
63+
args: args{
64+
config: tinyConfig{"example.com:oauth_token": "MYTOKEN"},
65+
appVersion: "v1.2.3",
66+
setAccept: true,
67+
},
68+
host: "github.com",
69+
wantHeader: map[string]string{
70+
"authorization": "",
71+
"user-agent": "GitHub CLI v1.2.3",
72+
"accept": "application/vnd.github.merge-info-preview+json",
73+
},
74+
wantStderr: "",
75+
},
76+
{
77+
name: "github.com in verbose mode",
78+
args: args{
79+
config: tinyConfig{"github.com:oauth_token": "MYTOKEN"},
80+
appVersion: "v1.2.3",
81+
setAccept: true,
82+
},
83+
host: "github.com",
84+
envDebug: "api",
85+
wantHeader: map[string]string{
86+
"authorization": "token MYTOKEN",
87+
"user-agent": "GitHub CLI v1.2.3",
88+
"accept": "application/vnd.github.merge-info-preview+json",
89+
},
90+
wantStderr: heredoc.Doc(`
91+
* Request at <time>
92+
* Request to http://<host>:<port>
93+
> GET / HTTP/1.1
94+
> Host: github.com
95+
> Accept: application/vnd.github.merge-info-preview+json
96+
> Authorization: token ████████████████████
97+
> User-Agent: GitHub CLI v1.2.3
98+
99+
< HTTP/1.1 204 No Content
100+
< Date: <time>
101+
102+
* Request took <duration>
103+
`),
104+
},
105+
{
106+
name: "GHES Accept header",
107+
args: args{
108+
config: tinyConfig{"example.com:oauth_token": "GHETOKEN"},
109+
appVersion: "v1.2.3",
110+
setAccept: true,
111+
},
112+
host: "example.com",
113+
wantHeader: map[string]string{
114+
"authorization": "token GHETOKEN",
115+
"user-agent": "GitHub CLI v1.2.3",
116+
"accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.antiope-preview, application/vnd.github.shadow-cat-preview",
117+
},
118+
wantStderr: "",
119+
},
120+
}
121+
122+
var gotReq *http.Request
123+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
124+
gotReq = r
125+
w.WriteHeader(http.StatusNoContent)
126+
}))
127+
defer ts.Close()
128+
129+
for _, tt := range tests {
130+
t.Run(tt.name, func(t *testing.T) {
131+
oldDebug := os.Getenv("DEBUG")
132+
os.Setenv("DEBUG", tt.envDebug)
133+
t.Cleanup(func() {
134+
os.Setenv("DEBUG", oldDebug)
135+
})
136+
137+
io, _, _, stderr := iostreams.Test()
138+
client := NewHTTPClient(io, tt.args.config, tt.args.appVersion, tt.args.setAccept)
139+
140+
req, err := http.NewRequest("GET", ts.URL, nil)
141+
req.Host = tt.host
142+
require.NoError(t, err)
143+
144+
res, err := client.Do(req)
145+
require.NoError(t, err)
146+
147+
for name, value := range tt.wantHeader {
148+
assert.Equal(t, value, gotReq.Header.Get(name), name)
149+
}
150+
151+
assert.Equal(t, 204, res.StatusCode)
152+
assert.Equal(t, tt.wantStderr, normalizeVerboseLog(stderr.String()))
153+
})
154+
}
155+
}
156+
157+
type tinyConfig map[string]string
158+
159+
func (c tinyConfig) Get(host, key string) (string, error) {
160+
return c[fmt.Sprintf("%s:%s", host, key)], nil
161+
}
162+
163+
var requestAtRE = regexp.MustCompile(`(?m)^\* Request at .+`)
164+
var dateRE = regexp.MustCompile(`(?m)^< Date: .+`)
165+
var hostWithPortRE = regexp.MustCompile(`127\.0\.0\.1:\d+`)
166+
var durationRE = regexp.MustCompile(`(?m)^\* Request took .+`)
167+
168+
func normalizeVerboseLog(t string) string {
169+
t = requestAtRE.ReplaceAllString(t, "* Request at <time>")
170+
t = hostWithPortRE.ReplaceAllString(t, "<host>:<port>")
171+
t = dateRE.ReplaceAllString(t, "< Date: <time>")
172+
t = durationRE.ReplaceAllString(t, "* Request took <duration>")
173+
return t
174+
}

0 commit comments

Comments
 (0)