gitproto: drop peeled "^{}" refs from AdvRefsToSlice

fedc41a→main·

nodo·2mo ago·2 files·+39 added/-1 removed

Pushes against any v1 target were failing with HTTP 400 "invalid reference name: refs/tags/^{}" whenever the planner scheduled a delete on a repo containing annotated tags. Affects every mode whose planner can produce deletes (replicate always, sync with --prune); bootstrap and no-prune sync are latently affected via extra have lines but don't fail at the wire level.

v1 target advertises annotated tags as TWO lines: refs/tags/X
refs/tags/X^{} ← wire metadata, not a ref

go-git v6 alpha.3 packp.AdvRefs stores peeled lines inline in References with the "^{}" suffix preserved — there is no separate Peeled map. ResolvedReferences() copies them verbatim, so RefHashMap put refs/tags/X^{} into target.refMap, and the planner scheduled a delete for it:

POST git-receive-pack
0000…0000 refs/tags/X^{}
──▶ HTTP 400
invalid reference name

The v2 ls-refs path returns peel info as an attribute and decodeV2LSRefs already discards it, so the bug only ever surfaced on v1.

Fix at the v1-to-slice boundary: AdvRefsToSlice now drops any entry where Name().IsPeeled(). It's the single chokepoint — nothing else reads adv.References directly — and the invariant "this function never returns a name ending in ^{}" is trivial to state, test, and review.

Changes

2

• internal/gitproto

• Mrefs.go+14/-1

• Mrefs_test.go+25

    86 unmodified lines
    
    87
    88
    89
    90
    91
    92
    93
    94
    95
    96
    97
    98
    99
    100
    95
    101
    102
    103
    104
    105
    106
    107
    108
    109
    110
    111
    
    86 unmodified lines
    
    }
    
    // AdvRefsToSlice converts an AdvRefs to a slice of references.
    //
    // Peeled tag entries (names ending in "^{}") are dropped: they are
    // wire-protocol metadata exposing the commit a tag points to, not real refs.
    // Including them in target ref maps causes the planner to schedule a delete
    // for a non-existent ref, which receive-pack rejects with HTTP 400
    // "invalid reference name".
    func AdvRefsToSlice(ar *packp.AdvRefs) ([]*plumbing.Reference, error) {
        refs, err := ar.ResolvedReferences()
        if err != nil {
            return nil, fmt.Errorf("resolved references: %w", err)
        }
        return refs, nil
        out := refs[:0]
        for _, ref := range refs {
            if ref.Name().IsPeeled() {
                continue
            }
            out = append(out, ref)
        }
        return out, nil
    }
    
    // AdvRefsCaps returns the sorted capability list from an AdvRefs.
    ```

Minternal/gitproto/refs.go+14/-1

129 unmodified lines

130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160

129 unmodified lines

}

// Regression: v1 advertisements include peeled "^{}" entries for annotated tags
// to expose the commit the tag points at. They are wire-protocol metadata, not
// real refs — receive-pack rejects them with "invalid reference name" when the
// planner schedules a delete for one. AdvRefsToSlice must drop them.
func TestAdvRefsToSliceDropsPeeledTagEntries(t *testing.T) {
    adv := &packp.AdvRefs{}
    tagHash := plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
    commitHash := plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")
    adv.References = []*plumbing.Reference{
    plumbing.NewHashReference("refs/tags/v1", tagHash),
    plumbing.NewHashReference("refs/tags/v1^{}", commitHash),
}

refs, err := AdvRefsToSlice(adv) if err != nil { t.Fatalf("AdvRefsToSlice: %v", err) } if len(refs) != 1 { t.Fatalf("expected 1 ref after peeled drop, got %d: %v", len(refs), refs) } if refs[0].Name() != "refs/tags/v1" { t.Errorf("got %s, want refs/tags/v1", refs[0].Name()) } }

func TestDecodeV1AdvRefs(t *testing.T) { // Empty data should return ErrEmptyRemoteRepository. _, err := decodeV1AdvRefs(nil)

```

Minternal/gitproto/refs_test.go+25