Invalid chars included in SPDX declared licenses

Tern was including commas and other invalid characters in SPDX license
expressions which resulted in failed validation for generated SPDX docs.
This commit removes the inappropriate characters and
replaces them with valid ones (i.e. swapping `&` for `and`) before
checking if a license expression is valid. It also updates the related
function descriptions and comments for clarity.

Fixes: #1223

Signed-off-by: Rose Judge <[email protected]>

Author: rnjudge

tern/formats/spdx/spdx_common.py

@@ -46,8 +46,10 @@ def get_license_ref(license_string):
     return 'LicenseRef-' + get_string_id(license_string)
 
 def is_spdx_license_expression(license_data):
-    '''Return True if the license is a valid SPDX license expression, else
-    return False'''
+    '''Given a license string, replace common invalid SPDX license characters before
+    checking if the string is a valid SPDX license expression. Return True/False 
+    and also return the updated license expression string.
+    If case of error, return False with a blank string.'''
     licensing = get_spdx_licensing()
     not_allowed = [',', ';', '/', '&']
     if any(x in license_data for x in not_allowed):
@@ -57,18 +59,22 @@ def is_spdx_license_expression(license_data):
         license_data = license_data.replace(';', '.')
         license_data = license_data.replace('&', 'and')
     try:
-        return licensing.validate(license_data).errors == []
+        return licensing.validate(license_data).errors == [], license_data
     # Catch any of the other invalid license chars here
     except AttributeError:
-        return False
+        return False, ''
 
 def get_package_license_declared(package_license_declared):
-    '''Determines if the declared license string for a package or file is a
-    valid SPDX license expression using the license_expression library. If not,
-    returns the SPDX LicenseRef or NONE if the string is blank.'''
+    '''After substituting common invalid SPDX license characters using
+    the is_spdx_license_expression() function, determines if the declared
+    license string for a package or file is a valid SPDX license expression.
+    If license expression is valid after substitutions, return the updated string.
+    If not, return the LicenseRef of the original declared license expression
+    passed in to the function. If a blank string is passed in, return `NONE`.'''
     if package_license_declared:
-        if is_spdx_license_expression(package_license_declared):
-            return package_license_declared
+        valid_spdx, license_expression = is_spdx_license_expression(package_license_declared)
+        if valid_spdx:
+            return license_expression
         return get_license_ref(package_license_declared)
     return 'NONE'
 

tern/formats/spdx/spdxjson/image_helpers.py

@@ -34,7 +34,8 @@ def get_image_extracted_licenses(image_obj):
                 unique_licenses.add(", ".join(package.pkg_licenses))
     extracted_texts = []
     for lic in list(unique_licenses):
-        if not spdx_common.is_spdx_license_expression(lic):
+        valid_spdx, _ = spdx_common.is_spdx_license_expression(lic)
+        if not valid_spdx:
             extracted_texts.append(json_formats.get_extracted_text_dict(
                 extracted_text=lic, license_ref=spdx_common.get_license_ref(
                     lic)))

tern/formats/spdx/spdxjson/layer_helpers.py

@@ -39,7 +39,8 @@ def get_layer_extracted_licenses(layer_obj):
             unique_licenses.add(package.pkg_license)
     extracted_texts = []
     for lic in list(unique_licenses):
-        if not spdx_common.is_spdx_license_expression(lic):
+        valid_spdx, _ = spdx_common.is_spdx_license_expression(lic)
+        if not valid_spdx:
             extracted_texts.append(json_formats.get_extracted_text_dict(
                 extracted_text=lic, license_ref=spdx_common.get_license_ref(
                     lic)))

tern/formats/spdx/spdxjson/package_helpers.py

@@ -49,7 +49,6 @@ def get_source_package_dict(package, template):
         mapping['PackageCopyrightText'] else 'NONE',
         'comment': json_formats.source_package_comment
     }
-
     return package_dict
 
 
@@ -60,6 +59,10 @@ def get_package_dict(package, template):
     mapping = package.to_dict(template)
     supplier_str = 'Organization: ' + mapping['PackageSupplier']
     pkg_ref, _ = spdx_common.get_package_spdxref(package)
+    # Define debian licenses from copyright text as one license
+    declared_lic = mapping['PackageLicenseDeclared']
+    if package.pkg_format == 'deb':
+        declared_lic = ', '.join(package.pkg_licenses)
     package_dict = {
         'name': mapping['PackageName'],
         'SPDXID': pkg_ref,
@@ -71,7 +74,7 @@ def get_package_dict(package, template):
         'filesAnalyzed': False,  # always false for packages
         'licenseConcluded': 'NOASSERTION',  # always NOASSERTION
         'licenseDeclared':  spdx_common.get_package_license_declared(
-            mapping['PackageLicenseDeclared']),
+            declared_lic),
         'copyrightText': mapping['PackageCopyrightText'] if
         mapping['PackageCopyrightText'] else 'NONE',
     }

tern/formats/spdx/spdxtagvalue/image_helpers.py

@@ -60,7 +60,8 @@ def get_image_packages_license_block(image_obj):
             if package.pkg_licenses:
                 licenses.add(", ".join(package.pkg_licenses))
     for lic in licenses:
-        if not spdx_common.is_spdx_license_expression(lic):
+        valid_expression, _ = spdx_common.is_spdx_license_expression(lic)
+        if not valid_expression:
             block += spdx_formats.license_id.format(
                 license_ref=spdx_common.get_license_ref(lic)) + '\n'
             block += spdx_formats.extracted_text.format(orig_license=lic) + '\n'
@@ -78,7 +79,8 @@ def get_image_file_license_block(image_obj):
             for lic in spdx_common.get_layer_licenses(layer):
                 licenses.add(lic)
     for lic in licenses:
-        if not spdx_common.is_spdx_license_expression(lic):
+        valid_expression, _ = spdx_common.is_spdx_license_expression(lic)
+        if not valid_expression:
             block += spdx_formats.license_id.format(
                 license_ref=spdx_common.get_license_ref(lic)) + '\n'
             block += spdx_formats.extracted_text.format(orig_license=lic) + '\n'