transport/internal: error handling fixes and clean up (#160)

* protocol/packp: remove redundant isFlush check on AdvRefs.
* protocol/packp: improve AdvRefs documentation.
* transport: improve error handling for non-existing repos.
* protocol/packp: AdvRefs Decode now returns different errors for
  empty, but syntactically correct, AdvRefs message (ErrEmptyAdvRefs)
  and empty input (ErrEmptyInput).
* transport/internal/common: read stderr only when needed (ErrEmptyInput).
  Close the client gracefully.
* transport/internal/common: missing stderr on non existing repository
  does not block.
* transport/internal/common: buffer error messages.
* transport/file: fix changing binary name, add tests.
* transport/file: support changing git-upload-pack and git-receive-pack
  binary names.
* transport/file: add tests for misbehaving servers.
* transport/internal/common: remove Stderr field.
* transport/internal/common: do not close twice.

1 files changed, 22 insertions, 19 deletions

diff --git a/plumbing/protocol/packp/advrefs_decode.go b/plumbing/protocol/packp/advrefs_decode.go
index 5926645..964e3eb 100644
--- a/plumbing/protocol/packp/advrefs_decode.go
+++ b/plumbing/protocol/packp/advrefs_decode.go
@@ -27,8 +27,13 @@ type advRefsDecoder struct {
 	data  *AdvRefs         // parsed data is stored here
 }
 
-// ErrEmptyAdvRefs is returned by Decode when there was no advertised-message at all
-var ErrEmptyAdvRefs = errors.New("empty advertised-ref message")
+var (
+	// ErrEmptyAdvRefs is returned by Decode if it gets an empty advertised
+	// references message.
+	ErrEmptyAdvRefs = errors.New("empty advertised-ref message")
+	// ErrEmptyInput is returned by Decode if the input is empty.
+	ErrEmptyInput = errors.New("empty input")
+)
 
 func newAdvRefsDecoder(r io.Reader) *advRefsDecoder {
 	return &advRefsDecoder{
@@ -67,7 +72,7 @@ func (d *advRefsDecoder) nextLine() bool {
 		}
 
 		if d.nLine == 1 {
-			d.err = ErrEmptyAdvRefs
+			d.err = ErrEmptyInput
 			return false
 		}
 
@@ -87,33 +92,31 @@ func decodePrefix(d *advRefsDecoder) decoderStateFn {
 		return nil
 	}
 
-	// If the repository is empty, we receive a flush here (SSH).
-	if isFlush(d.line) {
-		d.err = ErrEmptyAdvRefs
+	if !isPrefix(d.line) {
+		return decodeFirstHash
+	}
+
+	tmp := make([]byte, len(d.line))
+	copy(tmp, d.line)
+	d.data.Prefix = append(d.data.Prefix, tmp)
+	if ok := d.nextLine(); !ok {
 		return nil
 	}
 
-	if isPrefix(d.line) {
-		tmp := make([]byte, len(d.line))
-		copy(tmp, d.line)
-		d.data.Prefix = append(d.data.Prefix, tmp)
-		if ok := d.nextLine(); !ok {
-			return nil
-		}
+	if !isFlush(d.line) {
+		return decodeFirstHash
 	}
 
-	if isFlush(d.line) {
-		d.data.Prefix = append(d.data.Prefix, pktline.Flush)
-		if ok := d.nextLine(); !ok {
-			return nil
-		}
+	d.data.Prefix = append(d.data.Prefix, pktline.Flush)
+	if ok := d.nextLine(); !ok {
+		return nil
 	}
 
 	return decodeFirstHash
 }
 
 func isPrefix(payload []byte) bool {
-	return payload[0] == '#'
+	return len(payload) > 0 && payload[0] == '#'
 }
 
 func isFlush(payload []byte) bool {