From d95abbf6b5209459b9c260eea763ea44c96d50e9 Mon Sep 17 00:00:00 2001 From: Keith Zantow Date: Wed, 30 Sep 2026 15:52:45 -0400 Subject: [PATCH] chore: truncation reason to error type Signed-off-by: Keith Zantow --- internal/archive/extract.go | 6 +++--- internal/archive/extract_test.go | 34 +++++++++++++++---------------- internal/archive/resolver.go | 17 ++++++++-------- internal/archive/resolver_test.go | 4 ++-- internal/task/archive_tasks.go | 5 ++--- 5 files changed, 32 insertions(+), 34 deletions(-) diff --git a/internal/archive/extract.go b/internal/archive/extract.go index 801a6efa8..004b72e9f 100644 --- a/internal/archive/extract.go +++ b/internal/archive/extract.go @@ -149,16 +149,16 @@ func (r *Resolver) extract(ctx context.Context, format archives.Extractor, conte }) r.finish() if errors.Is(err, ErrDiskLimitReached) { - r.truncate("extraction stopped at the configured memory or disk limit") + r.truncate(fmt.Errorf("extraction stopped: %w", err)) return nil } if errors.Is(err, errBudgetSpent) { - r.truncate("extraction stopped: the archive and those nested in it decompressed to far more than its size (possible decompression bomb)") + r.truncate(fmt.Errorf("extraction stopped, possible decompression bomb: %w", err)) return nil } if err != nil && ctx.Err() == nil && len(r.files) > 0 { // the stream broke partway (a truncated .tar.gz); what was read before that is still good - r.truncate(fmt.Sprintf("extraction stopped early: %v", err)) + r.truncate(fmt.Errorf("extraction stopped early: %w", err)) return nil } return err diff --git a/internal/archive/extract_test.go b/internal/archive/extract_test.go index 70456a54f..d898d13f2 100644 --- a/internal/archive/extract_test.go +++ b/internal/archive/extract_test.go @@ -69,7 +69,7 @@ func TestResolver_extract_storesEveryEntryInMemoryWhenUnbounded(t *testing.T) { } { t.Run(name, func(t *testing.T) { r, root := extractBytes(t, data, name, nil) - assert.False(t, r.Truncated) + assert.NoError(t, r.Truncated) assert.Zero(t, r.written) assert.Empty(t, filesIn(t, root)) @@ -87,7 +87,7 @@ func TestResolver_extract_diskLimitTruncatesAtTheEntryThatDoesNotFit(t *testing. r, _ := extractBytes(t, data, "test.zip", diskCharge(2*(record+1)+record)) - assert.True(t, r.Truncated) + assert.Error(t, r.Truncated) assert.Equal(t, []string{"file1.txt", "file2.txt"}, storedNames(t, r), "the refused entry is not stored") entries := readStore(t, r) assert.Equal(t, "a", entries["file1.txt"].body) @@ -105,7 +105,7 @@ func TestResolver_extract_diskLimitBitesMidEntry(t *testing.T) { r, root := extractBytes(t, data, "test.tar.gz", charge) - assert.True(t, r.Truncated) + assert.Error(t, r.Truncated) assert.Equal(t, []string{"a/first.txt"}, storedNames(t, r)) assert.Greater(t, r.written, int64(40*1024), "the first entry landed whole and the second part way") _, disk := charge.held() @@ -123,11 +123,11 @@ func TestResolver_extract_diskLimitFallsWhenAnArchiveIsReleased(t *testing.T) { limiter := NewLimiter(Limits{MaxDiskBytes: oneArchive * 3 / 2}) first, _ := extractBytes(t, data, "test.zip", limiter.charge()) - require.False(t, first.Truncated) + require.NoError(t, first.Truncated) require.Equal(t, int64(200), first.written) second, _ := extractBytes(t, data, "test.zip", limiter.charge()) - assert.True(t, second.Truncated, "a second archive is bounded while the first is still held") + assert.Error(t, second.Truncated, "a second archive is bounded while the first is still held") second.Cleanup() first.Cleanup() @@ -135,7 +135,7 @@ func TestResolver_extract_diskLimitFallsWhenAnArchiveIsReleased(t *testing.T) { require.Zero(t, disk) third, _ := extractBytes(t, data, "test.zip", limiter.charge()) - assert.False(t, third.Truncated, "and extracts in full once the first is released") + assert.NoError(t, third.Truncated, "and extracts in full once the first is released") assert.Equal(t, int64(200), third.written) } @@ -152,7 +152,7 @@ func TestResolver_extract_directoryOnlyArchivesAreBoundedByTheIndexEstimate(t *t } { t.Run(name, func(t *testing.T) { r, _ := extractBytes(t, data, name, diskCharge(room)) - assert.True(t, r.Truncated) + assert.Error(t, r.Truncated) assert.Len(t, storedNames(t, r), 10) }) } @@ -175,7 +175,7 @@ func TestResolver_extract_entryNamesAreCleanedAndNothingIsWrittenUnderThem(t *te } { t.Run(name, func(t *testing.T) { r, root := extractBytes(t, data, name, nil) - require.False(t, r.Truncated) + require.NoError(t, r.Truncated) assert.Equal(t, []string{"META-INF", "META-INF/MANIFEST.MF", "bin/link.txt", "etc/hosts", "etc/passwd", "passwd", "shadow"}, storedNames(t, r)) @@ -205,7 +205,7 @@ func TestResolver_extract_dataEdgeCases(t *testing.T) { t.Run("an empty archive is not an error", func(t *testing.T) { r, _ := extractBytes(t, zipBytes(t, map[string]string{}), "test.zip", nil) assert.Empty(t, r.files) - assert.False(t, r.Truncated) + assert.NoError(t, r.Truncated) }) t.Run("a zero-byte entry costs no disk", func(t *testing.T) { @@ -251,7 +251,7 @@ func TestResolver_extract_excludedEntriesAreNeverStored(t *testing.T) { err := r.extract(context.Background(), identifyFormat(context.Background(), "test.zip", content), content, NewExclusions([]string{"**/vendor", "**/*.rpm"})) require.NoError(t, err) - require.False(t, r.Truncated) + require.NoError(t, r.Truncated) assert.Equal(t, []string{"keep.txt"}, storedNames(t, r)) mem, _ := limiter.InUse() @@ -266,7 +266,7 @@ func TestExtract_readsEntriesAndDigestsTheArchive(t *testing.T) { require.NotNil(t, extracted) t.Cleanup(extracted.Cleanup) - assert.False(t, extracted.Truncated) + assert.NoError(t, extracted.Truncated) require.Len(t, extracted.Digests, 1) assert.Equal(t, "sha1", extracted.Digests[0].Algorithm) @@ -358,7 +358,7 @@ func TestExtract_aTruncatedStreamKeepsWhatWasRead(t *testing.T) { require.NotNil(t, extracted) t.Cleanup(extracted.Cleanup) - assert.True(t, extracted.Truncated) + assert.Error(t, extracted.Truncated) locations, err := extracted.FilesByGlob("**") require.NoError(t, err) assert.Equal(t, []string{"a.txt"}, realPaths(locations)) @@ -404,7 +404,7 @@ func TestExtract_archiveBytesThatCannotBePlacedAreSkipped(t *testing.T) { require.NoError(t, err) require.NotNil(t, extracted) t.Cleanup(extracted.Cleanup) - assert.True(t, extracted.Truncated) + assert.Error(t, extracted.Truncated) locations, err := extracted.FilesByGlob("**/*") require.NoError(t, err) assert.Empty(t, locations) @@ -422,7 +422,7 @@ func TestExtract_aZipBehindALauncherScript(t *testing.T) { require.NotNil(t, extracted) t.Cleanup(extracted.Cleanup) - assert.False(t, extracted.Truncated) + assert.NoError(t, extracted.Truncated) locations, err := extracted.FilesByGlob("**/*") require.NoError(t, err) assert.Len(t, locations, 2) @@ -525,7 +525,7 @@ func TestExtract_decompressionBombIsBoundedByTheLimits(t *testing.T) { require.NotNil(t, r) t.Cleanup(r.Cleanup) - assert.True(t, r.Truncated) + assert.Error(t, r.Truncated) locations, err := r.FilesByGlob("**/*") require.NoError(t, err) assert.Empty(t, locations, "the entry that did not fit is not cataloged in part") @@ -550,7 +550,7 @@ func TestExtract_entryCountBombIsBoundedByTheMemoryLimit(t *testing.T) { require.NoError(t, err) t.Cleanup(r.Cleanup) - assert.True(t, r.Truncated) + assert.Error(t, r.Truncated) assert.Less(t, len(r.files), 20_000) assert.LessOrEqual(t, int64(len(r.byPath))*approxIndexBytesPerEntry, limits.MaxMemoryBytes+approxIndexBytesPerEntry, "nodes held must not exceed what the memory limit paid for") @@ -578,7 +578,7 @@ func TestExtract_deepPathBombIsBoundedByTheMemoryLimit(t *testing.T) { require.NoError(t, err) t.Cleanup(r.Cleanup) - assert.True(t, r.Truncated) + assert.Error(t, r.Truncated) assert.LessOrEqual(t, int64(len(r.byPath))*approxIndexBytesPerEntry, limits.MaxMemoryBytes+approxIndexBytesPerEntry) peakMemory, _ := limiter.Peak() assert.LessOrEqual(t, peakMemory, limits.MaxMemoryBytes) diff --git a/internal/archive/resolver.go b/internal/archive/resolver.go index 83f3e0efc..5550914e7 100644 --- a/internal/archive/resolver.go +++ b/internal/archive/resolver.go @@ -34,10 +34,9 @@ type Resolver struct { // Digests are of the archive file itself. Digests []file.Digest - // Truncated reports that the resolver covers only part of the archive, and TruncatedReason says why: - // the disk limit or the decompression budget stopped extraction early, or an entry was too large. - Truncated bool - TruncatedReason string + // Truncated is non-nil when the resolver covers only part of the archive, and says why: the disk limit + // or the decompression budget stopped extraction early, or an entry was too large or unreadable. + Truncated error fileSystemID string archivePath string @@ -164,14 +163,14 @@ func (r *Resolver) add(hdr tar.Header, content io.Reader) error { delete(r.byPath, entryPath) } if errors.Is(err, errEntryTooLarge) { - r.truncate(fmt.Sprintf("an entry larger than %s was skipped", humanize.IBytes(uint64(maxEntryBytes)))) + r.truncate(fmt.Errorf("an entry was skipped: %w of %s", err, humanize.IBytes(uint64(maxEntryBytes)))) return nil } if errors.Is(err, ErrDiskLimitReached) || errors.Is(err, errBudgetSpent) { return err } // a damaged entry (a CRC mismatch, a corrupt deflate stream) costs only that entry - r.truncate(fmt.Sprintf("entry %q could not be read: %v", hdr.Name, err)) + r.truncate(fmt.Errorf("entry %q could not be read: %w", hdr.Name, err)) return nil } } @@ -195,9 +194,9 @@ var maxEntryBytes int64 = intFile.PerFileReadLimit var errEntryTooLarge = errors.New("archive entry exceeds the per-entry size cap") // truncate marks the resolver as covering part of the archive, keeping the first reason given. -func (r *Resolver) truncate(reason string) { - if !r.Truncated { - r.Truncated, r.TruncatedReason = true, reason +func (r *Resolver) truncate(err error) { + if r.Truncated == nil { + r.Truncated = err } } diff --git a/internal/archive/resolver_test.go b/internal/archive/resolver_test.go index 2c691f66b..9c335bbeb 100644 --- a/internal/archive/resolver_test.go +++ b/internal/archive/resolver_test.go @@ -525,8 +525,8 @@ func TestResolver_add_anEntryOverTheCapIsSkippedAndReadNoFurther(t *testing.T) { r.finish() assert.LessOrEqual(t, endless.n, 1024+copyChunkSize, "reading stops just past the cap") - assert.True(t, r.Truncated) - assert.Contains(t, r.TruncatedReason, "an entry larger than 1.0 KiB was skipped") + assert.ErrorIs(t, r.Truncated, errEntryTooLarge) + assert.ErrorContains(t, r.Truncated, "per-entry size cap of 1.0 KiB") locations, err := r.FilesByGlob("**") require.NoError(t, err) assert.Equal(t, []string{"ok.txt"}, realPaths(locations)) diff --git a/internal/task/archive_tasks.go b/internal/task/archive_tasks.go index 4f023546c..f3fec0a97 100644 --- a/internal/task/archive_tasks.go +++ b/internal/task/archive_tasks.go @@ -1,7 +1,6 @@ package task import ( - "cmp" "context" "errors" "fmt" @@ -231,8 +230,8 @@ func (c *archiveCataloger) processArchive(ctx context.Context, parentResolver fi c.ancestors = append(c.ancestors, extracted.Digests...) defer func() { c.ancestors = c.ancestors[:len(c.ancestors)-len(extracted.Digests)] }() - if extracted.Truncated { - recordArchiveUnknown(builder, location.Coordinates, "archive cataloged from part of its contents: "+extracted.TruncatedReason) + if extracted.Truncated != nil { + recordArchiveUnknown(builder, location.Coordinates, "archive cataloged from part of its contents: "+extracted.Truncated.Error()) } traversal.Digests = extracted.Digests