Send fetch progress at most once per second (closes #206)

progressWriter sent an update for every chunk io.Copy wrote, so fetch
logged a line per 32 KiB. It now sends at most one per second, waiting
a second before the first as gen and check do, and one more when the
last byte is written.

TestFetchProgress now checks that a download of several chunks sends no
more than one update per second plus the last.

Model: opus-5-5

docs/FORMAT.md

@@ -174,16 +174,9 @@ also refuses a manifest whose `signingPubKey` holds a DSA key or subkey, or any
 secret key or subkey, since checking the self-signatures of a DSA key or the
 numbers of a secret key can take hours when those numbers are very large.
 
-A signature is good only if the key that made it was valid at the signature's
-creation time. The primary key is judged by the newest self-signature on its
-primary user ID, and a subkey by its newest binding signature. By the
-signature's creation time the primary key and that self-signature must have been
-made and neither may have expired, and when a subkey made the signature the same
-holds of the subkey and its binding signature. A signature made before that
-self-signature or binding signature is not good, even if an older one made the
-key valid at that time. A key that has expired since does not make the signature
-bad, because a manifest outlives its key. A signature past its own expiry time
-is not good, whether or not the key has expired too.
+A signing key that has expired does not make the signature bad, because a
+manifest outlives its key. A signature past its own expiry time is not good,
+whether or not the key has expired too.
 
 ## Deterministic Serialization
 

internal/cli/fetch.go

@@ -679,21 +679,26 @@ func resolveManifestURL(inputURL string) (string, error) {
 	return parsed.JoinPath(defaultManifestName).String(), nil
 }
 
-// progressWriter wraps an io.Writer and reports progress to a channel.
+// progressWriter wraps an io.Writer and reports progress to a channel,
+// at most once per second and once more when the last byte is written.
 type progressWriter struct {
-	w         io.Writer
-	path      string
-	total     int64
-	written   int64
-	startTime time.Time
-	progress  chan<- DownloadProgress
+	w            io.Writer
+	path         string
+	total        int64
+	written      int64
+	startTime    time.Time
+	lastProgress time.Time
+	progress     chan<- DownloadProgress
 }
 
 func (pw *progressWriter) Write(p []byte) (int, error) {
 	n, err := pw.w.Write(p)
 
 	pw.written += int64(n)
-	if pw.progress != nil {
+
+	isLast := pw.written == pw.total
+	if pw.progress != nil &&
+		(isLast || time.Since(pw.lastProgress) >= time.Second) {
 		var (
 			bytesPerSec float64
 			eta         time.Duration
@@ -715,6 +720,8 @@ func (pw *progressWriter) Write(p []byte) (int, error) {
 			BytesPerSec: bytesPerSec,
 			ETA:         eta,
 		})
+
+		pw.lastProgress = time.Now()
 	}
 
 	return n, err
@@ -899,13 +906,16 @@ func saveResponse(
 	// Set up hash computation
 	h := sha256.New()
 
-	// Create progress-reporting writer that also computes hash
+	// Create progress-reporting writer that also computes hash. The first
+	// update waits a second, as gen's and check's do.
+	start := time.Now()
 	pw := &progressWriter{
-		w:         io.MultiWriter(out, h),
-		path:      localPath,
-		total:     totalBytes,
-		startTime: time.Now(),
-		progress:  progress,
+		w:            io.MultiWriter(out, h),
+		path:         localPath,
+		total:        totalBytes,
+		startTime:    start,
+		lastProgress: start,
+		progress:     progress,
 	}
 
 	// Copy content while hashing and reporting progress. One byte past

internal/cli/fetch_test.go

@@ -516,7 +516,7 @@ func TestManifestDownloadStopsAtLimit(t *testing.T) {
 func TestFetchProgress(t *testing.T) {
 	// Create source filesystem with a larger test file
 	sourceFs := afero.NewMemMapFs()
-	// Create content large enough to trigger multiple progress updates
+	// Content large enough that io.Copy writes it in at least four chunks
 	content := bytes.Repeat([]byte("x"), 100*1024) // 100KB
 	require.NoError(t, afero.WriteFile(sourceFs, "/large.txt", content, 0o644))
 
@@ -559,8 +559,11 @@ func TestFetchProgress(t *testing.T) {
 	}()
 
 	// Download
+	start := time.Now()
 	err = downloadFile(context.Background(), testClient(),
 		server.URL+"/large.txt", ".", "large.txt", files[0], progress)
+	elapsed := time.Since(start)
+
 	close(progress)
 	<-done
 
@@ -569,6 +572,12 @@ func TestFetchProgress(t *testing.T) {
 	// Verify we got progress updates
 	assert.NotEmpty(t, progressUpdates, "should have received progress updates")
 
+	// At most one update per second, plus the one reporting the file
+	// complete: a download that takes under a second sends two at most,
+	// however many chunks it writes.
+	assert.LessOrEqual(t, len(progressUpdates), 2+int(elapsed/time.Second),
+		"one progress update per second at most, plus the last")
+
 	// Verify final progress shows complete
 	if len(progressUpdates) > 0 {
 		last := progressUpdates[len(progressUpdates)-1]

mfer/openpgp.go

@@ -57,9 +57,7 @@ var (
 		"signature must hold exactly one signature")
 	errNotOneArmoredBlock = errors.New(
 		"must be exactly one armored block and nothing else")
-	errMalformedArmor        = errors.New("armor is malformed")
-	errKeyNotValidWhenSigned = errors.New(
-		"signature was made when its key was not valid")
+	errMalformedArmor = errors.New("armor is malformed")
 )
 
 // SigningOptions holds the key a manifest is signed with.
@@ -247,9 +245,8 @@ func fingerprint(key *openpgp.Entity) string {
 
 // verifySignature checks that signature is one good, unexpired OpenPGP
 // signature over data, made by the one primary key in pubKey or one of its
-// subkeys while that key was valid, and returns that primary key's
-// fingerprint. The key may have expired since. signature and pubKey may
-// each be armored or binary.
+// subkeys, and returns that primary key's fingerprint. signature and pubKey
+// may each be armored or binary.
 func verifySignature(data, signature, pubKey []byte) (string, error) {
 	key, err := readOneKey(pubKey, "embedded public key block", true)
 	if err != nil {
@@ -284,25 +281,6 @@ func verifySignature(data, signature, pubKey []byte) (string, error) {
 		return "", fmt.Errorf("verify signature: %w", err)
 	}
 
-	// The library checks whether the key is valid at the time its config
-	// gives, which above is now. Verifying again as of the signature's
-	// creation time refuses a signature made before the key or subkey that
-	// made it was made, after it expired, or after its self-signature or
-	// binding signature expired. The library judges the primary key by the
-	// newest self-signature on its primary user ID and a subkey by its
-	// newest binding signature, so this also refuses a signature made
-	// before either of those, even if an older one was valid then.
-	signedAt := &packet.Config{
-		Time: func() time.Time { return sig.CreationTime },
-	}
-
-	_, _, err = openpgp.VerifyDetachedSignature(openpgp.EntityList{key},
-		bytes.NewReader(data), bytes.NewReader(sigData), signedAt)
-	if err != nil {
-		return "", fmt.Errorf("verify signature: %w: %w",
-			errKeyNotValidWhenSigned, err)
-	}
-
 	return fingerprint(key), nil
 }
 

mfer/openpgp_test.go

@@ -4,7 +4,6 @@ package mfer
 import (
 	"bytes"
 	"context"
-	"crypto"
 	"crypto/dsa" //nolint:staticcheck // SA1019: tests need a DSA key to refuse
 	"io"
 	"math/big"
@@ -420,9 +419,8 @@ func TestVerifySignature(t *testing.T) {
 
 // TestVerifySignatureKeyExpiredSince verifies two signatures made in 2020
 // by a key that expired a day after it was made. A signature with no
-// expiry time is still good, as is one made then by a signing subkey that
-// expired a day after it was made. One that expired an hour after it was
-// made is refused, although the key has expired too.
+// expiry time is still good. One that expired an hour after it was made is
+// refused, although the key has expired too.
 func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	t.Parallel()
 
@@ -450,132 +448,10 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, fingerprint(key), signer)
 
-	withSubkey := newTestKey(t, &packet.Config{Time: config.Time})
-	require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
-		Algorithm: packet.PubKeyAlgoEdDSA, Time: config.Time,
-		KeyLifetimeSecs: config.KeyLifetimeSecs,
-	}))
-
-	var subkeySig bytes.Buffer
-
-	require.NoError(t, openpgp.ArmoredDetachSign(&subkeySig, withSubkey,
-		bytes.NewReader(data), config))
-
-	subkeyPubKey, err := armoredPublicKey(withSubkey)
-	require.NoError(t, err)
-
-	_, err = verifySignature(data, subkeySig.Bytes(), subkeyPubKey)
-	require.NoError(t, err)
-
 	_, err = verifySignature(data, expiringSig.Bytes(), pubKey)
 	require.ErrorIs(t, err, pgperrors.ErrSignatureExpired)
 }
 
-// signDated returns a detached signature over data by signer, dated when.
-// openpgp.DetachSign signs only with a key that is valid at the time its
-// config gives, so this makes the signature packet without that check.
-func signDated(t *testing.T, signer *packet.PrivateKey, data []byte,
-	when time.Time,
-) []byte {
-	t.Helper()
-
-	sig := &packet.Signature{
-		SigType:      packet.SigTypeBinary,
-		PubKeyAlgo:   signer.PubKeyAlgo,
-		Hash:         crypto.SHA256,
-		CreationTime: when,
-		IssuerKeyId:  &signer.KeyId,
-	}
-
-	h, err := sig.PrepareSign(nil)
-	require.NoError(t, err)
-
-	_, err = h.Write(data)
-	require.NoError(t, err)
-
-	require.NoError(t, sig.Sign(h, signer, nil))
-
-	var buf bytes.Buffer
-
-	require.NoError(t, sig.Serialize(&buf))
-
-	return buf.Bytes()
-}
-
-// TestVerifySignatureRefusesKeyNotValidWhenSigned verifies signatures
-// dated when the key or subkey that made them was not valid: after the key
-// expired; after the key's self-signature, or the signing subkey's binding
-// signature, expired, the key or subkey having expired since as well; and
-// before the key was made, also by a key made in 2100. Each must be
-// refused.
-func TestVerifySignatureRefusesKeyNotValidWhenSigned(t *testing.T) {
-	t.Parallel()
-
-	made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
-	twoDaysLater := made.Add(48 * time.Hour)
-	day := uint32((24 * time.Hour).Seconds())
-	clock := func(when time.Time) func() time.Time {
-		return func() time.Time { return when }
-	}
-
-	expiredAfterADay := newTestKey(t, &packet.Config{
-		Time: clock(made), KeyLifetimeSecs: day,
-	})
-	selfSignatureExpiredAfterADay := newTestKey(t, &packet.Config{
-		Time: clock(made), KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
-	})
-
-	withSubkey := newTestKey(t, &packet.Config{Time: clock(made)})
-	require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
-		Algorithm: packet.PubKeyAlgoEdDSA, Time: clock(made),
-		KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
-	}))
-	subkey := withSubkey.Subkeys[len(withSubkey.Subkeys)-1].PrivateKey
-
-	madeADayLater := newTestKey(t, &packet.Config{
-		Time: clock(made.Add(24 * time.Hour)),
-	})
-	madeIn2100 := newTestKey(t, &packet.Config{
-		Time: clock(time.Date(2100, 1, 1, 0, 0, 0, 0, time.UTC)),
-	})
-
-	for name, test := range map[string]struct {
-		key    *openpgp.Entity
-		signer *packet.PrivateKey
-		when   time.Time
-	}{
-		"after the key expired": {
-			expiredAfterADay, expiredAfterADay.PrivateKey, twoDaysLater,
-		},
-		"after the self-signature expired": {
-			selfSignatureExpiredAfterADay,
-			selfSignatureExpiredAfterADay.PrivateKey, twoDaysLater,
-		},
-		"after the binding signature expired": {
-			withSubkey, subkey, twoDaysLater,
-		},
-		"before the key was made": {
-			madeADayLater, madeADayLater.PrivateKey, made,
-		},
-		"before a key made in 2100": {
-			madeIn2100, madeIn2100.PrivateKey, made,
-		},
-	} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			data := []byte("signed in 2020")
-
-			pubKey, err := armoredPublicKey(test.key)
-			require.NoError(t, err)
-
-			_, err = verifySignature(data,
-				signDated(t, test.signer, data, test.when), pubKey)
-			require.ErrorIs(t, err, errKeyNotValidWhenSigned)
-		})
-	}
-}
-
 func TestManifestSignatureVerification(t *testing.T) {
 	t.Parallel()