diff options
author | Santiago M. Mola <santi@mola.io> | 2016-11-25 09:25:49 +0100 |
---|---|---|
committer | Máximo Cuadros <mcuadros@gmail.com> | 2016-11-25 09:25:49 +0100 |
commit | 9e34f68d980de57631c588aaa910c9ea95ed7c2e (patch) | |
tree | b1bd9f867b757ca46ada2f349d122723dde3529c /plumbing | |
parent | 8966c042795509ed17730e50d352ad69901c3da8 (diff) | |
download | go-git-9e34f68d980de57631c588aaa910c9ea95ed7c2e.tar.gz |
plumbing/transport: add common tests and fixes. (#136)
* plumbing/transport: add common tests and fixes.
* add common test suite for different transport implementations.
* fix different behaviour on error handling for ssh and http.
fixes issue #123.
* support detecting unexisting repositories with SSH + GitHub/Bitbucket
(apparently, there is no standard for all SSH servers).
* remove ssh.NewClient (only DefaultClient makes sense at the moment).
* make ssh.Client and http.Client private.
* utils/ioutil: utilities to work with io interfaces.
* * transport: test actual objects fetched, not just packfile size.
* * fix doc typo.
* * improve UploadPackRequest.IsEmpty
Diffstat (limited to 'plumbing')
-rw-r--r-- | plumbing/format/packp/advrefs/decoder.go | 12 | ||||
-rw-r--r-- | plumbing/format/packp/advrefs/decoder_test.go | 26 | ||||
-rw-r--r-- | plumbing/transport/client/client_test.go | 6 | ||||
-rw-r--r-- | plumbing/transport/common.go | 3 | ||||
-rw-r--r-- | plumbing/transport/fetch_pack.go | 24 | ||||
-rw-r--r-- | plumbing/transport/fetch_pack_test.go | 25 | ||||
-rw-r--r-- | plumbing/transport/http/common.go | 15 | ||||
-rw-r--r-- | plumbing/transport/http/common_test.go | 19 | ||||
-rw-r--r-- | plumbing/transport/http/fetch_pack.go | 52 | ||||
-rw-r--r-- | plumbing/transport/http/fetch_pack_test.go | 110 | ||||
-rw-r--r-- | plumbing/transport/ssh/common.go | 52 | ||||
-rw-r--r-- | plumbing/transport/ssh/common_test.go | 8 | ||||
-rw-r--r-- | plumbing/transport/ssh/fetch_pack.go | 45 | ||||
-rw-r--r-- | plumbing/transport/ssh/fetch_pack_test.go | 82 | ||||
-rw-r--r-- | plumbing/transport/test/common.go | 166 |
15 files changed, 405 insertions, 240 deletions
diff --git a/plumbing/format/packp/advrefs/decoder.go b/plumbing/format/packp/advrefs/decoder.go index b654882..c50eeef 100644 --- a/plumbing/format/packp/advrefs/decoder.go +++ b/plumbing/format/packp/advrefs/decoder.go @@ -86,6 +86,12 @@ func decodePrefix(d *Decoder) decoderStateFn { return nil } + // If the repository is empty, we receive a flush here (SSH). + if isFlush(d.line) { + d.err = ErrEmpty + return nil + } + if isPrefix(d.line) { tmp := make([]byte, len(d.line)) copy(tmp, d.line) @@ -117,6 +123,12 @@ func isFlush(payload []byte) bool { // list-of-refs is comming, and the hash will be followed by the first // advertised ref. func decodeFirstHash(p *Decoder) decoderStateFn { + // If the repository is empty, we receive a flush here (HTTP). + if isFlush(p.line) { + p.err = ErrEmpty + return nil + } + if len(p.line) < hashSize { p.error("cannot read hash, pkt-line too short") return nil diff --git a/plumbing/format/packp/advrefs/decoder_test.go b/plumbing/format/packp/advrefs/decoder_test.go index 03867d3..bacf79a 100644 --- a/plumbing/format/packp/advrefs/decoder_test.go +++ b/plumbing/format/packp/advrefs/decoder_test.go @@ -26,6 +26,32 @@ func (s *SuiteDecoder) TestEmpty(c *C) { c.Assert(err, Equals, advrefs.ErrEmpty) } +func (s *SuiteDecoder) TestEmptyFlush(c *C) { + ar := advrefs.New() + var buf bytes.Buffer + e := pktline.NewEncoder(&buf) + e.Flush() + + d := advrefs.NewDecoder(&buf) + + err := d.Decode(ar) + c.Assert(err, Equals, advrefs.ErrEmpty) +} + +func (s *SuiteDecoder) TestEmptyPrefixFlush(c *C) { + ar := advrefs.New() + var buf bytes.Buffer + e := pktline.NewEncoder(&buf) + e.EncodeString("# service=git-upload-pack") + e.Flush() + e.Flush() + + d := advrefs.NewDecoder(&buf) + + err := d.Decode(ar) + c.Assert(err, Equals, advrefs.ErrEmpty) +} + func (s *SuiteDecoder) TestShortForHash(c *C) { payloads := []string{ "6ecf0ef2c2dffb796", diff --git a/plumbing/transport/client/client_test.go b/plumbing/transport/client/client_test.go index 90bad57..0f4b2e5 100644 --- a/plumbing/transport/client/client_test.go +++ b/plumbing/transport/client/client_test.go @@ -22,14 +22,14 @@ func (s *ClientSuite) TestNewClientHTTP(c *C) { output, err := NewClient(e) c.Assert(err, IsNil) - c.Assert(typeAsString(output), Equals, "*http.Client") + c.Assert(typeAsString(output), Equals, "*http.client") e, err = transport.NewEndpoint("https://github.com/src-d/go-git") c.Assert(err, IsNil) output, err = NewClient(e) c.Assert(err, IsNil) - c.Assert(typeAsString(output), Equals, "*http.Client") + c.Assert(typeAsString(output), Equals, "*http.client") } func (s *ClientSuite) TestNewClientSSH(c *C) { @@ -38,7 +38,7 @@ func (s *ClientSuite) TestNewClientSSH(c *C) { output, err := NewClient(e) c.Assert(err, IsNil) - c.Assert(typeAsString(output), Equals, "*ssh.Client") + c.Assert(typeAsString(output), Equals, "*ssh.client") } func (s *ClientSuite) TestNewClientUnknown(c *C) { diff --git a/plumbing/transport/common.go b/plumbing/transport/common.go index cc30564..8329069 100644 --- a/plumbing/transport/common.go +++ b/plumbing/transport/common.go @@ -24,9 +24,12 @@ import ( var ( ErrRepositoryNotFound = errors.New("repository not found") + ErrEmptyRemoteRepository = errors.New("remote repository is empty") ErrAuthorizationRequired = errors.New("authorization required") ErrEmptyUploadPackRequest = errors.New("empty git-upload-pack given") ErrInvalidAuthMethod = errors.New("invalid auth method") + + ErrAdvertistedReferencesAlreadyCalled = errors.New("cannot call AdvertisedReference twice") ) const ( diff --git a/plumbing/transport/fetch_pack.go b/plumbing/transport/fetch_pack.go index 3b2a39c..5346e9d 100644 --- a/plumbing/transport/fetch_pack.go +++ b/plumbing/transport/fetch_pack.go @@ -33,7 +33,7 @@ func (i *UploadPackInfo) Decode(r io.Reader) error { ar := advrefs.New() if err := d.Decode(ar); err != nil { if err == advrefs.ErrEmpty { - return plumbing.NewPermanentError(err) + return err } return plumbing.NewUnexpectedError(err) } @@ -128,6 +128,28 @@ func (r *UploadPackRequest) Have(h ...plumbing.Hash) { r.Haves = append(r.Haves, h...) } +func (r *UploadPackRequest) IsEmpty() bool { + return isSubset(r.Wants, r.Haves) +} + +func isSubset(needle []plumbing.Hash, haystack []plumbing.Hash) bool { + for _, h := range needle { + found := false + for _, oh := range haystack { + if h == oh { + found = true + break + } + } + + if !found { + return false + } + } + + return true +} + func (r *UploadPackRequest) String() string { b, _ := ioutil.ReadAll(r.Reader()) return string(b) diff --git a/plumbing/transport/fetch_pack_test.go b/plumbing/transport/fetch_pack_test.go index 7a7f86f..16112a9 100644 --- a/plumbing/transport/fetch_pack_test.go +++ b/plumbing/transport/fetch_pack_test.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/format/packp/advrefs" . "gopkg.in/check.v1" ) @@ -58,7 +59,7 @@ func (s *UploadPackSuite) TestUploadPackInfoEmpty(c *C) { i := NewUploadPackInfo() err := i.Decode(b) - c.Assert(err, ErrorMatches, "permanent.*empty.*") + c.Assert(err, Equals, advrefs.ErrEmpty) } func (s *UploadPackSuite) TestUploadPackEncode(c *C) { @@ -94,3 +95,25 @@ func (s *UploadPackSuite) TestUploadPackRequest(c *C) { "0009done\n", ) } + +func (s *UploadPackSuite) TestUploadPackRequest_IsEmpty(c *C) { + r := &UploadPackRequest{} + r.Want(plumbing.NewHash("d82f291cde9987322c8a0c81a325e1ba6159684c")) + r.Want(plumbing.NewHash("2b41ef280fdb67a9b250678686a0c3e03b0a9989")) + r.Have(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + + c.Assert(r.IsEmpty(), Equals, false) + + r = &UploadPackRequest{} + r.Want(plumbing.NewHash("d82f291cde9987322c8a0c81a325e1ba6159684c")) + r.Want(plumbing.NewHash("2b41ef280fdb67a9b250678686a0c3e03b0a9989")) + r.Have(plumbing.NewHash("d82f291cde9987322c8a0c81a325e1ba6159684c")) + + c.Assert(r.IsEmpty(), Equals, false) + + r = &UploadPackRequest{} + r.Want(plumbing.NewHash("d82f291cde9987322c8a0c81a325e1ba6159684c")) + r.Have(plumbing.NewHash("d82f291cde9987322c8a0c81a325e1ba6159684c")) + + c.Assert(r.IsEmpty(), Equals, true) +} diff --git a/plumbing/transport/http/common.go b/plumbing/transport/http/common.go index 038c469..a1b085b 100644 --- a/plumbing/transport/http/common.go +++ b/plumbing/transport/http/common.go @@ -9,33 +9,38 @@ import ( "gopkg.in/src-d/go-git.v4/plumbing/transport" ) -type Client struct { +type client struct { c *http.Client } +// DefaultClient is the default HTTP client, which uses `http.DefaultClient`. var DefaultClient = NewClient(nil) // NewClient creates a new client with a custom net/http client. // See `InstallProtocol` to install and override default http client. // Unless a properly initialized client is given, it will fall back into // `http.DefaultClient`. +// +// Note that for HTTP client cannot distinguist between private repositories and +// unexistent repositories on GitHub. So it returns `ErrAuthorizationRequired` +// for both. func NewClient(c *http.Client) transport.Client { if c == nil { - return &Client{http.DefaultClient} + return &client{http.DefaultClient} } - return &Client{ + return &client{ c: c, } } -func (c *Client) NewFetchPackSession(ep transport.Endpoint) ( +func (c *client) NewFetchPackSession(ep transport.Endpoint) ( transport.FetchPackSession, error) { return newFetchPackSession(c.c, ep), nil } -func (c *Client) NewSendPackSession(ep transport.Endpoint) ( +func (c *client) NewSendPackSession(ep transport.Endpoint) ( transport.SendPackSession, error) { return newSendPackSession(c.c, ep), nil diff --git a/plumbing/transport/http/common_test.go b/plumbing/transport/http/common_test.go index 1d09fba..432bd07 100644 --- a/plumbing/transport/http/common_test.go +++ b/plumbing/transport/http/common_test.go @@ -20,16 +20,20 @@ var _ = Suite(&ClientSuite{}) func (s *ClientSuite) SetUpSuite(c *C) { var err error - s.Endpoint, err = transport.NewEndpoint("https://github.com/git-fixtures/basic") + s.Endpoint, err = transport.NewEndpoint( + "https://github.com/git-fixtures/basic", + ) c.Assert(err, IsNil) } func (s *FetchPackSuite) TestNewClient(c *C) { - roundTripper := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}} - client := &http.Client{Transport: roundTripper} - r := NewClient(client).(*Client) - - c.Assert(r.c, Equals, client) + roundTripper := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + cl := &http.Client{Transport: roundTripper} + r, ok := NewClient(cl).(*client) + c.Assert(ok, Equals, true) + c.Assert(r.c, Equals, cl) } func (s *ClientSuite) TestNewBasicAuth(c *C) { @@ -54,7 +58,8 @@ func (s *ClientSuite) TestNewErrNotFound(c *C) { } func (s *ClientSuite) TestNewHTTPError40x(c *C) { - s.testNewHTTPError(c, http.StatusPaymentRequired, "unexpected client error.*") + s.testNewHTTPError(c, http.StatusPaymentRequired, + "unexpected client error.*") } func (s *ClientSuite) testNewHTTPError(c *C, code int, msg string) { diff --git a/plumbing/transport/http/fetch_pack.go b/plumbing/transport/http/fetch_pack.go index 0c32672..0becb7b 100644 --- a/plumbing/transport/http/fetch_pack.go +++ b/plumbing/transport/http/fetch_pack.go @@ -1,7 +1,6 @@ package http import ( - "bufio" "bytes" "fmt" "io" @@ -9,12 +8,15 @@ import ( "strings" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/format/packp/advrefs" "gopkg.in/src-d/go-git.v4/plumbing/format/packp/pktline" "gopkg.in/src-d/go-git.v4/plumbing/transport" + "gopkg.in/src-d/go-git.v4/utils/ioutil" ) type fetchPackSession struct { *session + advRefsRun bool } func newFetchPackSession(c *http.Client, @@ -31,6 +33,11 @@ func newFetchPackSession(c *http.Client, func (s *fetchPackSession) AdvertisedReferences() (*transport.UploadPackInfo, error) { + if s.advRefsRun { + return nil, transport.ErrAdvertistedReferencesAlreadyCalled + } + + defer func() { s.advRefsRun = true }() url := fmt.Sprintf( "%s/info/refs?service=%s", @@ -50,15 +57,28 @@ func (s *fetchPackSession) AdvertisedReferences() (*transport.UploadPackInfo, } defer res.Body.Close() + if res.StatusCode == http.StatusUnauthorized { return nil, transport.ErrAuthorizationRequired } i := transport.NewUploadPackInfo() - return i, i.Decode(res.Body) + if err := i.Decode(res.Body); err != nil { + if err == advrefs.ErrEmpty { + err = transport.ErrEmptyRemoteRepository + } + + return nil, err + } + + return i, nil } func (s *fetchPackSession) FetchPack(r *transport.UploadPackRequest) (io.ReadCloser, error) { + if r.IsEmpty() { + return nil, transport.ErrEmptyUploadPackRequest + } + url := fmt.Sprintf( "%s/%s", s.endpoint.String(), transport.UploadPackServiceName, @@ -69,20 +89,21 @@ func (s *fetchPackSession) FetchPack(r *transport.UploadPackRequest) (io.ReadClo return nil, err } - reader := newBufferedReadCloser(res.Body) - if _, err := reader.Peek(1); err != nil { - if err == io.ErrUnexpectedEOF { - return nil, transport.ErrEmptyUploadPackRequest - } + reader, err := ioutil.NonEmptyReader(res.Body) + if err == ioutil.ErrEmptyReader || err == io.ErrUnexpectedEOF { + return nil, transport.ErrEmptyUploadPackRequest + } + if err != nil { return nil, err } - if err := discardResponseInfo(reader); err != nil { + rc := ioutil.NewReadCloser(reader, res.Body) + if err := discardResponseInfo(rc); err != nil { return nil, err } - return reader, nil + return rc, nil } // Close does nothing. @@ -140,16 +161,3 @@ func (s *fetchPackSession) applyHeadersToRequest(req *http.Request, content *str req.Header.Add("Content-Length", string(content.Len())) } } - -type bufferedReadCloser struct { - *bufio.Reader - closer io.Closer -} - -func newBufferedReadCloser(r io.ReadCloser) *bufferedReadCloser { - return &bufferedReadCloser{bufio.NewReader(r), r} -} - -func (r *bufferedReadCloser) Close() error { - return r.closer.Close() -} diff --git a/plumbing/transport/http/fetch_pack_test.go b/plumbing/transport/http/fetch_pack_test.go index 5ec9991..c7666c8 100644 --- a/plumbing/transport/http/fetch_pack_test.go +++ b/plumbing/transport/http/fetch_pack_test.go @@ -1,122 +1,38 @@ package http import ( - "fmt" - "io/ioutil" - - "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/transport" + "gopkg.in/src-d/go-git.v4/plumbing/transport/test" . "gopkg.in/check.v1" ) type FetchPackSuite struct { - Endpoint transport.Endpoint + test.FetchPackSuite } var _ = Suite(&FetchPackSuite{}) func (s *FetchPackSuite) SetUpSuite(c *C) { - fmt.Println("SetUpSuite\n") - var err error - s.Endpoint, err = transport.NewEndpoint("https://github.com/git-fixtures/basic") - c.Assert(err, IsNil) -} + s.FetchPackSuite.Client = DefaultClient -func (s *FetchPackSuite) TestInfoEmpty(c *C) { - endpoint, _ := transport.NewEndpoint("https://github.com/git-fixture/empty") - r, err := DefaultClient.NewFetchPackSession(endpoint) + ep, err := transport.NewEndpoint("https://github.com/git-fixtures/basic.git") c.Assert(err, IsNil) - info, err := r.AdvertisedReferences() - c.Assert(err, Equals, transport.ErrAuthorizationRequired) - c.Assert(info, IsNil) -} + s.FetchPackSuite.Endpoint = ep -//TODO: Test this error with HTTP BasicAuth too. -func (s *FetchPackSuite) TestInfoNotExists(c *C) { - endpoint, _ := transport.NewEndpoint("https://github.com/git-fixture/not-exists") - r, err := DefaultClient.NewFetchPackSession(endpoint) + ep, err = transport.NewEndpoint("https://github.com/git-fixtures/empty.git") c.Assert(err, IsNil) - info, err := r.AdvertisedReferences() - c.Assert(err, Equals, transport.ErrAuthorizationRequired) - c.Assert(info, IsNil) -} + s.FetchPackSuite.EmptyEndpoint = ep -func (s *FetchPackSuite) TestDefaultBranch(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) + ep, err = transport.NewEndpoint("https://github.com/git-fixtures/non-existent.git") c.Assert(err, IsNil) - info, err := r.AdvertisedReferences() - c.Assert(err, IsNil) - c.Assert(info.Capabilities.SymbolicReference("HEAD"), Equals, "refs/heads/master") -} - -func (s *FetchPackSuite) TestCapabilities(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) - c.Assert(err, IsNil) - info, err := r.AdvertisedReferences() - c.Assert(err, IsNil) - c.Assert(info.Capabilities.Get("agent").Values, HasLen, 1) + s.FetchPackSuite.NonExistentEndpoint = ep } -func (s *FetchPackSuite) TestFullFetchPack(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) +func (s *FetchPackSuite) TestInfoNotExists(c *C) { + r, err := s.Client.NewFetchPackSession(s.NonExistentEndpoint) c.Assert(err, IsNil) - info, err := r.AdvertisedReferences() - c.Assert(err, IsNil) - c.Assert(info, NotNil) - - req := &transport.UploadPackRequest{} - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) - - reader, err := r.FetchPack(req) - c.Assert(err, IsNil) - - b, err := ioutil.ReadAll(reader) - c.Assert(err, IsNil) - c.Assert(b, HasLen, 85374) -} - -func (s *FetchPackSuite) TestFetchPack(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) - c.Assert(err, IsNil) - - req := &transport.UploadPackRequest{} - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) - - reader, err := r.FetchPack(req) - c.Assert(err, IsNil) - - b, err := ioutil.ReadAll(reader) - c.Assert(err, IsNil) - c.Assert(b, HasLen, 85374) -} - -func (s *FetchPackSuite) TestFetchPackNoChanges(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) - c.Assert(err, IsNil) - - req := &transport.UploadPackRequest{} - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) - req.Have(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) - - reader, err := r.FetchPack(req) - c.Assert(err, Equals, transport.ErrEmptyUploadPackRequest) - c.Assert(reader, IsNil) -} - -func (s *FetchPackSuite) TestFetchPackMulti(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) - c.Assert(err, IsNil) - - req := &transport.UploadPackRequest{} - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) - req.Want(plumbing.NewHash("e8d3ffab552895c19b9fcf7aa264d277cde33881")) - - reader, err := r.FetchPack(req) - c.Assert(err, IsNil) - - b, err := ioutil.ReadAll(reader) - c.Assert(err, IsNil) - c.Assert(b, HasLen, 85585) + c.Assert(err, Equals, transport.ErrAuthorizationRequired) + c.Assert(info, IsNil) } diff --git a/plumbing/transport/ssh/common.go b/plumbing/transport/ssh/common.go index 6f0f3d4..c327c41 100644 --- a/plumbing/transport/ssh/common.go +++ b/plumbing/transport/ssh/common.go @@ -11,32 +11,22 @@ import ( "golang.org/x/crypto/ssh" ) -// New errors introduced by this package. var ( - ErrAdvertistedReferencesAlreadyCalled = errors.New("cannot call AdvertisedReference twice") - ErrAlreadyConnected = errors.New("ssh session already created") - ErrAuthRequired = errors.New("cannot connect: auth required") - ErrNotConnected = errors.New("not connected") - ErrUploadPackAnswerFormat = errors.New("git-upload-pack bad answer format") - ErrUnsupportedVCS = errors.New("only git is supported") - ErrUnsupportedRepo = errors.New("only github.com is supported") + errAlreadyConnected = errors.New("ssh session already created") ) -type Client struct{} +type client struct{} -var DefaultClient = NewClient() +// DefaultClient is the default SSH client. +var DefaultClient = &client{} -func NewClient() transport.Client { - return &Client{} -} - -func (c *Client) NewFetchPackSession(ep transport.Endpoint) ( +func (c *client) NewFetchPackSession(ep transport.Endpoint) ( transport.FetchPackSession, error) { return newFetchPackSession(ep) } -func (c *Client) NewSendPackSession(ep transport.Endpoint) ( +func (c *client) NewSendPackSession(ep transport.Endpoint) ( transport.SendPackSession, error) { return newSendPackSession(ep) @@ -49,6 +39,7 @@ type session struct { session *ssh.Session stdin io.WriteCloser stdout io.Reader + stderr io.Reader sessionDone chan error auth AuthMethod } @@ -70,6 +61,11 @@ func (s *session) Close() error { } s.connected = false + + //XXX: If did read the full packfile, then the session might be already + // closed. + _ = s.session.Close() + return s.client.Close() } @@ -79,7 +75,7 @@ func (s *session) Close() error { // environment var. func (s *session) connect() error { if s.connected { - return ErrAlreadyConnected + return errAlreadyConnected } if err := s.setAuthFromEndpoint(); err != nil { @@ -138,6 +134,11 @@ func (s *session) openSSHSession() error { return fmt.Errorf("cannot pipe remote stdout: %s", err) } + s.stderr, err = s.session.StderrPipe() + if err != nil { + return fmt.Errorf("cannot pipe remote stderr: %s", err) + } + return nil } @@ -149,3 +150,20 @@ func (s *session) runCommand(cmd string) chan error { return done } + +const ( + githubRepoNotFoundErr = "ERROR: Repository not found." + bitbucketRepoNotFoundErr = "conq: repository does not exist." +) + +func isRepoNotFoundError(s string) bool { + if strings.HasPrefix(s, githubRepoNotFoundErr) { + return true + } + + if strings.HasPrefix(s, bitbucketRepoNotFoundErr) { + return true + } + + return false +} diff --git a/plumbing/transport/ssh/common_test.go b/plumbing/transport/ssh/common_test.go index ac4d03e..da99148 100644 --- a/plumbing/transport/ssh/common_test.go +++ b/plumbing/transport/ssh/common_test.go @@ -7,11 +7,3 @@ import ( ) func Test(t *testing.T) { TestingT(t) } - -type ClientSuite struct{} - -var _ = Suite(&ClientSuite{}) - -func (s *ClientSuite) TestNewClient(c *C) { - c.Assert(DefaultClient, DeepEquals, NewClient()) -} diff --git a/plumbing/transport/ssh/fetch_pack.go b/plumbing/transport/ssh/fetch_pack.go index bda4edf..b43160a 100644 --- a/plumbing/transport/ssh/fetch_pack.go +++ b/plumbing/transport/ssh/fetch_pack.go @@ -2,13 +2,16 @@ package ssh import ( + "bufio" "bytes" "fmt" "io" + "gopkg.in/src-d/go-git.v4/plumbing/format/packp/advrefs" "gopkg.in/src-d/go-git.v4/plumbing/format/packp/pktline" "gopkg.in/src-d/go-git.v4/plumbing/format/packp/ulreq" "gopkg.in/src-d/go-git.v4/plumbing/transport" + "gopkg.in/src-d/go-git.v4/utils/ioutil" "golang.org/x/crypto/ssh" ) @@ -35,17 +38,35 @@ func newFetchPackSession(ep transport.Endpoint) (*fetchPackSession, error) { func (s *fetchPackSession) AdvertisedReferences() (*transport.UploadPackInfo, error) { if s.advRefsRun { - return nil, ErrAdvertistedReferencesAlreadyCalled + return nil, transport.ErrAdvertistedReferencesAlreadyCalled } + defer func() { s.advRefsRun = true }() + if err := s.ensureRunCommand(); err != nil { return nil, err } - defer func() { s.advRefsRun = true }() - i := transport.NewUploadPackInfo() - return i, i.Decode(s.stdout) + if err := i.Decode(s.stdout); err != nil { + if err != advrefs.ErrEmpty { + return nil, err + } + + _ = s.stdin.Close() + scan := bufio.NewScanner(s.stderr) + if !scan.Scan() { + return nil, transport.ErrEmptyRemoteRepository + } + + if isRepoNotFoundError(string(scan.Bytes())) { + return nil, transport.ErrRepositoryNotFound + } + + return nil, err + } + + return i, nil } // FetchPack returns a packfile for a given upload request. @@ -53,6 +74,10 @@ func (s *fetchPackSession) AdvertisedReferences() (*transport.UploadPackInfo, er func (s *fetchPackSession) FetchPack(req *transport.UploadPackRequest) ( io.ReadCloser, error) { + if req.IsEmpty() { + return nil, transport.ErrEmptyUploadPackRequest + } + if !s.advRefsRun { if _, err := s.AdvertisedReferences(); err != nil { return nil, err @@ -63,11 +88,19 @@ func (s *fetchPackSession) FetchPack(req *transport.UploadPackRequest) ( return nil, err } - return &fetchSession{ + fs := &fetchSession{ Reader: s.stdout, session: s.session.session, done: s.done, - }, nil + } + + r, err := ioutil.NonEmptyReader(fs) + if err == ioutil.ErrEmptyReader { + _ = fs.Close() + return nil, transport.ErrEmptyUploadPackRequest + } + + return ioutil.NewReadCloser(r, fs), nil } func (s *fetchPackSession) ensureRunCommand() error { diff --git a/plumbing/transport/ssh/fetch_pack_test.go b/plumbing/transport/ssh/fetch_pack_test.go index 3d62e57..a0321b3 100644 --- a/plumbing/transport/ssh/fetch_pack_test.go +++ b/plumbing/transport/ssh/fetch_pack_test.go @@ -1,100 +1,36 @@ package ssh import ( - "io/ioutil" "os" - "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/transport" + "gopkg.in/src-d/go-git.v4/plumbing/transport/test" . "gopkg.in/check.v1" ) type FetchPackSuite struct { - Endpoint transport.Endpoint + test.FetchPackSuite } var _ = Suite(&FetchPackSuite{}) func (s *FetchPackSuite) SetUpSuite(c *C) { - var err error - s.Endpoint, err = transport.NewEndpoint("git@github.com:git-fixtures/basic.git") - c.Assert(err, IsNil) - if os.Getenv("SSH_AUTH_SOCK") == "" { c.Skip("SSH_AUTH_SOCK is not set") } -} - -func (s *FetchPackSuite) TestDefaultBranch(c *C) { - r, err := DefaultClient.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.SymbolicReference("HEAD"), Equals, "refs/heads/master") -} + s.FetchPackSuite.Client = DefaultClient -func (s *FetchPackSuite) TestCapabilities(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) + ep, err := transport.NewEndpoint("git@github.com:git-fixtures/basic.git") c.Assert(err, IsNil) - defer func() { c.Assert(r.Close(), IsNil) }() + s.FetchPackSuite.Endpoint = ep - info, err := r.AdvertisedReferences() + ep, err = transport.NewEndpoint("git@github.com:git-fixtures/empty.git") c.Assert(err, IsNil) - c.Assert(info.Capabilities.Get("agent").Values, HasLen, 1) -} - -func (s *FetchPackSuite) TestFullFetchPack(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) - c.Assert(err, IsNil) - defer func() { c.Assert(r.Close(), IsNil) }() - - _, err = r.AdvertisedReferences() - c.Assert(err, IsNil) - - req := &transport.UploadPackRequest{} - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) - req.Want(plumbing.NewHash("e8d3ffab552895c19b9fcf7aa264d277cde33881")) - reader, err := r.FetchPack(req) - c.Assert(err, IsNil) - - defer func() { c.Assert(reader.Close(), IsNil) }() + s.FetchPackSuite.EmptyEndpoint = ep - b, err := ioutil.ReadAll(reader) + ep, err = transport.NewEndpoint("git@github.com:git-fixtures/non-existent.git") c.Assert(err, IsNil) - c.Check(len(b), Equals, 85585) -} - -func (s *FetchPackSuite) TestFetchPack(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) - c.Assert(err, IsNil) - defer func() { c.Assert(r.Close(), IsNil) }() - - req := &transport.UploadPackRequest{} - req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) - req.Want(plumbing.NewHash("e8d3ffab552895c19b9fcf7aa264d277cde33881")) - reader, err := r.FetchPack(req) - c.Assert(err, IsNil) - defer func() { c.Assert(reader.Close(), IsNil) }() - - b, err := ioutil.ReadAll(reader) - c.Assert(err, IsNil) - c.Check(len(b), Equals, 85585) -} - -func (s *FetchPackSuite) TestFetchError(c *C) { - r, err := DefaultClient.NewFetchPackSession(s.Endpoint) - c.Assert(err, IsNil) - defer func() { c.Assert(r.Close(), IsNil) }() - - req := &transport.UploadPackRequest{} - req.Want(plumbing.NewHash("1111111111111111111111111111111111111111")) - - reader, err := r.FetchPack(req) - c.Assert(err, IsNil) - - err = reader.Close() - c.Assert(err, Not(IsNil)) + s.FetchPackSuite.NonExistentEndpoint = ep } diff --git a/plumbing/transport/test/common.go b/plumbing/transport/test/common.go new file mode 100644 index 0000000..059c0c9 --- /dev/null +++ b/plumbing/transport/test/common.go @@ -0,0 +1,166 @@ +// Package test implements common test suite for different transport +// implementations. +// +package test + +import ( + "bytes" + "io" + "io/ioutil" + + "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/format/packfile" + "gopkg.in/src-d/go-git.v4/plumbing/transport" + "gopkg.in/src-d/go-git.v4/storage/memory" + + . "gopkg.in/check.v1" +) + +type FetchPackSuite struct { + Endpoint transport.Endpoint + EmptyEndpoint transport.Endpoint + NonExistentEndpoint transport.Endpoint + Client transport.Client +} + +func (s *FetchPackSuite) TestInfoEmpty(c *C) { + r, err := s.Client.NewFetchPackSession(s.EmptyEndpoint) + c.Assert(err, IsNil) + info, err := r.AdvertisedReferences() + c.Assert(err, Equals, transport.ErrEmptyRemoteRepository) + c.Assert(info, IsNil) +} + +func (s *FetchPackSuite) TestInfoNotExists(c *C) { + r, err := s.Client.NewFetchPackSession(s.NonExistentEndpoint) + c.Assert(err, IsNil) + info, err := r.AdvertisedReferences() + c.Assert(err, Equals, transport.ErrRepositoryNotFound) + c.Assert(info, IsNil) + + r, err = s.Client.NewFetchPackSession(s.NonExistentEndpoint) + c.Assert(err, IsNil) + req := &transport.UploadPackRequest{} + req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + reader, err := r.FetchPack(req) + c.Assert(err, Equals, transport.ErrRepositoryNotFound) + c.Assert(reader, IsNil) +} + +func (s *FetchPackSuite) TestCannotCallAdvertisedReferenceTwice(c *C) { + r, err := s.Client.NewFetchPackSession(s.Endpoint) + c.Assert(err, IsNil) + _, err = r.AdvertisedReferences() + c.Assert(err, IsNil) + _, err = r.AdvertisedReferences() + c.Assert(err, Equals, transport.ErrAdvertistedReferencesAlreadyCalled) +} + +func (s *FetchPackSuite) TestDefaultBranch(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.SymbolicReference("HEAD"), Equals, "refs/heads/master") +} + +func (s *FetchPackSuite) TestCapabilities(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.Get("agent").Values, HasLen, 1) +} + +func (s *FetchPackSuite) TestFullFetchPack(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, NotNil) + + req := &transport.UploadPackRequest{} + req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + + reader, err := r.FetchPack(req) + c.Assert(err, IsNil) + + s.checkObjectNumber(c, reader, 28) +} + +func (s *FetchPackSuite) TestFetchPack(c *C) { + r, err := s.Client.NewFetchPackSession(s.Endpoint) + c.Assert(err, IsNil) + defer func() { c.Assert(r.Close(), IsNil) }() + + req := &transport.UploadPackRequest{} + req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + + reader, err := r.FetchPack(req) + c.Assert(err, IsNil) + + s.checkObjectNumber(c, reader, 28) +} + +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 := &transport.UploadPackRequest{} + req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Have(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + + reader, err := r.FetchPack(req) + c.Assert(err, Equals, transport.ErrEmptyUploadPackRequest) + c.Assert(reader, IsNil) +} + +func (s *FetchPackSuite) TestFetchPackMulti(c *C) { + r, err := s.Client.NewFetchPackSession(s.Endpoint) + c.Assert(err, IsNil) + defer func() { c.Assert(r.Close(), IsNil) }() + + req := &transport.UploadPackRequest{} + req.Want(plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5")) + req.Want(plumbing.NewHash("e8d3ffab552895c19b9fcf7aa264d277cde33881")) + + reader, err := r.FetchPack(req) + c.Assert(err, IsNil) + + s.checkObjectNumber(c, reader, 31) +} + +func (s *FetchPackSuite) TestFetchError(c *C) { + r, err := s.Client.NewFetchPackSession(s.Endpoint) + c.Assert(err, IsNil) + + req := &transport.UploadPackRequest{} + req.Want(plumbing.NewHash("1111111111111111111111111111111111111111")) + + reader, err := r.FetchPack(req) + c.Assert(err, Equals, transport.ErrEmptyUploadPackRequest) + c.Assert(reader, IsNil) + + //XXX: We do not test Close error, since implementations might return + // different errors if a previous error was found. +} + +func (s *FetchPackSuite) checkObjectNumber(c *C, r io.Reader, n int) { + b, err := ioutil.ReadAll(r) + c.Assert(err, IsNil) + buf := bytes.NewBuffer(b) + scanner := packfile.NewScanner(buf) + storage := memory.NewStorage() + d, err := packfile.NewDecoder(scanner, storage) + c.Assert(err, IsNil) + _, err = d.Decode() + c.Assert(err, IsNil) + c.Assert(len(storage.Objects), Equals, n) +} |