diff --git a/go.mod b/go.mod index 11c11585..4c4177df 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.26.2 require ( github.com/go-git/go-billy/v6 v6.0.0-alpha.1 - github.com/go-git/go-git/v6 v6.0.0-alpha.3 + github.com/go-git/go-git/v6 v6.0.0-alpha.4.0.20260521151600-590487407c38 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 github.com/zalando/go-keyring v0.2.8 @@ -26,8 +26,8 @@ require ( github.com/pmezard/go-difflib v1.0.0 // indirect github.com/sergi/go-diff v1.4.0 // indirect github.com/spf13/pflag v1.0.9 // indirect - golang.org/x/crypto v0.50.0 // indirect - golang.org/x/net v0.53.0 // indirect + golang.org/x/crypto v0.51.0 // indirect + golang.org/x/net v0.54.0 // indirect golang.org/x/sync v0.20.0 // indirect golang.org/x/sys v0.44.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index e03b6aeb..bc7b5d32 100644 --- a/go.sum +++ b/go.sum @@ -22,10 +22,10 @@ github.com/go-git/gcfg/v2 v2.0.2 h1:MY5SIIfTGGEMhdA7d7JePuVVxtKL7Hp+ApGDJAJ7dpo= github.com/go-git/gcfg/v2 v2.0.2/go.mod h1:/lv2NsxvhepuMrldsFilrgct6pxzpGdSRC13ydTLSLs= github.com/go-git/go-billy/v6 v6.0.0-alpha.1 h1:xVjAR4oUvrKy7/Xuw/lLlV3gkxR3KO2H8W+MamuVVsQ= github.com/go-git/go-billy/v6 v6.0.0-alpha.1/go.mod h1:eaCUpHbedW7//EwcYmUDfJe2N6sJC9O12AT0OTqJR1E= -github.com/go-git/go-git-fixtures/v6 v6.0.0-20260422085740-0c07409f52ec h1:FpCNUs50xfQyJJs31uO3mDnqU855OhzAzfkkTgE6/DI= -github.com/go-git/go-git-fixtures/v6 v6.0.0-20260422085740-0c07409f52ec/go.mod h1:F1SpxOny2UYXu62DzjEH4UqBjk4AoGs27cA8I9buK+o= -github.com/go-git/go-git/v6 v6.0.0-alpha.3 h1:lJGritJ5AcC0X7buV0lReZ4cEHqcKB3Ab2ZjD3Ku+Ss= -github.com/go-git/go-git/v6 v6.0.0-alpha.3/go.mod h1:DGnqu+twdAgtDx/4tQTWFrVE1an+2ACph3W9yOfSJZM= +github.com/go-git/go-git-fixtures/v6 v6.0.0-alpha.1 h1:gmqi2jvsreu0s8JMLylYDFq4sbjHwwlhktMw0DUg3mA= +github.com/go-git/go-git-fixtures/v6 v6.0.0-alpha.1/go.mod h1:ECf1MqJlBdYpKggBrOXjo/0EnvRZx6D++I86UYjPgAQ= +github.com/go-git/go-git/v6 v6.0.0-alpha.4.0.20260521151600-590487407c38 h1:uA2L2RZQTkmvHjzBqMNMFR+UWdjicJBc0UqhCrgodZs= +github.com/go-git/go-git/v6 v6.0.0-alpha.4.0.20260521151600-590487407c38/go.mod h1:4ODa/G7hPWrh4Y+7lmt59Ij3zW38IEfvRoAZxLYYBhc= github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ= github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= @@ -59,18 +59,18 @@ github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPccHs= github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= -golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= -golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= -golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= +golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= +golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= +golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= +golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/term v0.42.0 h1:UiKe+zDFmJobeJ5ggPwOshJIVt6/Ft0rcfrXZDLWAWY= -golang.org/x/term v0.42.0/go.mod h1:Dq/D+snpsbazcBG5+F9Q1n2rXV8Ma+71xEjTRufARgY= -golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= -golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= +golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= +golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/gitproto/push.go b/internal/gitproto/push.go index 704601da..39abeefb 100644 --- a/internal/gitproto/push.go +++ b/internal/gitproto/push.go @@ -8,6 +8,8 @@ import ( "io" "os" "strings" + "sync/atomic" + "time" "github.com/go-git/go-git/v6/plumbing" "github.com/go-git/go-git/v6/plumbing/format/packfile" @@ -199,6 +201,15 @@ func sendReceivePack( } // PushObjects pushes locally-materialized objects to the target. +// +// Delta selection runs synchronously up front via +// packfile.DeltaSelector. The selected objects are then handed back to +// a packfile.Encoder behind a passthrough ObjectSelector, so the +// encoder's write phase (Encode → encode(objects)) streams pack bytes +// continuously into an io.Pipe to the HTTP request body. This avoids +// the mid-stream stall that occurs when Encode runs selection itself — +// CDN edges treat the resulting idle gap as a stalled upload and close +// the connection. See go-git PR #2142 for the API hook. func PushObjects( ctx context.Context, conn Conn, @@ -217,12 +228,24 @@ func PushObjects( return sendReceivePack(ctx, conn, req, nil, verbose, onRejection) } + progressDest := progressSink(verbose, "target: ", conn.ProgressWriter()) + + stopSelect := startSelectionProgress(progressDest) + objects, err := packfile.NewDeltaSelector(store).ObjectsToPack(hashes, 10) + stopSelect(len(objects), err) + if err != nil { + return fmt.Errorf("select objects to pack: %w", err) + } + useRefDeltas := !adv.Capabilities.Supports(capability.OFSDelta) pr, pw := io.Pipe() done := make(chan error, 1) - go func() { - enc := packfile.NewEncoder(pw, store, useRefDeltas) + cw := &countingWriter{w: pw} + stopWrite := startPackWriteProgress(cw, progressDest) + defer stopWrite() + enc := packfile.NewEncoder(cw, store, useRefDeltas, + packfile.WithObjectSelector(precomputedSelector{objects: objects})) if _, err := enc.Encode(hashes, 10); err != nil { done <- pw.CloseWithError(fmt.Errorf("encode packfile: %w", err)) return @@ -239,6 +262,141 @@ func PushObjects( return encodeErr } +// precomputedSelector is a packfile.ObjectSelector that returns a +// fixed []*packfile.ObjectToPack, ignoring its arguments. It is the +// passthrough used by PushObjects to feed pre-selected objects back +// into packfile.Encoder via WithObjectSelector. Used exactly once per +// PushObjects call and not exposed outside this package. +type precomputedSelector struct { + objects []*packfile.ObjectToPack +} + +func (p precomputedSelector) ObjectsToPack(_ []plumbing.Hash, _ uint) ([]*packfile.ObjectToPack, error) { + return p.objects, nil +} + +// countingWriter wraps an io.Writer and tracks total bytes written. +// The count is read by the progress ticker concurrently with the +// encoder's writes, so the counter is atomic. +type countingWriter struct { + w io.Writer + n atomic.Int64 +} + +func (cw *countingWriter) Write(p []byte) (int, error) { + n, err := cw.w.Write(p) + cw.n.Add(int64(n)) + if err != nil { + return n, fmt.Errorf("counting writer: %w", err) + } + return n, nil +} + +func (cw *countingWriter) Count() int64 { return cw.n.Load() } + +// startSelectionProgress emits in-place "selecting deltas, elapsed X" +// updates every 500ms during the synchronous delta-selection phase of +// PushObjects. The returned stop function takes the number of selected +// objects and the selection error (nil on success); on success it +// finalizes the line with a permanent "selected N objects in Y" +// summary, on error it just stops the ticker without claiming success. +// When dest is nil (non-verbose mode) returns a no-op stop, so +// callers don't need to special-case verbosity. +// +// Selection has no observable byte progress — go-git's DeltaSelector +// is opaque to the caller — so elapsed time is the only signal we can +// surface to keep long selections from looking like a hang. +func startSelectionProgress(dest io.Writer) func(objectCount int, err error) { + if dest == nil { + return func(int, error) {} + } + start := time.Now() + ticker := time.NewTicker(500 * time.Millisecond) + stop := make(chan struct{}) + done := make(chan struct{}) + go func() { + defer close(done) + for { + select { + case <-stop: + return + case <-ticker.C: + fmt.Fprintf(dest, "selecting deltas, elapsed %s\r", + time.Since(start).Round(time.Second)) + } + } + }() + return func(objectCount int, err error) { + ticker.Stop() + close(stop) + <-done + if err != nil { + return + } + fmt.Fprintf(dest, "selected %d objects in %s\n", + objectCount, time.Since(start).Round(time.Second)) + } +} + +// startPackWriteProgress emits in-place "encoding pack: N MB, elapsed +// X" updates every 500ms while the encoder writes pack bytes through +// cw. The returned stop function finalizes the line with a permanent +// "encoded pack" summary. Single-use, typically via defer. When dest +// is nil returns a no-op stop. +// +// This is the second of two phases visible to a materialized push: +// startSelectionProgress runs synchronously first, then +// startPackWriteProgress takes over once selection has completed and +// the encoder begins streaming bytes to the request body. +func startPackWriteProgress(cw *countingWriter, dest io.Writer) func() { + if dest == nil { + return func() {} + } + start := time.Now() + ticker := time.NewTicker(500 * time.Millisecond) + stop := make(chan struct{}) + done := make(chan struct{}) + go func() { + defer close(done) + for { + select { + case <-stop: + return + case <-ticker.C: + fmt.Fprintf(dest, "encoding pack: %s, elapsed %s\r", + humanizeBytes(cw.Count()), time.Since(start).Round(time.Second)) + } + } + }() + return func() { + ticker.Stop() + close(stop) + <-done + fmt.Fprintf(dest, "encoded pack: %s in %s\n", + humanizeBytes(cw.Count()), time.Since(start).Round(time.Second)) + } +} + +// humanizeBytes renders n in IEC units with one decimal place for KB+ +// (e.g. "47.3 MB"). Anything below 1 KB is shown as raw bytes. +func humanizeBytes(n int64) string { + const ( + kb = 1024 + mb = kb * 1024 + gb = mb * 1024 + ) + switch { + case n < kb: + return fmt.Sprintf("%d B", n) + case n < mb: + return fmt.Sprintf("%.1f KB", float64(n)/float64(kb)) + case n < gb: + return fmt.Sprintf("%.1f MB", float64(n)/float64(mb)) + default: + return fmt.Sprintf("%.1f GB", float64(n)/float64(gb)) + } +} + // PushPack pushes a pack stream (relay) to the target. func PushPack( ctx context.Context, diff --git a/internal/gitproto/push_test.go b/internal/gitproto/push_test.go index 75265af2..5234f0d5 100644 --- a/internal/gitproto/push_test.go +++ b/internal/gitproto/push_test.go @@ -15,6 +15,7 @@ import ( "github.com/go-git/go-git/v6/plumbing/protocol/capability" "github.com/go-git/go-git/v6/plumbing/protocol/packp" "github.com/go-git/go-git/v6/plumbing/transport" + "github.com/go-git/go-git/v6/storage/memory" "github.com/stretchr/testify/require" ) @@ -227,6 +228,11 @@ func TestPushPackClosesPackOnContextCanceled(t *testing.T) { } } +// TestPushPackStartsHTTPBeforePackFullyRead asserts that PushPack — the +// relay path — keeps streaming source pack bytes through to the target +// with chunked encoding. Materialized push (PushObjects) gets the same +// property via precomputed delta selection; see +// TestPushObjectsStreamsBody. func TestPushPackStartsHTTPBeforePackFullyRead(t *testing.T) { started := make(chan struct{}, 1) release := make(chan struct{}) @@ -276,6 +282,72 @@ func TestPushPackStartsHTTPBeforePackFullyRead(t *testing.T) { } } +// TestPushObjectsStreamsBody asserts that PushObjects sends a chunked +// receive-pack request — the streaming property is what avoids the +// mid-stream stall (delta selection runs before the body opens, so +// pack bytes flow continuously once writing starts). A request with +// no Transfer-Encoding: chunked would mean we've regressed to +// buffering the whole pack before sending. +func TestPushObjectsStreamsBody(t *testing.T) { + type observation struct { + transferEncoding []string + contentLength int64 + bodyLen int64 + } + observed := make(chan observation, 1) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n, err := io.Copy(io.Discard, r.Body) + if err != nil { + t.Logf("drain request body: %v", err) + } + _ = r.Body.Close() + observed <- observation{ + transferEncoding: r.TransferEncoding, + contentLength: r.ContentLength, + bodyLen: n, + } + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + conn := connForServer(t, srv) + adv := &packp.AdvRefs{} + adv.Capabilities.Set(capability.OFSDelta) + + err := PushObjects(context.Background(), conn, adv, []PushCommand{{ + Name: "refs/heads/main", + New: plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), + }}, memory.NewStorage(), nil, false, nil) + if err != nil { + t.Fatalf("PushObjects: %v", err) + } + + var obs observation + select { + case obs = <-observed: + case <-time.After(2 * time.Second): + t.Fatal("server did not receive request") + } + + chunked := false + for _, te := range obs.transferEncoding { + if te == "chunked" { + chunked = true + break + } + } + if !chunked { + t.Errorf("Transfer-Encoding = %v, want to include \"chunked\"", obs.transferEncoding) + } + if obs.contentLength != -1 { + t.Errorf("Content-Length = %d, want -1 (unknown for chunked)", obs.contentLength) + } + if obs.bodyLen <= 0 { + t.Errorf("bodyLen = %d, want > 0", obs.bodyLen) + } +} + func TestBuildUpdateRequest(t *testing.T) { adv := &packp.AdvRefs{} adv.Capabilities.Set(capability.ReportStatus) diff --git a/internal/gitproto/smarthttp.go b/internal/gitproto/smarthttp.go index 90c5fe45..0da47835 100644 --- a/internal/gitproto/smarthttp.go +++ b/internal/gitproto/smarthttp.go @@ -8,7 +8,10 @@ import ( "io" "mime" "net/http" + "net/http/httptrace" + "net/http/httputil" "net/url" + "os" "strings" "github.com/go-git/go-git/v6/plumbing/protocol/capability" @@ -63,6 +66,111 @@ func httpError(res *http.Response) error { // current git-sync stats phase for round-trip tracking. const StatsPhaseHeader = "X-Git-Sync-Stats-Phase" +// HTTPTraceEnv enables verbose httptrace logging to stderr when set to any +// non-empty value other than "0" or "false". Diagnoses connection-pool +// behavior against hosts that close idle keep-alive connections more +// aggressively than Go's transport assumes (CDN edges, some hosted git +// providers) — a stale pooled connection surfaces as "use of closed network +// connection" on the next POST. Off by default; zero overhead unless set. +const HTTPTraceEnv = "GITSYNC_HTTP_TRACE" + +func httpTraceEnabled() bool { + v := os.Getenv(HTTPTraceEnv) + if v == "" { + return false + } + switch strings.ToLower(v) { + case "0", "false", "no", "off": + return false + } + return true +} + +// withHTTPTrace returns ctx with a ClientTrace that logs connection lifecycle +// events for one request to stderr. label is prepended to every line so +// concurrent or interleaved requests stay readable. Returns ctx unchanged +// when GITSYNC_HTTP_TRACE is not enabled. +func withHTTPTrace(ctx context.Context, label string) context.Context { + if !httpTraceEnabled() { + return ctx + } + trace := &httptrace.ClientTrace{ + GetConn: func(hostPort string) { + fmt.Fprintf(os.Stderr, "[httptrace] %s GetConn %s\n", label, hostPort) + }, + GotConn: func(info httptrace.GotConnInfo) { + fmt.Fprintf(os.Stderr, + "[httptrace] %s GotConn reused=%v wasIdle=%v idle=%s local=%s remote=%s\n", + label, info.Reused, info.WasIdle, info.IdleTime, + info.Conn.LocalAddr(), info.Conn.RemoteAddr()) + }, + PutIdleConn: func(err error) { + if err != nil { + fmt.Fprintf(os.Stderr, "[httptrace] %s PutIdleConn err=%v\n", label, err) + } else { + fmt.Fprintf(os.Stderr, "[httptrace] %s PutIdleConn ok\n", label) + } + }, + ConnectStart: func(network, addr string) { + fmt.Fprintf(os.Stderr, "[httptrace] %s ConnectStart %s %s\n", label, network, addr) + }, + ConnectDone: func(network, addr string, err error) { + fmt.Fprintf(os.Stderr, "[httptrace] %s ConnectDone %s %s err=%v\n", label, network, addr, err) + }, + TLSHandshakeStart: func() { + fmt.Fprintf(os.Stderr, "[httptrace] %s TLSHandshakeStart\n", label) + }, + TLSHandshakeDone: func(state tls.ConnectionState, err error) { + fmt.Fprintf(os.Stderr, "[httptrace] %s TLSHandshakeDone resumed=%v err=%v\n", + label, state.DidResume, err) + }, + WroteRequest: func(info httptrace.WroteRequestInfo) { + fmt.Fprintf(os.Stderr, "[httptrace] %s WroteRequest err=%v\n", label, info.Err) + }, + } + return httptrace.WithClientTrace(ctx, trace) +} + +// dumpOutgoingRequest prints the wire-format request line and headers for +// req to stderr, prefixed with label. The body is not consumed (passes +// body=false to httputil.DumpRequestOut), but Transfer-Encoding and +// Content-Length will reflect what Go's transport would actually send. +// Useful when a server behaves unexpectedly on a POST and you need to +// see what the request looked like at the protocol level — the +// connection-level trace tells you which TCP/TLS connection was used +// but not what was written on it. Best-effort: dump errors are +// surfaced as a single line so a transient dump failure doesn't mask +// the underlying request. +func dumpOutgoingRequest(req *http.Request, label string) { + dump, err := httputil.DumpRequestOut(req, false) + if err != nil { + fmt.Fprintf(os.Stderr, "[httptrace] %s dump error: %v\n", label, err) + return + } + fmt.Fprintf(os.Stderr, "[httptrace] %s outgoing request:\n%s\n", label, redactAuthorization(dump)) +} + +// redactAuthorization scrubs any Authorization header value from a dumped +// HTTP request so the credentials don't leak into stderr when +// GITSYNC_HTTP_TRACE is enabled in environments with shoulder-surfers, +// pasted-into-tickets logs, or shared shells. +func redactAuthorization(dump []byte) []byte { + const header = "Authorization:" + idx := bytes.Index(dump, []byte(header)) + if idx < 0 { + return dump + } + end := bytes.IndexByte(dump[idx:], '\n') + if end < 0 { + end = len(dump) - idx + } + out := make([]byte, 0, len(dump)) + out = append(out, dump[:idx]...) + out = append(out, []byte(header+" [REDACTED]")...) + out = append(out, dump[idx+end:]...) + return out +} + // AuthMethod authorizes outbound HTTP requests for a remote. It is satisfied // by *transporthttp.BasicAuth and *transporthttp.TokenAuth, whose Authorizer // methods replaced the AuthMethod interface that go-git removed in v6 alpha.2. @@ -134,20 +242,38 @@ func normalizeEndpointPath(ep *url.URL) { ep.RawPath = strings.TrimRight(ep.RawPath, "/") } -// NewHTTPTransport creates an http.Transport with optional TLS skip. +// NewHTTPTransport returns the default git-sync HTTP transport. It clones +// http.DefaultTransport so config changes (TLS, keep-alive policy) don't +// leak into other code in the same process. +// +// Keep-alives are disabled. The git smart-HTTP workflow over the same host +// is coarse-grained — info/refs, then a single upload-pack or receive-pack +// POST — with real work in between (planning, source fetch, local object +// materialization). On the push side that gap is long enough for CDN +// edges and some hosted git providers to close their end of an idle TLS +// socket; the next POST then fails with "use of closed network connection" +// because the pooled connection is half-dead. Pool reuse would save at +// most one TLS handshake per sync, which is negligible against multi-MB +// to multi-GB transfers, so we prefer a fresh connection per request and +// avoid the race entirely. +// +// Library callers that need pool reuse (e.g. embedding git-sync in a +// long-running process that hits the same host repeatedly with short +// gaps) can pass their own RoundTripper to NewHTTPConn instead. func NewHTTPTransport(skipTLS bool) http.RoundTripper { - if !skipTLS { + base, ok := http.DefaultTransport.(*http.Transport) + if !ok { return http.DefaultTransport } - if cloned, ok := http.DefaultTransport.(*http.Transport); ok { - tc := cloned.Clone() + tc := base.Clone() + tc.DisableKeepAlives = true + if skipTLS { if tc.TLSClientConfig == nil { tc.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} } tc.TLSClientConfig.InsecureSkipVerify = true - return tc } - return http.DefaultTransport + return tc } // RequestInfoRefs fetches /info/refs for the given service. @@ -162,6 +288,7 @@ func RequestInfoRefs(ctx context.Context, conn Conn, service string, gitProtocol // RequestInfoRefs fetches /info/refs for the given service. func (c *HTTPConn) RequestInfoRefs(ctx context.Context, service string, gitProtocol string) ([]byte, error) { reqURL := fmt.Sprintf("%s/info/refs?service=%s", c.EndpointURL.String(), service) + ctx = withHTTPTrace(ctx, "GET "+service+"/info/refs") req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) if err != nil { return nil, fmt.Errorf("create info-refs request: %w", err) @@ -251,8 +378,12 @@ func PostRPCStreamBody(ctx context.Context, conn Conn, service string, body io.R // PostRPCStreamBody sends a POST to the given service using a streaming request body. // Caller must close the returned ReadCloser. +// +// The body is sent as-is — streaming readers produce a chunked request. func (c *HTTPConn) PostRPCStreamBody(ctx context.Context, service string, body io.Reader, v2 bool, phase string) (io.ReadCloser, error) { reqURL := fmt.Sprintf("%s/%s", c.EndpointURL.String(), service) + ctx = withHTTPTrace(ctx, "POST "+service) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, body) if err != nil { return nil, fmt.Errorf("create RPC request: %w", err) @@ -266,6 +397,10 @@ func (c *HTTPConn) PostRPCStreamBody(ctx context.Context, service string, body i } ApplyAuth(req, c.Auth) + if httpTraceEnabled() { + dumpOutgoingRequest(req, "POST "+service) + } + res, err := c.HTTP.Do(req) if err != nil { return nil, fmt.Errorf("post RPC: %w", err) diff --git a/internal/gitproto/smarthttp_test.go b/internal/gitproto/smarthttp_test.go index 4ba264eb..176c354e 100644 --- a/internal/gitproto/smarthttp_test.go +++ b/internal/gitproto/smarthttp_test.go @@ -83,22 +83,35 @@ func TestNewHTTPConnStripsTrailingEndpointSlash(t *testing.T) { } func TestNewHTTPTransport(t *testing.T) { - // Without TLS skip should return default transport. + // Default (no TLS skip) returns a cloned transport, not the shared + // http.DefaultTransport — config must not leak into other code. rt := NewHTTPTransport(false) - if rt != http.DefaultTransport { - t.Error("expected http.DefaultTransport when skipTLS is false") + if rt == http.DefaultTransport { + t.Error("expected a cloned transport, got shared http.DefaultTransport") + } + ht, ok := rt.(*http.Transport) + if !ok { + t.Fatalf("expected *http.Transport, got %T", rt) + } + if !ht.DisableKeepAlives { + t.Error("expected DisableKeepAlives = true on the default transport") } - // With TLS skip should return a transport with InsecureSkipVerify. + // With TLS skip we still get a cloned transport with keep-alives off, + // plus InsecureSkipVerify on the TLS config. rt = NewHTTPTransport(true) if rt == http.DefaultTransport { - t.Error("expected a different transport when skipTLS is true") + t.Error("expected a cloned transport when skipTLS is true") } - // Verify the returned transport is an *http.Transport with skip verify. - if ht, ok := rt.(*http.Transport); ok { - if ht.TLSClientConfig == nil || !ht.TLSClientConfig.InsecureSkipVerify { - t.Error("expected InsecureSkipVerify = true") - } + ht, ok = rt.(*http.Transport) + if !ok { + t.Fatalf("expected *http.Transport, got %T", rt) + } + if !ht.DisableKeepAlives { + t.Error("expected DisableKeepAlives = true when skipTLS is true") + } + if ht.TLSClientConfig == nil || !ht.TLSClientConfig.InsecureSkipVerify { + t.Error("expected InsecureSkipVerify = true when skipTLS is true") } } diff --git a/internal/syncer/progress.go b/internal/syncer/progress.go index 9a4c5f06..d8f869a3 100644 --- a/internal/syncer/progress.go +++ b/internal/syncer/progress.go @@ -349,8 +349,15 @@ func truncateHost(host string, width int) string { // line in two calls — first the prefix ("source: "), then the content // with terminator — and would otherwise produce two separate notify // frames split mid-line. Use as a pointer (the buffer is stateful). +// +// mu guards buf and serializes notify/setTransient calls against +// concurrent writers. The HTTP push path is the case that motivated +// this: a materialized-push encode goroutine and the receive-pack +// response demuxer can both emit progress through the same +// conn.ProgressWriter() during overlapping windows. type sessionStderr struct { s *syncSession + mu sync.Mutex buf strings.Builder } @@ -362,6 +369,8 @@ func (w *sessionStderr) Write(b []byte) (int, error) { } return n, nil } + w.mu.Lock() + defer w.mu.Unlock() s := string(b) for s != "" { i := strings.IndexAny(s, "\r\n")