Export the loaded manifest type as Manifest (closes #236)

Package mfer handed out a loaded manifest as the unexported type
manifest. Without a name for that type, a program using the library
could not keep a loaded manifest in a struct field or pass one to its
own functions. As sneak approved, the type is now exported as Manifest
with its fields kept private. Its existing methods (Files, Signer,
CreatedAt, String) are its whole API.

NewManifestFromReader and NewManifestFromFile return *Manifest. The two
lint suppressions that excused returning an unexported type from them
are removed. The type's doc comment now names the two loaders and drops
the pointer to the settled design question. String gets a doc comment.
A test in package mfer_test uses *mfer.Manifest from outside the
package.

Model: opus-5-5

docs/FORMAT.md

@@ -61,7 +61,7 @@ enforce a decompression size limit to prevent decompression bombs. The reference
 implementation limits decompressed size to 256 MiB. It writes zstd frames with a
 window of at most 8 MiB, the largest window the zstd format recommends decoders
 support, and refuses frames that ask for a larger one. It also refuses an inner
-message whose file entries, hashes and timestamps, counted at 176, 112 and 64
+message whose file entries, hashes and timestamps, counted at 160, 112 and 64
 bytes each, add up to more than 8 times its size. It refuses a manifest file
 larger than 258 MiB without reading the rest of it: zstd's worst case grows a
 256 MiB inner message by 1/256 to 257 MiB, and the last MiB is room for the

mfer/builder.go

@@ -335,7 +335,7 @@ func (b *Builder) Build(ctx context.Context, w io.Writer) error {
 	}
 
 	// Create a temporary manifest to use existing serialization
-	m := &manifest{
+	m := &Manifest{
 		pbInner:        inner,
 		signingOptions: b.signingOptions,
 		fixedUUID:      b.fixedUUID,

mfer/checker.go

@@ -123,7 +123,7 @@ func NewChecker(opts *CheckerOptions) (*Checker, error) {
 	}
 
 	var (
-		m            *manifest
+		m            *Manifest
 		manifestInfo os.FileInfo
 		err          error
 	)

mfer/constants.go

@@ -29,14 +29,14 @@ const (
 	// however short its encoding. checkDecodedSize refuses an inner
 	// message for which these add up to more than maxDecodedGrowth times
 	// its size. The mode is held in the file entry itself.
-	decodedFileEntrySize = 176
+	decodedFileEntrySize = 160
 	decodedHashSize      = 112
 	decodedTimestampSize = 64
 
 	// Each file entry mfer writes holds a path of at least one byte, a
 	// multihash at least as long as SHA-256's 34 bytes (AddFileWithHash
 	// refuses shorter ones) and a modification time: at least 47 bytes,
-	// counted at 352. So its manifests add up to at most about 7.49 times
+	// counted at 336. So its manifests add up to at most about 7.15 times
-	// their size, and this limit is about 7% above that.
+	// their size, and this limit is about 12% above that.
 	maxDecodedGrowth = 8
 )

mfer/deserialize.go

@@ -48,7 +48,7 @@ func validateUUID(data []byte) error {
 
 // validateOuterHeader checks the outer message's version, compression
 // type, and UUID.
-func (m *manifest) validateOuterHeader() error {
+func (m *Manifest) validateOuterHeader() error {
 	if m.pbOuter.GetVersion() != MFFileOuter_VERSION_ONE {
 		return errUnknownVersion
 	}
@@ -70,7 +70,7 @@ func (m *manifest) validateOuterHeader() error {
 // signature is present, verifies it against the embedded public key, which
 // must be one key, and checks that the signer field is that key's
 // fingerprint.
-func (m *manifest) verifyOuterIntegrity() error {
+func (m *Manifest) verifyOuterIntegrity() error {
 	h := sha256.New()
 
 	_, err := h.Write(m.pbOuter.GetInnerMessage())
@@ -117,7 +117,7 @@ func (m *manifest) verifyOuterIntegrity() error {
 
 // decompressInner decompresses the inner payload, enforcing size limits
 // to prevent decompression bombs.
-func (m *manifest) decompressInner() ([]byte, error) {
+func (m *Manifest) decompressInner() ([]byte, error) {
 	bb := bytes.NewBuffer(m.pbOuter.GetInnerMessage())
 
 	// By default the decoder decodes a payload under 128 KiB in full,
@@ -259,7 +259,7 @@ func validateEntryHashes(entry *MFFilePath) error {
 	}
 }
 
-func (m *manifest) deserializeInner() error {
+func (m *Manifest) deserializeInner() error {
 	err := m.validateOuterHeader()
 	if err != nil {
 		return err
@@ -364,10 +364,8 @@ func validateMagic(dat []byte) bool {
 
 // NewManifestFromReader reads a manifest from an io.Reader. It refuses a
 // manifest larger than MaxManifestSize, reading at most one byte past it.
-//
+func NewManifestFromReader(input io.Reader) (*Manifest, error) {
-//nolint:revive // unexported-return: exporting manifest is owner question 13
+	m := &Manifest{}
-func NewManifestFromReader(input io.Reader) (*manifest, error) {
-	m := &manifest{}
 
 	dat, err := readAtMost(input, MaxManifestSize)
 	if err != nil {
@@ -426,9 +424,7 @@ type ManifestFromFileOptions struct {
 
 // NewManifestFromFile reads a manifest from a file. It returns an error if
 // opts is nil or its path is empty.
-//
+func NewManifestFromFile(opts *ManifestFromFileOptions) (*Manifest, error) {
-//nolint:revive // unexported-return: exporting manifest is owner question 13
-func NewManifestFromFile(opts *ManifestFromFileOptions) (*manifest, error) {
 	if opts == nil || opts.Path == "" {
 		return nil, errManifestPathEmpty
 	}

mfer/deserialize_path_test.go

@@ -296,13 +296,13 @@ func TestDeserializeRefusesModeOutsidePermissionBits(t *testing.T) {
 }
 
 // Entries of a path, a SHA-256 hash, an empty second hash and an empty
-// modification time are counted at 464 bytes each (176 + 112 + 112 + 64)
+// modification time are counted at 448 bytes each (160 + 112 + 112 + 64)
-// and take 48 bytes plus the path to encode. A 9-character path makes that
+// and take 48 bytes plus the path to encode. A 7-character path makes that
-// 57 bytes, about 8.1 times: refused, and leaving any one of the four
+// 55 bytes, about 8.1 times: refused, and leaving any one of the four
-// uncounted, even the modification time, brings it under 8. A 10-character
+// uncounted, even the modification time, brings it under 8. An 8-character
-// path makes it 58 bytes, 8 times exactly: loaded. Without the second hash
+// path makes it 56 bytes, 8 times exactly: loaded. Without the second hash
 // an entry is never refused: it takes at least 47 bytes and is counted at
-// 352. There is one entry for each base-36 digit, enough that the 22 bytes
+// 336. There is one entry for each base-36 digit, enough that the 22 bytes
 // of the version and UUID change neither outcome, and each entry's path is
 // its number in base 36, padded with zeros to that length, since a manifest
 // lists a path only once.
@@ -319,8 +319,8 @@ func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
 		pathLen int
 		refused bool
 	}{
-		{9, true},
+		{7, true},
-		{10, false},
+		{8, false},
 	}
 
 	for _, tt := range tests {

mfer/errmsg_test.go

@@ -85,7 +85,7 @@ func TestValidatePathMessagesVerbatim(t *testing.T) {
 func TestSerializeInnerNotSetMessagesVerbatim(t *testing.T) {
 	t.Parallel()
 
-	m := &manifest{}
+	m := &Manifest{}
 
 	err := m.generate(context.Background())
 	require.ErrorIs(t, err, errInnerNotSet)

mfer/manifest.go

@@ -15,13 +15,10 @@ var (
 	errSHA256NotSet = errors.New("SHA256 hash not set")
 )
 
-// manifest holds the internal representation of a manifest file.
+// Manifest is a manifest loaded by NewManifestFromFile or
-// Use NewManifestFromFile or NewManifestFromReader to load an existing
+// NewManifestFromReader. Its fields are private; only its methods are the
-// manifest, or use Builder to create a new one.
+// library's API.
-//
+type Manifest struct {
-// Whether this type should be exported is an open design question owned by
-// the repository owner; see README design question 13.
-type manifest struct {
 	pbInner        *MFFile
 	pbOuter        *MFFileOuter
 	output         *bytes.Buffer
@@ -29,7 +26,9 @@ type manifest struct {
 	fixedUUID      []byte // if set, use this UUID instead of generating one
 }
 
-func (m *manifest) String() string {
+// String returns a short description giving the number of files the
+// manifest lists, such as "<Manifest count=3>".
+func (m *Manifest) String() string {
 	count := 0
 	if m.pbInner != nil {
 		count = len(m.pbInner.GetFiles())
@@ -39,7 +38,7 @@ func (m *manifest) String() string {
 }
 
 // Files returns all file entries from a loaded manifest.
-func (m *manifest) Files() []*MFFilePath {
+func (m *Manifest) Files() []*MFFilePath {
 	if m.pbInner == nil {
 		return nil
 	}
@@ -50,7 +49,7 @@ func (m *manifest) Files() []*MFFilePath {
 // Signer returns the fingerprint of the key that made a loaded manifest's
 // signature, which loading the manifest checked, or nil if the manifest is
 // not signed.
-func (m *manifest) Signer() []byte {
+func (m *Manifest) Signer() []byte {
 	if len(m.pbOuter.GetSignature()) == 0 {
 		return nil
 	}
@@ -60,14 +59,14 @@ func (m *manifest) Signer() []byte {
 
 // CreatedAt returns the createdAt timestamp of a loaded manifest, or nil
 // if the manifest has none.
-func (m *manifest) CreatedAt() *Timestamp {
+func (m *Manifest) CreatedAt() *Timestamp {
 	return m.pbInner.GetCreatedAt()
 }
 
 // signatureString generates the canonical string used for signing/verification.
 // Format: MAGIC-UUID-MULTIHASH where UUID and multihash are hex-encoded.
 // Requires pbOuter to be set with Uuid and Sha256 fields.
-func (m *manifest) signatureString() (string, error) {
+func (m *Manifest) signatureString() (string, error) {
 	if m.pbOuter == nil {
 		return "", errOuterNotSet
 	}

mfer/manifest_external_test.go

@@ -0,0 +1,49 @@
+package mfer_test
+
+import (
+	"bytes"
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/mfer/mfer"
+)
+
+// archive keeps a loaded manifest in a field, as a type in a program that
+// imports mfer would.
+type archive struct {
+	manifest *mfer.Manifest
+}
+
+// fileCount takes a loaded manifest, as a function in such a program would.
+func fileCount(m *mfer.Manifest) int {
+	return len(m.Files())
+}
+
+// A program that imports mfer can name the type of a loaded manifest, keep
+// one in a struct field, pass it to its own functions, and read it through
+// its methods.
+func TestManifestIsUsableOutsideThePackage(t *testing.T) {
+	t.Parallel()
+
+	content := []byte("hello\n")
+	builder := mfer.NewBuilder()
+
+	_, err := builder.AddFile("hello.txt", mfer.FileSize(len(content)),
+		mfer.ModTime{}, 0, bytes.NewReader(content), nil)
+	require.NoError(t, err)
+
+	var written bytes.Buffer
+	require.NoError(t, builder.Build(context.Background(), &written))
+
+	loaded, err := mfer.NewManifestFromReader(&written)
+	require.NoError(t, err)
+
+	a := archive{manifest: loaded}
+
+	require.Equal(t, 1, fileCount(a.manifest))
+	require.Equal(t, "hello.txt", a.manifest.Files()[0].GetPath())
+	require.Nil(t, a.manifest.Signer())
+	require.Nil(t, a.manifest.CreatedAt())
+	require.Equal(t, "<Manifest count=1>", a.manifest.String())
+}

mfer/manifest_test.go

@@ -21,7 +21,7 @@ func TestSignatureString(t *testing.T) {
 		"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855")
 	require.NoError(t, err)
 
-	m := &manifest{pbOuter: &MFFileOuter{Uuid: manifestUUID[:], Sha256: sha256Hash}}
+	m := &Manifest{pbOuter: &MFFileOuter{Uuid: manifestUUID[:], Sha256: sha256Hash}}
 
 	got, err := m.signatureString()
 	require.NoError(t, err)

mfer/serialize.go

@@ -51,7 +51,7 @@ func newTimestampFromTime(t time.Time) *Timestamp {
 	}
 }
 
-func (m *manifest) generate(ctx context.Context) error {
+func (m *Manifest) generate(ctx context.Context) error {
 	if m.pbInner == nil {
 		return errInnerNotSet
 	}
@@ -78,7 +78,7 @@ func (m *manifest) generate(ctx context.Context) error {
 	return nil
 }
 
-func (m *manifest) generateOuter(ctx context.Context) error {
+func (m *Manifest) generateOuter(ctx context.Context) error {
 	if m.pbInner == nil {
 		return errInternal
 	}
@@ -145,7 +145,7 @@ func (m *manifest) generateOuter(ctx context.Context) error {
 // signOuter signs the outer message with the secret key in the signing
 // options and embeds the signature, the key's fingerprint and its public
 // key.
-func (m *manifest) signOuter(ctx context.Context) error {
+func (m *Manifest) signOuter(ctx context.Context) error {
 	// Unlocking a protected key can take a while; do not start once ctx
 	// has ended.
 	err := ctx.Err()