Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split up archive.go into type-specific files; add wider zlib support #723

Merged
merged 2 commits into from
Dec 18, 2024

Conversation

egibs
Copy link
Member

@egibs egibs commented Dec 18, 2024

This PR splits up all of the archive-specific functions in archive.go into their own respective files for readability.

As part of this, I also added extension-agnostic zlib support depending on whether the file returns &{.Z, application/zlib} from programkind.File.

I also fixed a panic:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xd648e6]

goroutine 19400 [running]:
github.com/chainguard-dev/malcontent/pkg/action.extractGzip({0x2b5ce48?, 0xc0000ff6b0?}, {0xc001d680c0, 0x39}, {0xc0009b8240, 0x85})
	github.com/chainguard-dev/[email protected]/pkg/action/archive.go:249 +0x2c6
github.com/chainguard-dev/malcontent/pkg/action.extractArchiveToTempDir({0x2b5ce48, 0xc0000ff6b0}, {0xc0009b8240, 0x85})
	github.com/chainguard-dev/[email protected]/pkg/action/archive.go:653 +0x383
github.com/chainguard-dev/malcontent/pkg/action.processArchive({0x2b5ce48, 0xc0000ff6b0}, {0x8, 0x0, 0x0, 0x0, 0x0, 0x1, {0x0, 0x0, ...}, ...}, ...)
	github.com/chainguard-dev/[email protected]/pkg/action/scan.go:496 +0x134
github.com/chainguard-dev/malcontent/pkg/action.recursiveScan.func1({0xc0009b8240, 0x85})
	github.com/chainguard-dev/[email protected]/pkg/action/scan.go:328 +0xd0
github.com/chainguard-dev/malcontent/pkg/action.recursiveScan.func4()
	github.com/chainguard-dev/[email protected]/pkg/action/scan.go:436 +0xa2
golang.org/x/sync/errgroup.(*Group).Go.func1()
	golang.org/x/[email protected]/errgroup/errgroup.go:78 +0x50
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 19398
	golang.org/x/[email protected]/errgroup/errgroup.go:75 +0x96

This was happening because we were trying to print ft.Ext debug logs for files that returned nil from programkind.File.

@egibs egibs requested a review from tstromberg December 18, 2024 14:32
func extractNestedArchive(
ctx context.Context,
d string,
f string,
extracted *sync.Map,
) error {
isArchive := false
ext := getExt(f)
if _, ok := archiveMap[ext]; ok {
// zlib-compressed files are also archives
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines, L101-112, L152-162, and the additions in zlib.go are the only material changes in this PR. The rest are just moving existing code into new files (and updating the debug logging in gzip.go.

@tstromberg
Copy link
Collaborator

It looks good; what do you think about moving this out of action and into an archive package? It feels like archive handling has grown up into its own feature at this point.

@egibs
Copy link
Member Author

egibs commented Dec 18, 2024

It looks good; what do you think about moving this out of action and into an archive package? It feels like archive handling has grown up into its own feature at this point.

I ran into some circular imports earlier but I can give it a try.

@egibs
Copy link
Member Author

egibs commented Dec 18, 2024

It looks good; what do you think about moving this out of action and into an archive package? It feels like archive handling has grown up into its own feature at this point.

Updated in 43b922d (#723).

I left archive_test.go in action since it uses Scan and CachedRules. I also moved the extension and archive map code to programkind since that seemed like a better fit.

@egibs egibs merged commit f18fb0e into chainguard-dev:main Dec 18, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants