From 092c374dd9a9dd23681604b527f2c8fae1db39e2 Mon Sep 17 00:00:00 2001
From: Will Murphy <willmurphyscode@users.noreply.github.com>
Date: Tue, 10 Dec 2024 16:45:45 -0500
Subject: [PATCH] fix: stop omitting redundantly parenthesized licenses in CDX
 formatter

Previously, a bug in the formatter would cause SPDX expressions that
were surrounded in redundant parentheses to be dropped instead of
normalized.

Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>
---
 .../cyclonedxutil/helpers/licenses.go         | 49 ++++++++++---------
 .../cyclonedxutil/helpers/licenses_test.go    | 23 +++++++++
 syft/license/license_test.go                  |  5 ++
 3 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/syft/format/internal/cyclonedxutil/helpers/licenses.go b/syft/format/internal/cyclonedxutil/helpers/licenses.go
index 24c5d6ffb79..4f5f3f7f2a0 100644
--- a/syft/format/internal/cyclonedxutil/helpers/licenses.go
+++ b/syft/format/internal/cyclonedxutil/helpers/licenses.go
@@ -1,7 +1,6 @@
 package helpers
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/CycloneDX/cyclonedx-go"
@@ -158,40 +157,42 @@ func mergeSPDX(ex []string) string {
 		// if the expression does not have balanced parens add them
 		if !strings.HasPrefix(e, "(") && !strings.HasSuffix(e, ")") {
 			e = "(" + e + ")"
-			candidate = append(candidate, e)
 		}
+		candidate = append(candidate, e)
 	}
 
 	if len(candidate) == 1 {
-		return reduceOuter(strings.Join(candidate, " AND "))
+		return reduceOuter(candidate[0])
 	}
 
-	return strings.Join(candidate, " AND ")
+	return reduceOuter(strings.Join(candidate, " AND "))
 }
 
 func reduceOuter(expression string) string {
-	var (
-		sb        strings.Builder
-		openCount int
-	)
+	expression = strings.TrimSpace(expression)
 
-	for _, c := range expression {
-		if string(c) == "(" && openCount > 0 {
-			_, _ = fmt.Fprintf(&sb, "%c", c)
-		}
-		if string(c) == "(" {
-			openCount++
-			continue
-		}
-		if string(c) == ")" && openCount > 1 {
-			_, _ = fmt.Fprintf(&sb, "%c", c)
-		}
-		if string(c) == ")" {
-			openCount--
-			continue
+	// If the entire expression is wrapped in parentheses, check if they are redundant.
+	if strings.HasPrefix(expression, "(") && strings.HasSuffix(expression, ")") {
+		trimmed := expression[1 : len(expression)-1]
+		if isBalanced(trimmed) {
+			return reduceOuter(trimmed) // Recursively reduce the trimmed expression.
 		}
-		_, _ = fmt.Fprintf(&sb, "%c", c)
 	}
 
-	return sb.String()
+	return expression
+}
+
+func isBalanced(expression string) bool {
+	count := 0
+	for _, c := range expression {
+		if c == '(' {
+			count++
+		} else if c == ')' {
+			count--
+			if count < 0 {
+				return false
+			}
+		}
+	}
+	return count == 0
 }
diff --git a/syft/format/internal/cyclonedxutil/helpers/licenses_test.go b/syft/format/internal/cyclonedxutil/helpers/licenses_test.go
index 373a0937758..737555d6325 100644
--- a/syft/format/internal/cyclonedxutil/helpers/licenses_test.go
+++ b/syft/format/internal/cyclonedxutil/helpers/licenses_test.go
@@ -164,6 +164,29 @@ func Test_encodeLicense(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "single parenthesized SPDX expression",
+			input: pkg.Package{
+				Licenses: pkg.NewLicenseSet(pkg.NewLicensesFromValues("(MIT OR Apache-2.0)")...),
+			},
+			expected: &cyclonedx.Licenses{
+				{
+					Expression: "MIT OR Apache-2.0",
+				},
+			},
+		},
+		{
+			name: "single license AND to parenthesized SPDX expression",
+			// (LGPL-3.0-or-later OR GPL-2.0-or-later OR (LGPL-3.0-or-later AND GPL-2.0-or-later)) AND GFDL-1.3-invariants-or-later
+			input: pkg.Package{
+				Licenses: pkg.NewLicenseSet(pkg.NewLicensesFromValues("(LGPL-3.0-or-later OR GPL-2.0-or-later OR (LGPL-3.0-or-later AND GPL-2.0-or-later)) AND GFDL-1.3-invariants-or-later")...),
+			},
+			expected: &cyclonedx.Licenses{
+				{
+					Expression: "(LGPL-3.0-or-later OR GPL-2.0-or-later OR (LGPL-3.0-or-later AND GPL-2.0-or-later)) AND GFDL-1.3-invariants-or-later",
+				},
+			},
+		},
 		// TODO: do we drop the non SPDX ID license and do a single expression
 		// OR do we keep the non SPDX ID license and do multiple licenses where the complex
 		// expressions are set as the NAME field?
diff --git a/syft/license/license_test.go b/syft/license/license_test.go
index 6387f38da18..2641d39e5b9 100644
--- a/syft/license/license_test.go
+++ b/syft/license/license_test.go
@@ -20,6 +20,11 @@ func TestParseExpression(t *testing.T) {
 			expression: "MIT OR Apache-2.0",
 			want:       "MIT OR Apache-2.0",
 		},
+		{
+			name:       "accept redundant parentheses",
+			expression: "(MIT OR Apache-2.0)",
+			want:       "(MIT OR Apache-2.0)",
+		},
 		{
 			name:       "Invalid SPDX expression returns error",
 			expression: "MIT OR Apache-2.0 OR invalid",