Detect invalid license expressions containing () (#879)

The following condensed list of expressions are all currently
mislabelled as valid by `canonicalize_license_expression()`.

    ( ) or MIT
    MIT and ( )
    ( ) and MIT or MIT
    ( ) with ( MIT )
    ( ) with ( ) or MIT
    MIT with ( ) with ( ) or MIT

The trick of converting license expressions to Python statements runs
afoul of `()` being an empty tuple (i.e. valid syntax and falsy).
Depending on how `()` is combined with real `False`s, it may or may not
get caught by the subsequent `eval(python_expression) is False` check:

    MIT or ()  ->  False or ()  ->  ()     # Caught
    () or MIT  ->  () or False  ->  False  # Not caught

Consider a license expression invalid if a `)` comes immediately after a
`(` token. This also removes the one case where the `eval()` would
return something other than `False` so that `eval()` can now be
downgraded to a `compile()` (which I find very anxiety relieving even
though I can't think of any way the old `eval()` could ever have been
exposed to more than `False`s or `()`s).

Author: bwoodsend

src/packaging/licenses/__init__.py

@@ -80,9 +80,10 @@ def canonicalize_license_expression(
 
     tokens = license_expression.split()
 
-    # Rather than implementing boolean logic, we create an expression that Python can
-    # parse. Everything that is not involved with the grammar itself is treated as
-    # `False` and the expression should evaluate as such.
+    # Rather than implementing a parenthesis/boolean logic parser, create an
+    # expression that Python can parse. Everything that is not involved with the
+    # grammar itself is replaced with the placeholder `False` and the resultant
+    # expression should become a valid Python expression.
     python_tokens = []
     for token in tokens:
         if token not in {"or", "and", "with", "(", ")"}:
@@ -92,16 +93,16 @@ def canonicalize_license_expression(
         elif token == "(" and python_tokens and python_tokens[-1] not in {"or", "and"}:
             message = f"Invalid license expression: {raw_license_expression!r}"
             raise InvalidLicenseExpression(message)
+        elif token == ")" and python_tokens and python_tokens[-1] == "(":
+            message = f"Invalid license expression: {raw_license_expression!r}"
+            raise InvalidLicenseExpression(message)
         else:
             python_tokens.append(token)
 
     python_expression = " ".join(python_tokens)
     try:
-        invalid = eval(python_expression, globals(), locals())
-    except Exception:
-        invalid = True
-
-    if invalid is not False:
+        compile(python_expression, "", "eval")
+    except SyntaxError:
         message = f"Invalid license expression: {raw_license_expression!r}"
         raise InvalidLicenseExpression(message) from None
 

tests/test_metadata.py

@@ -710,11 +710,20 @@ def test_valid_license_expression(self, license_expression, expected):
             "with mit",
             "(mit",
             "mit)",
+            ") mit",
+            "mit (",
             "mit or or apache-2.0",
             # Missing an operator before `(`.
             "mit or apache-2.0 (bsd-3-clause and MPL-2.0)",
             # "2-BSD-Clause is not a valid license.
             "Apache-2.0 OR 2-BSD-Clause",
+            # Empty parenthesis.
+            "()",
+            "( ) or mit",
+            "mit and ( )",
+            "( ) or mit and ( )",
+            "( ) with ( ) or mit",
+            "mit with ( ) with ( ) or mit",
         ],
     )
     def test_invalid_license_expression(self, license_expression):