From 73fa9ef25a8af9c8337a4cf34a67cfe208f1a7c5 Mon Sep 17 00:00:00 2001 From: Alberto Cortés Date: Wed, 26 Oct 2016 17:56:26 +0200 Subject: Use advrefs in gituploadpackinfo (#92) * add advrefs encoder and parser * modify advrefs encoder to resemble json encoder * turn advrefs parser into a decoder * clean code * improve documentation * improve documentation * clean code * upgrade to new pktline.Add and add Flush const to easy integration * gometalinter * Use packp/advrefs for GitUploadPackInfo parsing - GitUploadPackInfo now uses packp/advrefs instead of parsing the message by itself. - Capabilities has been moved from clients/common to packp to avoid a circular import. - Cleaning of advrefs_test code. - Add support for prefix encoding and decoding in advrefs. * clean advrefs test code * clean advrefs test code * clean advrefs test code * gometalinter * add pktline encoder * change pktline.EncodeFlush to pktline.Flush * make scanner tests use the encoder instead of Pktlines * check errors on flush and clean constants * ubstitute the PktLines type with a pktline.Encoder * use pktline.Encoder in all go-git * add example of pktline.Encodef() * add package overview * documentation * support symbolic links other than HEAD * simplify decoding of shallows * packp: fix mcuadros comments - all abbreviates removed (by visual inspection, some may remain) - all empty maps are initialized using make - simplify readRef with a switch - make decodeShallow malformed error more verbose - add pktline.Encoder.encodeLine - remove infamous panic in checkPayloadLength by refactoring out the whole function --- clients/common/common.go | 268 ++++++++++------------------------------ clients/common/common_test.go | 43 +------ clients/http/git_upload_pack.go | 2 +- clients/ssh/git_upload_pack.go | 3 +- 4 files changed, 69 insertions(+), 247 deletions(-) (limited to 'clients') diff --git a/clients/common/common.go b/clients/common/common.go index 0179bfd..c723cdd 100644 --- a/clients/common/common.go +++ b/clients/common/common.go @@ -2,6 +2,7 @@ package common import ( + "bytes" "errors" "fmt" "io" @@ -11,6 +12,8 @@ import ( "strings" "gopkg.in/src-d/go-git.v4/core" + "gopkg.in/src-d/go-git.v4/formats/packp" + "gopkg.in/src-d/go-git.v4/formats/packp/advrefs" "gopkg.in/src-d/go-git.v4/formats/packp/pktline" "gopkg.in/src-d/go-git.v4/storage/memory" ) @@ -75,246 +78,100 @@ func (e *Endpoint) String() string { return u.String() } -// Capabilities contains all the server capabilities -// https://github.com/git/git/blob/master/Documentation/technical/protocol-capabilities.txt -type Capabilities struct { - m map[string]*Capability - o []string -} - -// Capability represents a server capability -type Capability struct { - Name string - Values []string +type GitUploadPackInfo struct { + Capabilities *packp.Capabilities + Refs memory.ReferenceStorage } -// NewCapabilities returns a new Capabilities struct -func NewCapabilities() *Capabilities { - return &Capabilities{ - m: make(map[string]*Capability, 0), - } +func NewGitUploadPackInfo() *GitUploadPackInfo { + return &GitUploadPackInfo{Capabilities: packp.NewCapabilities()} } -// Decode decodes a string -func (c *Capabilities) Decode(raw string) { - params := strings.Split(raw, " ") - for _, p := range params { - s := strings.SplitN(p, "=", 2) - - var value string - if len(s) == 2 { - value = s[1] +func (i *GitUploadPackInfo) Decode(r io.Reader) error { + d := advrefs.NewDecoder(r) + ar := advrefs.New() + if err := d.Decode(ar); err != nil { + if err == advrefs.ErrEmpty { + return core.NewPermanentError(err) } - - c.Add(s[0], value) - } -} - -// Get returns the values for a capability -func (c *Capabilities) Get(capability string) *Capability { - return c.m[capability] -} - -// Set sets a capability removing the values -func (c *Capabilities) Set(capability string, values ...string) { - if _, ok := c.m[capability]; ok { - delete(c.m, capability) + return core.NewUnexpectedError(err) } - c.Add(capability, values...) -} - -// Add adds a capability, values are optional -func (c *Capabilities) Add(capability string, values ...string) { - if !c.Supports(capability) { - c.m[capability] = &Capability{Name: capability} - c.o = append(c.o, capability) - } + i.Capabilities = ar.Capabilities - if len(values) == 0 { - return + if err := i.addRefs(ar); err != nil { + return core.NewUnexpectedError(err) } - c.m[capability].Values = append(c.m[capability].Values, values...) -} - -// Supports returns true if capability is present -func (c *Capabilities) Supports(capability string) bool { - _, ok := c.m[capability] - return ok + return nil } -// SymbolicReference returns the reference for a given symbolic reference -func (c *Capabilities) SymbolicReference(sym string) string { - if !c.Supports("symref") { - return "" - } - - for _, symref := range c.Get("symref").Values { - parts := strings.Split(symref, ":") - if len(parts) != 2 { - continue - } - - if parts[0] == sym { - return parts[1] - } +func (i *GitUploadPackInfo) addRefs(ar *advrefs.AdvRefs) error { + i.Refs = make(memory.ReferenceStorage, 0) + for name, hash := range ar.References { + ref := core.NewReferenceFromStrings(name, hash.String()) + i.Refs.Set(ref) } - return "" + return i.addSymbolicRefs(ar) } -func (c *Capabilities) String() string { - if len(c.o) == 0 { - return "" - } - - var o string - for _, key := range c.o { - cap := c.m[key] - - added := false - for _, value := range cap.Values { - if value == "" { - continue - } - - added = true - o += fmt.Sprintf("%s=%s ", key, value) - } - - if len(cap.Values) == 0 || !added { - o += key + " " - } - } - - if len(o) == 0 { - return o +func (i *GitUploadPackInfo) addSymbolicRefs(ar *advrefs.AdvRefs) error { + if !hasSymrefs(ar) { + return nil } - return o[:len(o)-1] -} - -type GitUploadPackInfo struct { - Capabilities *Capabilities - Refs memory.ReferenceStorage -} - -func NewGitUploadPackInfo() *GitUploadPackInfo { - return &GitUploadPackInfo{Capabilities: NewCapabilities()} -} - -func (r *GitUploadPackInfo) Decode(s *pktline.Scanner) error { - if err := r.read(s); err != nil { - if err == ErrEmptyGitUploadPack { - return core.NewPermanentError(err) + for _, symref := range ar.Capabilities.Get("symref").Values { + chunks := strings.Split(symref, ":") + if len(chunks) != 2 { + err := fmt.Errorf("bad number of `:` in symref value (%q)", symref) + return core.NewUnexpectedError(err) } - - return core.NewUnexpectedError(err) + name := core.ReferenceName(chunks[0]) + target := core.ReferenceName(chunks[1]) + ref := core.NewSymbolicReference(name, target) + i.Refs.Set(ref) } return nil } -func (r *GitUploadPackInfo) read(s *pktline.Scanner) error { - isEmpty := true - r.Refs = make(memory.ReferenceStorage, 0) - smartCommentIgnore := false - for s.Scan() { - line := string(s.Bytes()) - - if smartCommentIgnore { - // some servers like Github add a flush-pkt after the smart http comment - // that we must ignore to prevent a premature termination of the read. - if len(line) == 0 { - continue - } - smartCommentIgnore = false - } - - // exit on first flush-pkt - if len(line) == 0 { - break - } - - if isSmartHttpComment(line) { - smartCommentIgnore = true - continue - } - - if err := r.readLine(line); err != nil { - return err - } - - isEmpty = false - } - - if isEmpty { - return ErrEmptyGitUploadPack - } - - return s.Err() +func hasSymrefs(ar *advrefs.AdvRefs) bool { + return ar.Capabilities.Supports("symref") } -func isSmartHttpComment(line string) bool { - return line[0] == '#' +func (i *GitUploadPackInfo) Head() *core.Reference { + ref, _ := core.ResolveReference(i.Refs, core.HEAD) + return ref } -func (r *GitUploadPackInfo) readLine(line string) error { - hashEnd := strings.Index(line, " ") - hash := line[:hashEnd] - - zeroID := strings.Index(line, string([]byte{0})) - if zeroID == -1 { - name := line[hashEnd+1 : len(line)-1] - ref := core.NewReferenceFromStrings(name, hash) - return r.Refs.Set(ref) - } - - name := line[hashEnd+1 : zeroID] - r.Capabilities.Decode(line[zeroID+1 : len(line)-1]) - if !r.Capabilities.Supports("symref") { - ref := core.NewReferenceFromStrings(name, hash) - return r.Refs.Set(ref) - } - - target := r.Capabilities.SymbolicReference(name) - ref := core.NewSymbolicReference(core.ReferenceName(name), core.ReferenceName(target)) - return r.Refs.Set(ref) +func (i *GitUploadPackInfo) String() string { + return string(i.Bytes()) } -func (r *GitUploadPackInfo) Head() *core.Reference { - ref, _ := core.ResolveReference(r.Refs, core.HEAD) - return ref -} +func (i *GitUploadPackInfo) Bytes() []byte { + var buf bytes.Buffer + e := pktline.NewEncoder(&buf) -func (r *GitUploadPackInfo) String() string { - return string(r.Bytes()) -} + _ = e.EncodeString("# service=git-upload-pack\n") -func (r *GitUploadPackInfo) Bytes() []byte { - p := pktline.New() - _ = p.AddString("# service=git-upload-pack\n") // inserting a flush-pkt here violates the protocol spec, but some // servers do it, like Github.com - p.AddFlush() + e.Flush() - firstLine := fmt.Sprintf("%s HEAD\x00%s\n", r.Head().Hash(), r.Capabilities.String()) - _ = p.AddString(firstLine) + _ = e.Encodef("%s HEAD\x00%s\n", i.Head().Hash(), i.Capabilities.String()) - for _, ref := range r.Refs { + for _, ref := range i.Refs { if ref.Type() != core.HashReference { continue } - ref := fmt.Sprintf("%s %s\n", ref.Hash(), ref.Name()) - _ = p.AddString(ref) + _ = e.Encodef("%s %s\n", ref.Hash(), ref.Name()) } - p.AddFlush() - b, _ := ioutil.ReadAll(p) + e.Flush() - return b + return buf.Bytes() } type GitUploadPackRequest struct { @@ -337,24 +194,23 @@ func (r *GitUploadPackRequest) String() string { } func (r *GitUploadPackRequest) Reader() *strings.Reader { - p := pktline.New() + var buf bytes.Buffer + e := pktline.NewEncoder(&buf) for _, want := range r.Wants { - _ = p.AddString(fmt.Sprintf("want %s\n", want)) + _ = e.Encodef("want %s\n", want) } for _, have := range r.Haves { - _ = p.AddString(fmt.Sprintf("have %s\n", have)) + _ = e.Encodef("have %s\n", have) } if r.Depth != 0 { - _ = p.AddString(fmt.Sprintf("deepen %d\n", r.Depth)) + _ = e.Encodef("deepen %d\n", r.Depth) } - p.AddFlush() - _ = p.AddString("done\n") - - b, _ := ioutil.ReadAll(p) + _ = e.Flush() + _ = e.EncodeString("done\n") - return strings.NewReader(string(b)) + return strings.NewReader(buf.String()) } diff --git a/clients/common/common_test.go b/clients/common/common_test.go index 66a49e1..5809584 100644 --- a/clients/common/common_test.go +++ b/clients/common/common_test.go @@ -6,7 +6,7 @@ import ( "testing" "gopkg.in/src-d/go-git.v4/core" - "gopkg.in/src-d/go-git.v4/formats/packp/pktline" + "gopkg.in/src-d/go-git.v4/formats/packp" . "gopkg.in/check.v1" ) @@ -38,7 +38,7 @@ func (s *SuiteCommon) TestNewEndpointWrongForgat(c *C) { const CapabilitiesFixture = "6ecf0ef2c2dffb796033e5a02219af86ec6584e5 HEADmulti_ack thin-pack side-band side-band-64k ofs-delta shallow no-progress include-tag multi_ack_detailed no-done symref=HEAD:refs/heads/master agent=git/2:2.4.8~dbussink-fix-enterprise-tokens-compilation-1167-gc7006cf" func (s *SuiteCommon) TestCapabilitiesSymbolicReference(c *C) { - cap := NewCapabilities() + cap := packp.NewCapabilities() cap.Decode(CapabilitiesFixture) c.Assert(cap.SymbolicReference("HEAD"), Equals, "refs/heads/master") } @@ -49,7 +49,7 @@ func (s *SuiteCommon) TestGitUploadPackInfo(c *C) { b, _ := base64.StdEncoding.DecodeString(GitUploadPackInfoFixture) i := NewGitUploadPackInfo() - err := i.Decode(pktline.NewScanner(bytes.NewBuffer(b))) + err := i.Decode(bytes.NewBuffer(b)) c.Assert(err, IsNil) name := i.Capabilities.SymbolicReference("HEAD") @@ -71,7 +71,7 @@ func (s *SuiteCommon) TestGitUploadPackInfoNoHEAD(c *C) { b, _ := base64.StdEncoding.DecodeString(GitUploadPackInfoNoHEADFixture) i := NewGitUploadPackInfo() - err := i.Decode(pktline.NewScanner(bytes.NewBuffer(b))) + err := i.Decode(bytes.NewBuffer(b)) c.Assert(err, IsNil) name := i.Capabilities.SymbolicReference("HEAD") @@ -87,43 +87,10 @@ func (s *SuiteCommon) TestGitUploadPackInfoEmpty(c *C) { b := bytes.NewBuffer(nil) i := NewGitUploadPackInfo() - err := i.Decode(pktline.NewScanner(b)) + err := i.Decode(b) c.Assert(err, ErrorMatches, "permanent.*empty.*") } -func (s *SuiteCommon) TestCapabilitiesDecode(c *C) { - cap := NewCapabilities() - cap.Decode("symref=foo symref=qux thin-pack") - - c.Assert(cap.m, HasLen, 2) - c.Assert(cap.Get("symref").Values, DeepEquals, []string{"foo", "qux"}) - c.Assert(cap.Get("thin-pack").Values, DeepEquals, []string{""}) -} - -func (s *SuiteCommon) TestCapabilitiesSet(c *C) { - cap := NewCapabilities() - cap.Add("symref", "foo", "qux") - cap.Set("symref", "bar") - - c.Assert(cap.m, HasLen, 1) - c.Assert(cap.Get("symref").Values, DeepEquals, []string{"bar"}) -} - -func (s *SuiteCommon) TestCapabilitiesSetEmpty(c *C) { - cap := NewCapabilities() - cap.Set("foo", "bar") - - c.Assert(cap.Get("foo").Values, HasLen, 1) -} - -func (s *SuiteCommon) TestCapabilitiesAdd(c *C) { - cap := NewCapabilities() - cap.Add("symref", "foo", "qux") - cap.Add("thin-pack") - - c.Assert(cap.String(), Equals, "symref=foo symref=qux thin-pack") -} - func (s *SuiteCommon) TestGitUploadPackEncode(c *C) { info := NewGitUploadPackInfo() info.Capabilities.Add("symref", "HEAD:refs/heads/master") diff --git a/clients/http/git_upload_pack.go b/clients/http/git_upload_pack.go index eb8db0b..f652150 100644 --- a/clients/http/git_upload_pack.go +++ b/clients/http/git_upload_pack.go @@ -78,7 +78,7 @@ func (s *GitUploadPackService) Info() (*common.GitUploadPackInfo, error) { defer res.Body.Close() i := common.NewGitUploadPackInfo() - return i, i.Decode(pktline.NewScanner(res.Body)) + return i, i.Decode(res.Body) } // Fetch request and returns a reader to a packfile diff --git a/clients/ssh/git_upload_pack.go b/clients/ssh/git_upload_pack.go index 513e528..3bdb64f 100644 --- a/clients/ssh/git_upload_pack.go +++ b/clients/ssh/git_upload_pack.go @@ -11,7 +11,6 @@ import ( "strings" "gopkg.in/src-d/go-git.v4/clients/common" - "gopkg.in/src-d/go-git.v4/formats/packp/pktline" "golang.org/x/crypto/ssh" ) @@ -123,7 +122,7 @@ func (s *GitUploadPackService) Info() (i *common.GitUploadPackInfo, err error) { } i = common.NewGitUploadPackInfo() - return i, i.Decode(pktline.NewScanner(bytes.NewReader(out))) + return i, i.Decode(bytes.NewReader(out)) } // Disconnect the SSH client. -- cgit