Preserve target endpoint across HEAD discovery when redirects are followed

5a9df91·

Soph·2mo ago·2 files·+70 added/-1 removed

DiscoverHEAD reuses targetConn, which Pusher captured after the receive-pack info-refs settled on the resolved push host under FollowInfoRefsRedirect. If the target's own upload-pack info-refs redirects to a different host (a common read-replica setup), RequestInfoRefs would clobber conn.Endpoint.Host/Scheme to the read host, and subsequent push POSTs would land on a host that 404s receive-pack.

Fix: temporarily toggle conn.FollowInfoRefsRedirect off during the DiscoverHEAD call. http.Client still follows the 30x internally to read the advertisement; we just suppress the post-call conn.Endpoint mutation, leaving Pusher's endpoint intact.

Regression test wraps the real receive-pack server with a handler that 307s GETs of /info/refs?service=git-upload-pack to a decoy host that 404s POSTs. Without the fix, sync fails with "decoy refuses POSTs"; with it, push lands on the real receive-pack host.

Refs #45.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

Sessions

7e51a96fa59aView transcript

Changes

2

• internal/syncer
• Mintegration_test.go+59
• Msyncer.go+11/-1

2888 unmodified lines

2889
2890
2891
2892
2893
2894
2895
2896
2897
2898
2899
2900
2901
2902
2903
2904
2905
2906
2907
2908
2909
2910
2911
2912
2913
2914
2915
2916
2917
2918
2919
2920
2921
2922
2923
2924
2925
2926
2927
2928
2929
2930
2931
2932
2933
2934
2935
2936
2937
2938
2939
2940
2941
2942
2943
2944
2945
2946
2947
2948
2949
2950
2951
2952
2953

2888 unmodified lines

}
}

// Regression: when target has FollowInfoRefsRedirect=true and the target's
// upload-pack info-refs redirects to a different host (real-world read-
// replica setup), the HEAD discovery must not clobber the receive-pack-
// resolved endpoint that Pusher already captured. Before the fix, sync
// would push to the upload-pack redirect host and the receive-pack POST
// would 404.
func TestRun_IntegrationSyncTargetFollowRedirectPreservesPushHost(t *testing.T) {
    sourceRepo, sourceFS := newSourceRepo(t)
    makeCommits(t, sourceRepo, sourceFS, 1)
    targetRepo, _ := seedTargetWithFeatureHEAD(t, sourceRepo)
    makeCommits(t, sourceRepo, sourceFS, 1)

sourceServer := newSmartHTTPRepoServerV2(t, sourceRepo)
    defer sourceServer.Close()

// decoy serves upload-pack info-refs (same repo, so the advert is
    // well-formed) but 404s POSTs. With the bug present, sync would push
    // here and fail; with the fix, push lands on realTarget.
    decoy := newSmartHTTPRepoServer(t, targetRepo)
    defer decoy.Close()
    decoy.receivePackRaw = func(w http.ResponseWriter, _ *http.Request) bool {
        http.Error(w, "decoy refuses POSTs", http.StatusNotFound)
        return true
    }

realInner := newSmartHTTPRepoServer(t, targetRepo)
    defer realInner.Close()

// realTarget wraps realInner: GETs for upload-pack info-refs redirect
    // to decoy (simulating a server that routes reads to a replica);
    // everything else delegates to realInner. The wrapper has its own
    // Host (own httptest.Server) so the redirect destination differs.
    realTarget := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        if r.Method == http.MethodGet &&
            r.URL.Path == realInner.repoPath+"/info/refs" &&
            r.URL.Query().Get("service") == serviceUploadPack {
            http.Redirect(w, r, decoy.server.URL+r.URL.Path+"?"+r.URL.RawQuery, http.StatusTemporaryRedirect)
            return
        }
        realInner.handle(w, r)
    }))
    defer realTarget.Close()

result, err := Run(context.Background(), Config{
        Source:       Endpoint{URL: sourceServer.RepoURL()},
        Target:       Endpoint{URL: realTarget.URL + realInner.repoPath, FollowInfoRefsRedirect: true},
        ProtocolMode: protocolModeAuto,
    })
    if err != nil {
        t.Fatalf("sync: %v", err)
    }
    if decoy.Count(serviceReceivePack, metricPack) > 0 {
        t.Errorf("push leaked to decoy host: bug regression")
    }
    if realInner.Count(serviceReceivePack, metricPack) == 0 {
        t.Errorf("expected push to land on real target after fix, got result=%+v", result)
    }
}

// Sync surfaces the source's HEAD symref via the existing upload-pack
// discovery — no extra round-trip needed for this side.
func TestRun_IntegrationSyncSurfacesSourceHEADInResult(t *testing.T) {

Minternal/syncer/integration_test.go+59

637 unmodified lines

638
639
640
641
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655

637 unmodified lines

// Receive-pack adverts omit HEAD, so make a second upload-pack
// info-refs call. Run after the receive-pack call (not concurrent):
// both share targetConn, and RequestInfoRefs mutates conn.Endpoint
// under FollowInfoRefsRedirect. Failures are non-fatal.
// under FollowInfoRefsRedirect. We also disable redirect-following
// for this single call — upload-pack and receive-pack on the same
// repo can redirect to different hosts (read replica vs push
// primary), and we don't want the discovery to clobber the
// receive-pack-resolved endpoint that Pusher already captured.
// http.Client still follows the redirect internally to read the
// advert; we just suppress the conn.Endpoint mutation. Failures
// are non-fatal.
prevFollow := targetConn.FollowInfoRefsRedirect
    targetConn.FollowInfoRefsRedirect = false
    targetHEAD, err := gitproto.DiscoverHEAD(ctx, targetConn)
    targetConn.FollowInfoRefsRedirect = prevFollow
    if err != nil {
        targetHEAD = ""
    }