From a1cb37e8c3c2962f208cf3e3c25800a994f103e9 Mon Sep 17 00:00:00 2001
From: Sebastian Schuberth <sebastian@doubleopen.org>
Date: Wed, 14 Feb 2024 18:44:03 +0100
Subject: [PATCH] fix(spdx-utils): Do not test for sub-expressions based on
 strings

The string-approach fails for compound expressions whose second operand
ends with an exception (the `WITH` operator binds stronger than `AND` /
`OR`). As the string comparison was meant as a performance optimization
any anyway, just remove it.

In exchange, a special case for the `AND` operator is required, for
which the `containsAll()` check with valid choices is be too strict: All
valid choices for an `AND` expression would contain the `AND` itself.
However, sub-expressions should be allowed to also match only either side
of the `AND`.

Signed-off-by: Sebastian Schuberth <sebastian@doubleopen.org>
---
 utils/spdx/src/main/kotlin/SpdxExpression.kt     |  7 ++++---
 utils/spdx/src/test/kotlin/SpdxExpressionTest.kt | 15 +++++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/utils/spdx/src/main/kotlin/SpdxExpression.kt b/utils/spdx/src/main/kotlin/SpdxExpression.kt
index f83a36c6e317e..de3c8b2c3026a 100644
--- a/utils/spdx/src/main/kotlin/SpdxExpression.kt
+++ b/utils/spdx/src/main/kotlin/SpdxExpression.kt
@@ -318,10 +318,11 @@ class SpdxCompoundExpression(
     override fun isSubExpression(subExpression: SpdxExpression?): Boolean {
         if (subExpression == null) return false
 
-        val expressionString = toString()
-        val subExpressionString = subExpression.toString()
+        if (operator == SpdxOperator.AND) {
+            if (left.isSubExpression(subExpression) || right.isSubExpression(subExpression)) return true
+        }
 
-        return subExpressionString in expressionString || validChoices().containsAll(subExpression.validChoices())
+        return validChoices().containsAll(subExpression.validChoices())
     }
 
     override fun equals(other: Any?): Boolean {
diff --git a/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt b/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt
index d65f98b062941..9dec2a3a264ee 100644
--- a/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt
+++ b/utils/spdx/src/test/kotlin/SpdxExpressionTest.kt
@@ -657,6 +657,21 @@ class SpdxExpressionTest : WordSpec({
 
             mit.isSubExpression(mit) shouldBe true
         }
+
+        "work correctly for compound expressions with exceptions" {
+            val gplWithException = "CDDL-1.1 OR GPL-2.0-only WITH Classpath-exception-2.0".toSpdx()
+            val gpl = "CDDL-1.1 OR GPL-2.0-only".toSpdx()
+
+            gplWithException.isSubExpression(gpl) shouldBe false
+        }
+
+        "work correctly for nested compound expressions" {
+            val expression = "(CDDL-1.1 OR GPL-2.0-only) AND (CDDL-1.1 OR GPL-2.0-only WITH Classpath-exception-2.0)"
+                .toSpdx()
+            val subExpression = "CDDL-1.1 OR GPL-2.0-only".toSpdx()
+
+            expression.isSubExpression(subExpression) shouldBe true
+        }
     }
 
     "applyChoice()" should {