From c15bf1dff332873644290db0e186b8f5ad9b8fb2 Mon Sep 17 00:00:00 2001 From: Máximo Cuadros Date: Thu, 1 Dec 2016 09:59:19 +0100 Subject: capabilities: full integration (#151) * format/pktline: fix readPayloadLen err handling * protocol/pakp: UploadReq validation and creation of capabilities * protocol/pakp: AdvRef tests * protocol/pakp: capability.List.Delete * protocol: filter unsupported capabilities * remote capability negociation * transport: UploadRequest validation * requested changes --- plumbing/transport/common.go | 20 ++++++++++++++ plumbing/transport/common_test.go | 10 +++++++ plumbing/transport/http/fetch_pack.go | 5 ++++ plumbing/transport/http/fetch_pack_test.go | 6 ++--- plumbing/transport/internal/common/common.go | 5 ++++ plumbing/transport/test/common.go | 40 ++++++++++++++++++++++------ 6 files changed, 75 insertions(+), 11 deletions(-) (limited to 'plumbing/transport') diff --git a/plumbing/transport/common.go b/plumbing/transport/common.go index 9715a39..bfc999f 100644 --- a/plumbing/transport/common.go +++ b/plumbing/transport/common.go @@ -21,6 +21,7 @@ import ( "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/protocol/packp" + "gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/capability" ) var ( @@ -124,3 +125,22 @@ func transformSCPLikeIfNeeded(endpoint string) string { return endpoint } + +// UnsupportedCapabilities are the capabilities not supported by any client +// implementation +var UnsupportedCapabilities = []capability.Capability{ + capability.MultiACK, + capability.MultiACKDetailed, + capability.Sideband, + capability.Sideband64k, + capability.ThinPack, +} + +// FilterUnsupportedCapabilities it filter out all the UnsupportedCapabilities +// from a capability.List, the intended usage is on the client implementation +// to filter the capabilities from an AdvRefs message. +func FilterUnsupportedCapabilities(list *capability.List) { + for _, c := range UnsupportedCapabilities { + list.Delete(c) + } +} diff --git a/plumbing/transport/common_test.go b/plumbing/transport/common_test.go index 9ca4459..e9a5efa 100644 --- a/plumbing/transport/common_test.go +++ b/plumbing/transport/common_test.go @@ -3,6 +3,8 @@ package transport import ( "testing" + "gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/capability" + . "gopkg.in/check.v1" ) @@ -29,3 +31,11 @@ func (s *SuiteCommon) TestNewEndpointWrongForgat(c *C) { c.Assert(err, Not(IsNil)) c.Assert(e.Host, Equals, "") } + +func (s *SuiteCommon) TestFilterUnsupportedCapabilities(c *C) { + l := capability.NewList() + l.Set(capability.MultiACK) + + FilterUnsupportedCapabilities(l) + c.Assert(l.Supports(capability.MultiACK), Equals, false) +} diff --git a/plumbing/transport/http/fetch_pack.go b/plumbing/transport/http/fetch_pack.go index 40c3a7f..f250667 100644 --- a/plumbing/transport/http/fetch_pack.go +++ b/plumbing/transport/http/fetch_pack.go @@ -70,6 +70,7 @@ func (s *fetchPackSession) AdvertisedReferences() (*packp.AdvRefs, error) { return nil, err } + transport.FilterUnsupportedCapabilities(ar.Capabilities) return ar, nil } @@ -78,6 +79,10 @@ func (s *fetchPackSession) FetchPack(r *packp.UploadPackRequest) (io.ReadCloser, return nil, transport.ErrEmptyUploadPackRequest } + if err := r.Validate(); err != nil { + return nil, err + } + url := fmt.Sprintf( "%s/%s", s.endpoint.String(), transport.UploadPackServiceName, diff --git a/plumbing/transport/http/fetch_pack_test.go b/plumbing/transport/http/fetch_pack_test.go index 7471208..920b623 100644 --- a/plumbing/transport/http/fetch_pack_test.go +++ b/plumbing/transport/http/fetch_pack_test.go @@ -43,9 +43,9 @@ func (s *FetchPackSuite) TestInfoNotExists(c *C) { func (s *FetchPackSuite) TestuploadPackRequestToReader(c *C) { r := packp.NewUploadPackRequest() - r.Want(plumbing.NewHash("d82f291cde9987322c8a0c81a325e1ba6159684c")) - r.Want(plumbing.NewHash("2b41ef280fdb67a9b250678686a0c3e03b0a9989")) - r.Have(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + r.Wants = append(r.Wants, plumbing.NewHash("d82f291cde9987322c8a0c81a325e1ba6159684c")) + r.Wants = append(r.Wants, plumbing.NewHash("2b41ef280fdb67a9b250678686a0c3e03b0a9989")) + r.Haves = append(r.Haves, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) sr, err := uploadPackRequestToReader(r) c.Assert(err, IsNil) diff --git a/plumbing/transport/internal/common/common.go b/plumbing/transport/internal/common/common.go index 10e395e..56edab0 100644 --- a/plumbing/transport/internal/common/common.go +++ b/plumbing/transport/internal/common/common.go @@ -162,6 +162,7 @@ func (s *session) AdvertisedReferences() (*packp.AdvRefs, error) { return nil, err } + transport.FilterUnsupportedCapabilities(ar.Capabilities) return ar, nil } @@ -172,6 +173,10 @@ func (s *session) FetchPack(req *packp.UploadPackRequest) (io.ReadCloser, error) return nil, transport.ErrEmptyUploadPackRequest } + if err := req.Validate(); err != nil { + return nil, err + } + if !s.advRefsRun { if _, err := s.AdvertisedReferences(); err != nil { return nil, err diff --git a/plumbing/transport/test/common.go b/plumbing/transport/test/common.go index ed2e141..3b7f05f 100644 --- a/plumbing/transport/test/common.go +++ b/plumbing/transport/test/common.go @@ -43,7 +43,7 @@ func (s *FetchPackSuite) TestInfoNotExists(c *C) { r, err = s.Client.NewFetchPackSession(s.NonExistentEndpoint) c.Assert(err, IsNil) req := packp.NewUploadPackRequest() - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Wants = append(req.Wants, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) reader, err := r.FetchPack(req) c.Assert(err, Equals, transport.ErrRepositoryNotFound) c.Assert(reader, IsNil) @@ -70,6 +70,16 @@ func (s *FetchPackSuite) TestDefaultBranch(c *C) { c.Assert(symrefs[0], Equals, "HEAD:refs/heads/master") } +func (s *FetchPackSuite) TestAdvertisedReferencesFilterUnsupported(c *C) { + r, err := s.Client.NewFetchPackSession(s.Endpoint) + c.Assert(err, IsNil) + defer func() { c.Assert(r.Close(), IsNil) }() + + info, err := r.AdvertisedReferences() + c.Assert(err, IsNil) + c.Assert(info.Capabilities.Supports(capability.MultiACK), Equals, false) +} + func (s *FetchPackSuite) TestCapabilities(c *C) { r, err := s.Client.NewFetchPackSession(s.Endpoint) c.Assert(err, IsNil) @@ -90,7 +100,7 @@ func (s *FetchPackSuite) TestFullFetchPack(c *C) { c.Assert(info, NotNil) req := packp.NewUploadPackRequest() - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Wants = append(req.Wants, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) reader, err := r.FetchPack(req) c.Assert(err, IsNil) @@ -104,7 +114,7 @@ func (s *FetchPackSuite) TestFetchPack(c *C) { defer func() { c.Assert(r.Close(), IsNil) }() req := packp.NewUploadPackRequest() - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Wants = append(req.Wants, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) reader, err := r.FetchPack(req) c.Assert(err, IsNil) @@ -112,14 +122,28 @@ func (s *FetchPackSuite) TestFetchPack(c *C) { s.checkObjectNumber(c, reader, 28) } +func (s *FetchPackSuite) TestFetchPackInvalidReq(c *C) { + r, err := s.Client.NewFetchPackSession(s.Endpoint) + c.Assert(err, IsNil) + defer func() { c.Assert(r.Close(), IsNil) }() + + req := packp.NewUploadPackRequest() + req.Wants = append(req.Wants, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Capabilities.Set(capability.Sideband) + req.Capabilities.Set(capability.Sideband64k) + + _, err = r.FetchPack(req) + c.Assert(err, NotNil) +} + func (s *FetchPackSuite) TestFetchPackNoChanges(c *C) { r, err := s.Client.NewFetchPackSession(s.Endpoint) c.Assert(err, IsNil) defer func() { c.Assert(r.Close(), IsNil) }() req := packp.NewUploadPackRequest() - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) - req.Have(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Wants = append(req.Wants, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Haves = append(req.Haves, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) reader, err := r.FetchPack(req) c.Assert(err, Equals, transport.ErrEmptyUploadPackRequest) @@ -132,8 +156,8 @@ func (s *FetchPackSuite) TestFetchPackMulti(c *C) { defer func() { c.Assert(r.Close(), IsNil) }() req := packp.NewUploadPackRequest() - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) - req.Want(plumbing.NewHash("e8d3ffab552895c19b9fcf7aa264d277cde33881")) + req.Wants = append(req.Wants, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Wants = append(req.Wants, plumbing.NewHash("e8d3ffab552895c19b9fcf7aa264d277cde33881")) reader, err := r.FetchPack(req) c.Assert(err, IsNil) @@ -146,7 +170,7 @@ func (s *FetchPackSuite) TestFetchError(c *C) { c.Assert(err, IsNil) req := packp.NewUploadPackRequest() - req.Want(plumbing.NewHash("1111111111111111111111111111111111111111")) + req.Wants = append(req.Wants, plumbing.NewHash("1111111111111111111111111111111111111111")) reader, err := r.FetchPack(req) c.Assert(err, Equals, transport.ErrEmptyUploadPackRequest) -- cgit