Skip to content

Commit c53483a

Browse files
pjbgfpjbgf
authored
Merge pull request #1348 from entireio/useragent
git-remote-entire: stamp HTTP User-Agent on outbound requests
2 parents e3104a6 + 6684735 commit c53483a

5 files changed

Lines changed: 223 additions & 6 deletions

File tree

‎cmd/git-remote-entire/main.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,17 @@ func run(args []string) int {
6767
return 128
6868
}
6969

70-
// Build info drives the agent string the helper advertises upstream.
70+
// Build info drives the identifier the helper advertises upstream.
71+
// One string covers both surfaces:
72+
// - githelper.Agent rides in the git protocol pkt-line agent=
73+
// capability appended to upload-pack / receive-pack / v2 requests.
74+
// - httpUserAgent rides in the HTTP User-Agent header on every
75+
// outbound request so server access logs can attribute traffic.
76+
// Using the same value keeps the two log surfaces correlatable.
7177
versioninfo.Load()
72-
githelper.Agent = remotehelper.BinaryName + "/" + versioninfo.Commit
78+
helperAgent := remotehelper.BinaryName + "/" + versioninfo.Version
79+
githelper.Agent = helperAgent
80+
httpUserAgent := helperAgent
7381

7482
rawURL := args[2]
7583
parsedURL, err := url.Parse(rawURL)
@@ -98,8 +106,11 @@ func run(args []string) int {
98106
repoSlug := parsedURL.Path
99107

100108
httpClient := &http.Client{
101-
Timeout: 30 * time.Second,
102-
Transport: httpclient.NewTransport(skipTLS),
109+
Timeout: 30 * time.Second,
110+
Transport: &httpclient.UserAgentTransport{
111+
Next: httpclient.NewTransport(skipTLS),
112+
UA: httpUserAgent,
113+
},
103114
}
104115

105116
creds, err := resolveCreds(ctx, parsedURL, clusterBaseURL, skipTLS, httpClient)
@@ -132,6 +143,7 @@ func run(args []string) int {
132143
SkipTLS: skipTLS,
133144
SetAuth: setAuth,
134145
OnNodeFailed: onNodeFailed,
146+
UserAgent: httpUserAgent,
135147
})
136148

137149
protocolVersion := resolveProtocolVersion()

‎internal/entireclient/httpclient/useragent.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package httpclient
2+
3+
import "net/http"
4+
5+
// UserAgentTransport wraps another http.RoundTripper and stamps the
6+
// User-Agent header on every outgoing request so the server can
7+
// attribute traffic to the calling binary. Callers that already set
8+
// User-Agent are overwritten: the wrapper exists to give the binary a
9+
// single identity in upstream access logs, not to be overridden per
10+
// request.
11+
//
12+
// The wrapper clones the request before mutating headers so the
13+
// caller's original *http.Request is left untouched — important for
14+
// retries and for callers that hold a reference after Do returns.
15+
//
16+
// Concurrent use is safe iff Next is safe for concurrent use.
17+
type UserAgentTransport struct {
18+
Next http.RoundTripper
19+
UA string
20+
}
21+
22+
// RoundTrip implements http.RoundTripper.
23+
func (t *UserAgentTransport) RoundTrip(req *http.Request) (*http.Response, error) {
24+
r := req.Clone(req.Context())
25+
r.Header.Set("User-Agent", t.UA)
26+
//nolint:wrapcheck // thin passthrough: wrapping would change error semantics for callers that errors.As on transport errors.
27+
return t.Next.RoundTrip(r)
28+
}

‎internal/entireclient/httpclient/useragent_test.go‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package httpclient
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
)
8+
9+
func TestUserAgentTransport_SetsHeader(t *testing.T) {
10+
t.Parallel()
11+
12+
var got string
13+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
14+
got = r.Header.Get("User-Agent")
15+
w.WriteHeader(http.StatusOK)
16+
}))
17+
t.Cleanup(srv.Close)
18+
19+
client := &http.Client{
20+
Transport: &UserAgentTransport{
21+
Next: http.DefaultTransport,
22+
UA: "test-binary/1.2.3",
23+
},
24+
}
25+
req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL, nil)
26+
if err != nil {
27+
t.Fatalf("NewRequest: %v", err)
28+
}
29+
resp, err := client.Do(req)
30+
if err != nil {
31+
t.Fatalf("Do: %v", err)
32+
}
33+
_ = resp.Body.Close()
34+
35+
if want := "test-binary/1.2.3"; got != want {
36+
t.Errorf("User-Agent = %q, want %q", got, want)
37+
}
38+
}
39+
40+
func TestUserAgentTransport_OverwritesCallerHeader(t *testing.T) {
41+
t.Parallel()
42+
43+
var got string
44+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
45+
got = r.Header.Get("User-Agent")
46+
w.WriteHeader(http.StatusOK)
47+
}))
48+
t.Cleanup(srv.Close)
49+
50+
client := &http.Client{
51+
Transport: &UserAgentTransport{
52+
Next: http.DefaultTransport,
53+
UA: "wrapper-set",
54+
},
55+
}
56+
req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL, nil)
57+
if err != nil {
58+
t.Fatalf("NewRequest: %v", err)
59+
}
60+
req.Header.Set("User-Agent", "caller-set")
61+
resp, err := client.Do(req)
62+
if err != nil {
63+
t.Fatalf("Do: %v", err)
64+
}
65+
_ = resp.Body.Close()
66+
67+
if want := "wrapper-set"; got != want {
68+
t.Errorf("User-Agent = %q, want %q", got, want)
69+
}
70+
}
71+
72+
func TestUserAgentTransport_DoesNotMutateCallerRequest(t *testing.T) {
73+
t.Parallel()
74+
75+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
76+
w.WriteHeader(http.StatusOK)
77+
}))
78+
t.Cleanup(srv.Close)
79+
80+
client := &http.Client{
81+
Transport: &UserAgentTransport{
82+
Next: http.DefaultTransport,
83+
UA: "wrapper-set",
84+
},
85+
}
86+
req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL, nil)
87+
if err != nil {
88+
t.Fatalf("NewRequest: %v", err)
89+
}
90+
req.Header.Set("User-Agent", "caller-set")
91+
resp, err := client.Do(req)
92+
if err != nil {
93+
t.Fatalf("Do: %v", err)
94+
}
95+
_ = resp.Body.Close()
96+
97+
if got := req.Header.Get("User-Agent"); got != "caller-set" {
98+
t.Errorf("caller request mutated: User-Agent = %q, want %q", got, "caller-set")
99+
}
100+
}

‎internal/remotehelper/transport/proxy.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ type Config struct {
4141
SkipTLS bool
4242
SetAuth SetAuthFunc
4343
OnNodeFailed func(failedNode string)
44+
// UserAgent is stamped on every outbound HTTP request so the
45+
// server can attribute git smart-HTTP traffic to the remote
46+
// helper. Empty disables the wrapper, in which case the request
47+
// carries Go's default ("Go-http-client/1.1") — useful for tests
48+
// that don't care about identity. Production callers set it.
49+
UserAgent string
4450
}
4551

4652
// Proxy is the HTTP transport the helper protocol uses to talk to the
@@ -103,9 +109,17 @@ func New(cfg Config) *Proxy {
103109
p.repoPath = cfg.Nodes.RepoPath
104110
}
105111

106-
transport := httpclient.NewTransport(cfg.SkipTLS)
112+
// User-Agent must be set before httpdebug logs the request, so the
113+
// wrapper sits outside httpdebug. The order is:
114+
// user-agent → httpdebug → http.Transport
115+
// so the debug log captures the same headers the wire sees.
116+
var rt http.RoundTripper = httpclient.NewTransport(cfg.SkipTLS)
117+
rt = &httpdebug.RoundTripper{Next: rt}
118+
if cfg.UserAgent != "" {
119+
rt = &httpclient.UserAgentTransport{Next: rt, UA: cfg.UserAgent}
120+
}
107121
p.client = &http.Client{
108-
Transport: &httpdebug.RoundTripper{Next: transport},
122+
Transport: rt,
109123
CheckRedirect: p.checkRedirect,
110124
}
111125
return p

‎internal/remotehelper/transport/proxy_test.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,69 @@ func TestNewProxy(t *testing.T) {
114114

115115
const serviceParam = "git-upload-pack"
116116

117+
func TestProxy_SetsUserAgentHeader(t *testing.T) {
118+
t.Parallel()
119+
120+
var got string
121+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
122+
got = r.Header.Get("User-Agent")
123+
w.WriteHeader(http.StatusOK)
124+
}))
125+
t.Cleanup(server.Close)
126+
127+
p := New(Config{
128+
Nodes: replicas.NodeConfig{
129+
InitialNodes: []string{server.URL},
130+
EntryURL: server.URL,
131+
ClusterHost: mustHost(t, server.URL),
132+
RepoPath: "owner/repo",
133+
},
134+
Path: "/et/owner/repo",
135+
UserAgent: "git-remote-entire/9.9.9",
136+
})
137+
138+
resp, err := p.InfoRefs(t.Context(), serviceParam)
139+
if err != nil {
140+
t.Fatalf("InfoRefs: %v", err)
141+
}
142+
_ = resp.Close()
143+
144+
if want := "git-remote-entire/9.9.9"; got != want {
145+
t.Errorf("User-Agent = %q, want %q", got, want)
146+
}
147+
}
148+
149+
func TestProxy_OmitsUserAgentWrapperWhenEmpty(t *testing.T) {
150+
t.Parallel()
151+
152+
var got string
153+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
154+
got = r.Header.Get("User-Agent")
155+
w.WriteHeader(http.StatusOK)
156+
}))
157+
t.Cleanup(server.Close)
158+
159+
p := New(Config{
160+
Nodes: replicas.NodeConfig{
161+
InitialNodes: []string{server.URL},
162+
EntryURL: server.URL,
163+
ClusterHost: mustHost(t, server.URL),
164+
RepoPath: "owner/repo",
165+
},
166+
Path: "/et/owner/repo",
167+
})
168+
169+
resp, err := p.InfoRefs(t.Context(), serviceParam)
170+
if err != nil {
171+
t.Fatalf("InfoRefs: %v", err)
172+
}
173+
_ = resp.Close()
174+
175+
if !strings.HasPrefix(got, "Go-http-client/") {
176+
t.Errorf("User-Agent = %q, want Go's default when wrapper omitted", got)
177+
}
178+
}
179+
117180
func TestInfoRefs(t *testing.T) {
118181
t.Parallel()
119182
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

0 commit comments

Comments
 (0)