plumbing: pktline rewrite (#1)

* plumbing: pktline, dry parsing length and rename empty pkt

* plumbing: pktline, implement pktline reader

* plumbing: pktline, implement pktline writer

* plumbing: pktline tests, add benchmarks

* plumbing: pktline, deprecate the old impl and use pktline read/writer

* plumbing: pktline, use static methods approach

* plumbing: pktline, add WriteResponseEnd and method docs

* plumbing: pktline, remove scanner/encoder

* plumbing: pktline, update comment

* plumbing: pktline, add benchmark tests

* plumbing: pktline, error on nil writer

* plumbing: pktline, update plumbing/protocol/packp/uppackresp.go

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>

* plumbing: pktline, update plumbing/protocol/packp/report_status.go

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>

* plumbing: pktline, update plumbing/protocol/packp/report_status.go

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>

* plumbing: pktline, update plumbing/protocol/packp/gitproto.go

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>

* plumbing: pktline, update plumbing/format/pktline/pktline_bench_test.go

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>

* plumbing: packp, update imports

* plumbing: pktline, add GetPacketBuffer and PutPacketBuffer

* plumbing: pktline, use shared buffer with ReadPacketLine

* plumbing: pktline, add scanner based type

* plumbing: pktline, update benchmarks

* plumbing: pktline, fix errors and improve perf and memory

* plumbing: pktline, shorten method names

Pktline is already implied in the package name, no need to have "Packet"
in the method names.

* plumbing: pktline, update plumbing/protocol/packp/advrefs_decode.go

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>

---------

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>

plumbing/format/pktline/common.go

@@ -0,0 +1,56 @@
+package pktline
+
+import "errors"
+
+const (
+	// Err is returned when the pktline has encountered an error.
+	Err = iota - 1
+
+	// Flush is the numeric value of a flush packet. It is returned when the
+	// pktline is a flush packet.
+	Flush
+
+	// Delim is the numeric value of a delim packet. It is returned when the
+	// pktline is a delim packet.
+	Delim
+
+	// ResponseEnd is the numeric value of a response-end packet. It is
+	// returned when the pktline is a response-end packet.
+	ResponseEnd
+)
+
+const (
+	// MaxPayloadSize is the maximum payload size of a pkt-line in bytes.
+	// See https://git-scm.com/docs/protocol-common#_pkt_line_format
+	MaxPayloadSize = MaxPacketSize - PacketLenSize
+
+	// MaxPacketSize is the maximum packet size of a pkt-line in bytes.
+	// See https://git-scm.com/docs/protocol-common#_pkt_line_format
+	MaxPacketSize = 65520
+
+	// PacketLenSize is the size of the packet length in bytes.
+	PacketLenSize = 4
+)
+
+var (
+	// ErrPayloadTooLong is returned by the Encode methods when any of the
+	// provided payloads is bigger than MaxPayloadSize.
+	ErrPayloadTooLong = errors.New("payload is too long")
+
+	// ErrInvalidPktLen is returned by Err() when an invalid pkt-len is found.
+	ErrInvalidPktLen = errors.New("invalid pkt-len found")
+)
+
+var (
+	// flushPkt are the contents of a flush-pkt pkt-line.
+	flushPkt = []byte{'0', '0', '0', '0'}
+
+	// delimPkt are the contents of a delim-pkt pkt-line.
+	delimPkt = []byte{'0', '0', '0', '1'}
+
+	// responseEndPkt are the contents of a response-end-pkt pkt-line.
+	responseEndPkt = []byte{'0', '0', '0', '2'}
+
+	// emptyPkt is an empty string pkt-line payload.
+	emptyPkt = []byte{'0', '0', '0', '4'}
+)

plumbing/format/pktline/error.go

@@ -1,20 +1,25 @@
 package pktline
 
 import (
-	"bytes"
 	"errors"
 	"io"
-	"strings"
 )
 
 var (
 	// ErrInvalidErrorLine is returned by Decode when the packet line is not an
 	// error line.
 	ErrInvalidErrorLine = errors.New("expected an error-line")
 
+	// ErrNilWriter is returned when a nil writer is passed to WritePacket.
+	ErrNilWriter = errors.New("nil writer")
+
 	errPrefix = []byte("ERR ")
 )
 
+const (
+	errPrefixSize = PacketLenSize
+)
+
 // ErrorLine is a packet line that contains an error message.
 // Once this packet is sent by client or server, the data transfer process is
 // terminated.
@@ -30,22 +35,17 @@ func (e *ErrorLine) Error() string {
 
 // Encode encodes the ErrorLine into a packet line.
 func (e *ErrorLine) Encode(w io.Writer) error {
-	p := NewEncoder(w)
-	return p.Encodef("%s%s\n", string(errPrefix), e.Text)
+	_, err := Writef(w, "%s%s\n", errPrefix, e.Text)
+	return err
 }
 
 // Decode decodes a packet line into an ErrorLine.
 func (e *ErrorLine) Decode(r io.Reader) error {
-	s := NewScanner(r)
-	if !s.Scan() {
-		return s.Err()
-	}
-
-	line := s.Bytes()
-	if !bytes.HasPrefix(line, errPrefix) {
+	_, _, err := ReadLine(r)
+	var el *ErrorLine
+	if !errors.As(err, &el) {
 		return ErrInvalidErrorLine
 	}
-
-	e.Text = strings.TrimSpace(string(line[4:]))
+	e.Text = el.Text
 	return nil
 }

plumbing/format/pktline/error_test.go

@@ -1,6 +1,7 @@
 package pktline
 
 import (
+	"bufio"
 	"bytes"
 	"errors"
 	"io"
@@ -33,7 +34,7 @@ func TestDecodeEmptyErrorLine(t *testing.T) {
 	var buf bytes.Buffer
 	e := &ErrorLine{}
 	err := e.Decode(&buf)
-	if err != nil {
+	if !errors.Is(err, ErrInvalidErrorLine) {
 		t.Fatal(err)
 	}
 	if e.Text != "" {
@@ -44,10 +45,10 @@ func TestDecodeEmptyErrorLine(t *testing.T) {
 func TestDecodeErrorLine(t *testing.T) {
 	var buf bytes.Buffer
 	buf.WriteString("000eERR foobar")
-	var e *ErrorLine
+	var e ErrorLine
 	err := e.Decode(&buf)
-	if !errors.As(err, &e) {
-		t.Fatalf("expected error line, got: %T: %v", err, err)
+	if err != nil {
+		t.Fatal(err)
 	}
 	if e.Text != "foobar" {
 		t.Fatalf("unexpected error line: %q", e.Text)
@@ -57,12 +58,22 @@ func TestDecodeErrorLine(t *testing.T) {
 func TestDecodeErrorLineLn(t *testing.T) {
 	var buf bytes.Buffer
 	buf.WriteString("000fERR foobar\n")
-	var e *ErrorLine
+	var e ErrorLine
 	err := e.Decode(&buf)
-	if !errors.As(err, &e) {
-		t.Fatalf("expected error line, got: %T: %v", err, err)
+	if err != nil {
+		t.Fatal(err)
 	}
 	if e.Text != "foobar" {
 		t.Fatalf("unexpected error line: %q", e.Text)
 	}
 }
+
+func TestPeekErrorLine(t *testing.T) {
+	var buf bytes.Buffer
+	buf.WriteString("000fERR foobar\n")
+	var e *ErrorLine
+	_, _, err := PeekLine(bufio.NewReader(&buf))
+	if !errors.As(err, &e) {
+		t.Fatalf("expected error line, got: %T: %v", err, err)
+	}
+}

plumbing/format/pktline/length.go

@@ -0,0 +1,85 @@
+package pktline
+
+// ParseLength parses a four digit hexadecimal number from the given byte slice
+// into its integer representation. If the byte slice contains non-hexadecimal,
+// it will return an error.
+func ParseLength(b []byte) (int, error) {
+	if b == nil {
+		return Err, ErrInvalidPktLen
+	}
+
+	n, err := hexDecode(b)
+	if err != nil {
+		return Err, err
+	}
+
+	if n == 3 {
+		return Err, ErrInvalidPktLen
+	}
+
+	// Limit the maximum size of a pkt-line to 65520 bytes.
+	// Fixes: b4177b89c08b (plumbing: format: pktline, Accept oversized pkt-lines up to 65524 bytes)
+	// See https://github.com/git/git/commit/7841c4801ce51f1f62d376d164372e8677c6bc94
+	if n > MaxPacketSize {
+		return Err, ErrInvalidPktLen
+	}
+
+	return n, nil
+}
+
+// Turns the hexadecimal representation of a number in a byte slice into
+// a number. This function substitute strconv.ParseUint(string(buf), 16,
+// 16) and/or hex.Decode, to avoid generating new strings, thus helping the
+// GC.
+func hexDecode(buf []byte) (int, error) {
+	if len(buf) < 4 {
+		return 0, ErrInvalidPktLen
+	}
+
+	var ret int
+	for i := 0; i < PacketLenSize; i++ {
+		n, err := asciiHexToByte(buf[i])
+		if err != nil {
+			return 0, ErrInvalidPktLen
+		}
+		ret = 16*ret + int(n)
+	}
+	return ret, nil
+}
+
+// turns the hexadecimal ascii representation of a byte into its
+// numerical value.  Example: from 'b' to 11 (0xb).
+func asciiHexToByte(b byte) (byte, error) {
+	switch {
+	case b >= '0' && b <= '9':
+		return b - '0', nil
+	case b >= 'a' && b <= 'f':
+		return b - 'a' + 10, nil
+	default:
+		return 0, ErrInvalidPktLen
+	}
+}
+
+// Returns the hexadecimal ascii representation of the 16 less
+// significant bits of n.  The length of the returned slice will always
+// be 4.  Example: if n is 1234 (0x4d2), the return value will be
+// []byte{'0', '4', 'd', '2'}.
+func asciiHex16(n int) []byte {
+	var ret [4]byte
+	ret[0] = byteToASCIIHex(byte(n & 0xf000 >> 12))
+	ret[1] = byteToASCIIHex(byte(n & 0x0f00 >> 8))
+	ret[2] = byteToASCIIHex(byte(n & 0x00f0 >> 4))
+	ret[3] = byteToASCIIHex(byte(n & 0x000f))
+
+	return ret[:]
+}
+
+// turns a byte into its hexadecimal ascii representation.  Example:
+// from 11 (0xb) to 'b'.
+func byteToASCIIHex(n byte) byte {
+	if n < 10 {
+		return '0' + n
+	}
+
+	return 'a' - 10 + n
+}