From 25b0970bc9a0edd9fdb3b62230844b81202f4915 Mon Sep 17 00:00:00 2001 From: Antonio Jesus Navarro Perez Date: Tue, 24 Jan 2017 09:26:55 +0100 Subject: [PATCH 1/2] transport/http: fix partial request with haves (Fix #216) Fix #216 Using the http transport, now if we request a partial packfile, (using haves that aren't head ones) the response is not an empty packfile. Skip this test in server implementation. Revlist implementation is returning more objects than expected. --- plumbing/protocol/packp/uppackreq.go | 4 ++-- plumbing/protocol/packp/uppackreq_test.go | 2 +- plumbing/transport/http/upload_pack.go | 7 +++++-- plumbing/transport/http/upload_pack_test.go | 2 +- plumbing/transport/internal/common/common.go | 2 +- plumbing/transport/server/upload_pack_test.go | 5 +++++ plumbing/transport/test/upload_pack.go | 15 +++++++++++++++ 7 files changed, 30 insertions(+), 7 deletions(-) diff --git a/plumbing/protocol/packp/uppackreq.go b/plumbing/protocol/packp/uppackreq.go index 84e2b4e01..eb14bfc25 100644 --- a/plumbing/protocol/packp/uppackreq.go +++ b/plumbing/protocol/packp/uppackreq.go @@ -69,7 +69,7 @@ type UploadHaves struct { } // Encode encodes the UploadHaves into the Writer. -func (u *UploadHaves) Encode(w io.Writer) error { +func (u *UploadHaves) Encode(w io.Writer, flush bool) error { e := pktline.NewEncoder(w) plumbing.HashesSort(u.Haves) @@ -87,7 +87,7 @@ func (u *UploadHaves) Encode(w io.Writer) error { last = have } - if len(u.Haves) != 0 { + if flush && len(u.Haves) != 0 { if err := e.Flush(); err != nil { return fmt.Errorf("sending flush-pkt after haves: %s", err) } diff --git a/plumbing/protocol/packp/uppackreq_test.go b/plumbing/protocol/packp/uppackreq_test.go index 273f91612..f776c0757 100644 --- a/plumbing/protocol/packp/uppackreq_test.go +++ b/plumbing/protocol/packp/uppackreq_test.go @@ -58,7 +58,7 @@ func (s *UploadHavesSuite) TestEncode(c *C) { ) buf := bytes.NewBuffer(nil) - err := uh.Encode(buf) + err := uh.Encode(buf, true) c.Assert(err, IsNil) c.Assert(buf.String(), Equals, ""+ "0032have 1111111111111111111111111111111111111111\n"+ diff --git a/plumbing/transport/http/upload_pack.go b/plumbing/transport/http/upload_pack.go index e828857d4..fd1787c95 100644 --- a/plumbing/transport/http/upload_pack.go +++ b/plumbing/transport/http/upload_pack.go @@ -171,10 +171,13 @@ func uploadPackRequestToReader(req *packp.UploadPackRequest) (*bytes.Buffer, err return nil, fmt.Errorf("sending upload-req message: %s", err) } - if err := req.UploadHaves.Encode(buf); err != nil { + if err := req.UploadHaves.Encode(buf, false); err != nil { return nil, fmt.Errorf("sending haves message: %s", err) } - _ = e.EncodeString("done\n") + if err := e.EncodeString("done\n"); err != nil { + return nil, err + } + return buf, nil } diff --git a/plumbing/transport/http/upload_pack_test.go b/plumbing/transport/http/upload_pack_test.go index 13b7f75ff..a793efb9e 100644 --- a/plumbing/transport/http/upload_pack_test.go +++ b/plumbing/transport/http/upload_pack_test.go @@ -54,7 +54,7 @@ func (s *UploadPackSuite) TestuploadPackRequestToReader(c *C) { c.Assert(string(b), Equals, "0032want 2b41ef280fdb67a9b250678686a0c3e03b0a9989\n"+ "0032want d82f291cde9987322c8a0c81a325e1ba6159684c\n0000"+ - "0032have 6ecf0ef2c2dffb796033e5a02219af86ec6584e5\n0000"+ + "0032have 6ecf0ef2c2dffb796033e5a02219af86ec6584e5\n"+ "0009done\n", ) } diff --git a/plumbing/transport/internal/common/common.go b/plumbing/transport/internal/common/common.go index 2285e2684..b45919858 100644 --- a/plumbing/transport/internal/common/common.go +++ b/plumbing/transport/internal/common/common.go @@ -371,7 +371,7 @@ func uploadPack(w io.WriteCloser, r io.Reader, req *packp.UploadPackRequest) err return fmt.Errorf("sending upload-req message: %s", err) } - if err := req.UploadHaves.Encode(w); err != nil { + if err := req.UploadHaves.Encode(w, true); err != nil { return fmt.Errorf("sending haves message: %s", err) } diff --git a/plumbing/transport/server/upload_pack_test.go b/plumbing/transport/server/upload_pack_test.go index 137f88785..7ba1e7436 100644 --- a/plumbing/transport/server/upload_pack_test.go +++ b/plumbing/transport/server/upload_pack_test.go @@ -38,3 +38,8 @@ func (s *UploadPackSuite) TestAdvertisedReferencesNotExists(c *C) { c.Assert(err, Equals, transport.ErrRepositoryNotFound) c.Assert(r, IsNil) } + +// TODO revList implementation is returning more objects than expected. +func (s *UploadPackSuite) TestUploadPackPartial(c *C) { + c.Skip("Fix revList implementation") +} diff --git a/plumbing/transport/test/upload_pack.go b/plumbing/transport/test/upload_pack.go index c1a9050a0..06ae893d7 100644 --- a/plumbing/transport/test/upload_pack.go +++ b/plumbing/transport/test/upload_pack.go @@ -168,6 +168,21 @@ func (s *UploadPackSuite) TestUploadPackMulti(c *C) { s.checkObjectNumber(c, reader, 31) } +func (s *UploadPackSuite) TestUploadPackPartial(c *C) { + r, err := s.Client.NewUploadPackSession(s.Endpoint, s.EmptyAuth) + c.Assert(err, IsNil) + defer func() { c.Assert(r.Close(), IsNil) }() + + req := packp.NewUploadPackRequest() + req.Wants = append(req.Wants, plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Haves = append(req.Haves, plumbing.NewHash("918c48b83bd081e863dbe1b80f8998f058cd8294")) + + reader, err := r.UploadPack(req) + c.Assert(err, IsNil) + + s.checkObjectNumber(c, reader, 4) +} + func (s *UploadPackSuite) TestFetchError(c *C) { r, err := s.Client.NewUploadPackSession(s.Endpoint, s.EmptyAuth) c.Assert(err, IsNil) From b6423ea6a04b21cc8941a7bd1adc4b9ab484c8ca Mon Sep 17 00:00:00 2001 From: Antonio Jesus Navarro Perez Date: Tue, 24 Jan 2017 15:27:06 +0100 Subject: [PATCH 2/2] Requested changes --- plumbing/protocol/packp/uppackreq.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plumbing/protocol/packp/uppackreq.go b/plumbing/protocol/packp/uppackreq.go index eb14bfc25..4bb22d0a0 100644 --- a/plumbing/protocol/packp/uppackreq.go +++ b/plumbing/protocol/packp/uppackreq.go @@ -68,7 +68,8 @@ type UploadHaves struct { Haves []plumbing.Hash } -// Encode encodes the UploadHaves into the Writer. +// Encode encodes the UploadHaves into the Writer. If flush is true, a flush +// command will be encoded at the end of the writer content. func (u *UploadHaves) Encode(w io.Writer, flush bool) error { e := pktline.NewEncoder(w)