simplify unzip; update java error statements

imgbom/cataloger/java/archive_filename.go

@@ -5,6 +5,8 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/anchore/imgbom/internal/log"
+
 	"github.com/anchore/imgbom/imgbom/pkg"
 )
 
@@ -15,25 +17,16 @@ import (
 var versionPattern = regexp.MustCompile(`(?P<name>.+)-(?P<version>(\d+\.)?(\d+\.)?(\*|\d+)(-[a-zA-Z0-9\-\.]+)*)`)
 
 type archiveFilename struct {
-	raw string
+	raw    string
+	fields []map[string]string
 }
 
 func newJavaArchiveFilename(raw string) archiveFilename {
-	return archiveFilename{
-		raw: raw,
-	}
-}
-
-func (a archiveFilename) normalize() string {
 	// trim the file extension and remove any path prefixes
-	return strings.TrimSuffix(filepath.Base(a.raw), "."+a.extension())
+	name := strings.TrimSuffix(filepath.Base(raw), filepath.Ext(raw))
-}
-
-func (a archiveFilename) fields() []map[string]string {
-	name := a.normalize()
 
 	matches := versionPattern.FindAllStringSubmatch(name, -1)
-	items := make([]map[string]string, 0)
+	fields := make([]map[string]string, 0)
 	for _, match := range matches {
 		item := make(map[string]string)
 		for i, name := range versionPattern.SubexpNames() {
@@ -41,9 +34,13 @@ func (a archiveFilename) fields() []map[string]string {
 				item[name] = match[i]
 			}
 		}
-		items = append(items, item)
+		fields = append(fields, item)
+	}
+
+	return archiveFilename{
+		raw:    raw,
+		fields: fields,
 	}
-	return items
 }
 
 func (a archiveFilename) extension() string {
@@ -62,23 +59,21 @@ func (a archiveFilename) pkgType() pkg.Type {
 }
 
 func (a archiveFilename) version() string {
-	fields := a.fields()
+	if len(a.fields) > 1 {
-
+		log.Errorf("discovered multiple name-version pairings from %q: %+v", a.raw, a.fields)
-	// there should be only one version, if there is more or less then something is wrong
+		return ""
-	if len(fields) != 1 {
+	} else if len(a.fields) < 1 {
 		return ""
 	}
 
-	return fields[0]["version"]
+	return a.fields[0]["version"]
 }
 
 func (a archiveFilename) name() string {
-	fields := a.fields()
-
 	// there should be only one name, if there is more or less then something is wrong
-	if len(fields) != 1 {
+	if len(a.fields) != 1 {
 		return ""
 	}
 
-	return fields[0]["name"]
+	return a.fields[0]["name"]
 }

imgbom/cataloger/java/java_manifest.go

@@ -54,7 +54,7 @@ func parseJavaManifest(reader io.Reader) (*pkg.JavaManifest, error) {
 	}
 
 	if err := mapstructure.Decode(manifestMap, &manifest); err != nil {
-		return nil, fmt.Errorf("unable parse java manifest: %w", err)
+		return nil, fmt.Errorf("unable to parse java manifest: %w", err)
 	}
 
 	return &manifest, nil
@@ -73,7 +73,7 @@ func newPackageFromJavaManifest(virtualPath, archivePath string, fileManifest fi
 	// fetch the manifest file
 	contents, err := file.ExtractFilesFromZip(archivePath, manifestMatches...)
 	if err != nil {
-		return nil, fmt.Errorf("unable to extract java manifests: %w", err)
+		return nil, fmt.Errorf("unable to extract java manifests (%s): %w", virtualPath, err)
 	}
 
 	// parse the manifest file into a rich object

internal/file/ziputil.go

@@ -83,7 +83,6 @@ func ExtractFilesFromZip(archivePath string, paths ...string) (map[string]string
 	return results, nil
 }
 
-// nolint:funlen
 func UnzipToDir(archivePath, targetDir string) error {
 	zipReader, err := zip.OpenReader(archivePath)
 	if err != nil {
@@ -106,46 +105,54 @@ func UnzipToDir(archivePath, targetDir string) error {
 			return fmt.Errorf("potential zip slip attack: %q", expandedFilePath)
 		}
 
-		zippedFile, err := file.Open()
+		err = extractSingleFile(file, expandedFilePath, archivePath)
 		if err != nil {
-			return fmt.Errorf("unable to read file=%q from zip=%q: %w", file.Name, archivePath, err)
+			return err
 		}
+	}
+	return nil
+}
 
-		if file.FileInfo().IsDir() {
+func extractSingleFile(file *zip.File, expandedFilePath, archivePath string) error {
-			err = os.MkdirAll(expandedFilePath, file.Mode())
+	zippedFile, err := file.Open()
-			if err != nil {
+	if err != nil {
-				return fmt.Errorf("unable to create dir=%q from zip=%q: %w", expandedFilePath, archivePath, err)
+		return fmt.Errorf("unable to read file=%q from zip=%q: %w", file.Name, archivePath, err)
-			}
+	}
-		} else {
-			// Open an output file for writing
-			outputFile, err := os.OpenFile(
-				expandedFilePath,
-				os.O_WRONLY|os.O_CREATE|os.O_TRUNC,
-				file.Mode(),
-			)
-			if err != nil {
-				return fmt.Errorf("unable to create dest file=%q from zip=%q: %w", expandedFilePath, archivePath, err)
-			}
 
-			// limit the zip reader on each file read to prevent decompression bomb attacks
+	if file.FileInfo().IsDir() {
-			numBytes, err := io.Copy(outputFile, io.LimitReader(zippedFile, readLimit))
+		err = os.MkdirAll(expandedFilePath, file.Mode())
-			if numBytes >= readLimit || errors.Is(err, io.EOF) {
-				return fmt.Errorf("zip read limit hit (potential decompression bomb attack)")
-			}
-			if err != nil {
-				return fmt.Errorf("unable to copy source=%q to dest=%q for zip=%q: %w", file.Name, outputFile.Name(), archivePath, err)
-			}
-
-			err = outputFile.Close()
-			if err != nil {
-				return fmt.Errorf("unable to close dest file=%q from zip=%q: %w", outputFile.Name(), archivePath, err)
-			}
-		}
-
-		err = zippedFile.Close()
 		if err != nil {
-			return fmt.Errorf("unable to close source file=%q from zip=%q: %w", file.Name, archivePath, err)
+			return fmt.Errorf("unable to create dir=%q from zip=%q: %w", expandedFilePath, archivePath, err)
 		}
+	} else {
+		// Open an output file for writing
+		outputFile, err := os.OpenFile(
+			expandedFilePath,
+			os.O_WRONLY|os.O_CREATE|os.O_TRUNC,
+			file.Mode(),
+		)
+		if err != nil {
+			return fmt.Errorf("unable to create dest file=%q from zip=%q: %w", expandedFilePath, archivePath, err)
+		}
+
+		// limit the zip reader on each file read to prevent decompression bomb attacks
+		numBytes, err := io.Copy(outputFile, io.LimitReader(zippedFile, readLimit))
+		if numBytes >= readLimit || errors.Is(err, io.EOF) {
+			return fmt.Errorf("zip read limit hit (potential decompression bomb attack)")
+		}
+		if err != nil {
+			return fmt.Errorf("unable to copy source=%q to dest=%q for zip=%q: %w", file.Name, outputFile.Name(), archivePath, err)
+		}
+
+		err = outputFile.Close()
+		if err != nil {
+			return fmt.Errorf("unable to close dest file=%q from zip=%q: %w", outputFile.Name(), archivePath, err)
+		}
+	}
+
+	err = zippedFile.Close()
+	if err != nil {
+		return fmt.Errorf("unable to close source file=%q from zip=%q: %w", file.Name, archivePath, err)
 	}
 	return nil
 }