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

1 files changed, 30 insertions, 22 deletions

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()
-}