Refactor the eager-header reads for readability.

This pulls the header reads into a function that works in terms of the
number of records requested.  This preserves the existing logic of
initially reading 15 records and then falling back if that fails.

In the event of a header with more than 15 records, it will read all
records, including the already-seen final 15 records.

internal/pack/pack.go

@@ -170,26 +170,74 @@ func (p *Packer) String() string {
 	return fmt.Sprintf("<Packer %d blobs, %d bytes>", len(p.blobs), p.bytes)
 }
 
-const maxHeaderSize = 16 * 1024 * 1024
+var (
-
 	// size of the header-length field at the end of the file
-var headerLengthSize = binary.Size(uint32(0))
+	headerLengthSize = binary.Size(uint32(0))
-
 	// we require at least one entry in the header, and one blob for a pack file
-var minFileSize = entrySize + crypto.Extension + uint(headerLengthSize)
+	minFileSize = entrySize + crypto.Extension + uint(headerLengthSize)
+)
 
+const (
+	maxHeaderSize = 16 * 1024 * 1024
 	// number of header enries to download as part of header-length request
-var eagerEntries = uint(15)
+	eagerEntries = 15
+)
+
+// readRecords reads count records from the underlying ReaderAt, returning the
+// raw header, the total number of records in the header, and any error.  If
+// the header contains fewer than count entries, the return value is truncated.
+func readRecords(rd io.ReaderAt, size int64, count int) ([]byte, int, error) {
+	var bufsize int
+	bufsize += count * int(entrySize)
+	bufsize += crypto.Extension
+	bufsize += headerLengthSize
+
+	if bufsize > int(size) {
+		bufsize = int(size)
+	}
+
+	b := make([]byte, bufsize)
+	off := size - int64(bufsize)
+	if _, err := rd.ReadAt(b, off); err != nil {
+		return nil, 0, err
+	}
+
+	header := b[len(b)-headerLengthSize:]
+	b = b[:len(b)-headerLengthSize]
+	hl := binary.LittleEndian.Uint32(header)
+	debug.Log("header length: %v", hl)
+
+	var err error
+	switch {
+	case hl == 0:
+		err = InvalidFileError{Message: "header length is zero"}
+	case hl < crypto.Extension:
+		err = InvalidFileError{Message: "header length is too small"}
+	case (hl-crypto.Extension)%uint32(entrySize) != 0:
+		err = InvalidFileError{Message: "header length is invalid"}
+	case int64(hl) > size-int64(headerLengthSize):
+		err = InvalidFileError{Message: "header is larger than file"}
+	case int64(hl) > maxHeaderSize:
+		err = InvalidFileError{Message: "header is larger than maxHeaderSize"}
+	}
+	if err != nil {
+		return nil, 0, errors.Wrap(err, "readHeader")
+	}
+
+	c := (int(hl) - crypto.Extension) / int(entrySize)
+	if c < count {
+		recordSize := c * int(entrySize)
+		start := len(b) - (recordSize + crypto.Extension)
+		b = b[start:]
+	}
+
+	return b, c, nil
+}
 
 // readHeader reads the header at the end of rd. size is the length of the
 // whole data accessible in rd.
 func readHeader(rd io.ReaderAt, size int64) ([]byte, error) {
 	debug.Log("size: %v", size)
-	if size == 0 {
-		err := InvalidFileError{Message: "file is empty"}
-		return nil, errors.Wrap(err, "readHeader")
-	}
-
 	if size < int64(minFileSize) {
 		err := InvalidFileError{Message: "file is too small"}
 		return nil, errors.Wrap(err, "readHeader")
@@ -199,69 +247,19 @@ func readHeader(rd io.ReaderAt, size int64) ([]byte, error) {
 	// eagerly download eagerEntries header entries as part of header-length request.
 	// only make second request if actual number of entries is greater than eagerEntries
 
-	eagerHl := uint32((eagerEntries * entrySize) + crypto.Extension)
+	b, c, err := readRecords(rd, size, eagerEntries)
-	if int64(eagerHl)+int64(headerLengthSize) > size {
-		eagerHl = uint32(size) - uint32(headerLengthSize)
-	}
-	eagerBuf := make([]byte, eagerHl+uint32(headerLengthSize))
-
-	n, err := rd.ReadAt(eagerBuf, size-int64(len(eagerBuf)))
 	if err != nil {
 		return nil, err
 	}
-	if n != len(eagerBuf) {
+	if c <= eagerEntries {
-		return nil, errors.New("not enough bytes read")
+		// eager read sufficed, return what we got
+		return b, nil
 	}
-
+	b, _, err = readRecords(rd, size, c)
-	hl := binary.LittleEndian.Uint32(eagerBuf[eagerHl:])
-	debug.Log("header length: %v", size)
-
-	if hl == 0 {
-		err := InvalidFileError{Message: "header length is zero"}
-		return nil, errors.Wrap(err, "readHeader")
-	}
-
-	if hl < crypto.Extension {
-		err := InvalidFileError{Message: "header length is too small"}
-		return nil, errors.Wrap(err, "readHeader")
-	}
-
-	if (hl-crypto.Extension)%uint32(entrySize) != 0 {
-		err := InvalidFileError{Message: "header length is invalid"}
-		return nil, errors.Wrap(err, "readHeader")
-	}
-
-	if int64(hl) > size-int64(headerLengthSize) {
-		err := InvalidFileError{Message: "header is larger than file"}
-		return nil, errors.Wrap(err, "readHeader")
-	}
-
-	if int64(hl) > maxHeaderSize {
-		err := InvalidFileError{Message: "header is larger than maxHeaderSize"}
-		return nil, errors.Wrap(err, "readHeader")
-	}
-
-	eagerBuf = eagerBuf[:eagerHl]
-
-	var buf []byte
-	if hl <= eagerHl {
-		// already have all header bytes. yay.
-		buf = eagerBuf[eagerHl-hl:]
-	} else {
-		// need more header bytes
-		buf = make([]byte, hl)
-		missingHl := hl - eagerHl
-		n, err := rd.ReadAt(buf[:missingHl], size-int64(hl)-int64(headerLengthSize))
 	if err != nil {
-			return nil, errors.Wrap(err, "ReadAt")
+		return nil, err
 	}
-		if uint32(n) != missingHl {
+	return b, nil
-			return nil, errors.New("not enough bytes read")
-		}
-		copy(buf[hl-eagerHl:], eagerBuf)
-	}
-
-	return buf, nil
 }
 
 // InvalidFileError is return when a file is found that is not a pack file.