Skip to content

[issue-386] Finish Package validation #461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
armintaenzertng merged 3 commits into from
Feb 1, 2023
Merged

[issue-386] Finish Package validation #461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b330e5
[issue-386] add tests for package_verification_code_validator
armintaenzertng Feb 1, 2023
512bf71
[issue-386] test "no contains relationship when files_analyzed=False"
armintaenzertng Feb 1, 2023
f34bd1a
[issue-386] test uppercase checksums/verification codes
armintaenzertng Feb 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/spdx/validation/checksum_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def validate_checksum(checksum: Checksum, parent_id: str) -> List[ValidationMess
length = algorithm_length[algorithm]
validation_messages.append(
ValidationMessage(
f"value of {algorithm} must consist of {length} hexadecimal digits, but is: {checksum.value} (length: {len(checksum.value)} digits)",
f"value of {algorithm} must consist of {length} lowercase hexadecimal digits, but is: {checksum.value} (length: {len(checksum.value)} digits)",
context)
)

Expand Down
25 changes: 10 additions & 15 deletions src/spdx/validation/package_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@

from spdx.model.document import Document
from spdx.model.package import Package
from spdx.model.relationship import RelationshipType
from spdx.model.relationship import RelationshipType, Relationship
from spdx.model.relationship_filters import filter_by_type_and_origin, filter_by_type_and_target
from spdx.validation.checksum_validator import validate_checksums
from spdx.validation.external_package_ref_validator import validate_external_package_refs
from spdx.validation.license_expression_validator import validate_license_expression, validate_license_expressions
Expand Down Expand Up @@ -43,23 +44,18 @@ def validate_package_within_document(package: Package, document: Document) -> Li
for message in validate_spdx_id(package.spdx_id, document):
validation_messages.append(ValidationMessage(message, context))

# TODO: make test for this (https://github.com/spdx/tools-python/issues/386)
if not package.files_analyzed:
package_contains_relationships = [relationship for relationship in document.relationships if
relationship.relationship_type == RelationshipType.CONTAINS and relationship.spdx_element_id == package.spdx_id]
if package_contains_relationships:
validation_messages.append(
ValidationMessage(
f"package must contain no elements if files_analyzed is False, but found {package_contains_relationships}",
context)
)
package_contains_relationships = filter_by_type_and_origin(document.relationships, RelationshipType.CONTAINS,
package.spdx_id)
contained_in_package_relationships = filter_by_type_and_target(document.relationships,
RelationshipType.CONTAINED_BY, package.spdx_id)

combined_relationships: List[Relationship] = package_contains_relationships + contained_in_package_relationships

contained_in_package_relationships = [relationship for relationship in document.relationships if
relationship.relationship_type == RelationshipType.CONTAINED_BY and relationship.related_spdx_element_id == package.spdx_id]
if contained_in_package_relationships:
if combined_relationships:
validation_messages.append(
ValidationMessage(
f"package must contain no elements if files_analyzed is False, but found {contained_in_package_relationships}",
f"package must contain no elements if files_analyzed is False, but found {combined_relationships}",
Copy link
Collaborator

@meretp meretp Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading this, I was wondering if a package must contain no elements in general or only files? The spec is mostly referring to files and I am not sure, if a package could contain anything other than a file.

Copy link
Collaborator Author

@armintaenzertng armintaenzertng Feb 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it can contain anything else than files. This is not explicitly stated by the spec for relationships, so who knows... I would leave it like that for now.

context)
)

Expand All @@ -83,7 +79,6 @@ def validate_package(package: Package, context: Optional[ValidationContext] = No
for message in validate_url(homepage):
validation_messages.append(ValidationMessage("homepage " + message, context))

# TODO: is verification_code required if files_analyzed=True? (https://github.com/spdx/tools-python/issues/386)
verification_code = package.verification_code
if verification_code:
if not package.files_analyzed:
Expand Down
7 changes: 3 additions & 4 deletions src/spdx/validation/package_verification_code_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,23 @@
from spdx.validation.validation_message import ValidationMessage, ValidationContext, SpdxElementType


# TODO: make test for this (https://github.com/spdx/tools-python/issues/386)
def validate_verification_code(verification_code: PackageVerificationCode, parent_id: str) -> List[ValidationMessage]:
validation_messages: List[ValidationMessage] = []
context = ValidationContext(parent_id=parent_id, element_type=SpdxElementType.PACKAGE_VERIFICATION_CODE,
full_element=verification_code)

for file in verification_code.excluded_files:
if not file.startswith("./"):
if file.startswith("/"):
validation_messages.append(
ValidationMessage(
f'file name must be a relative path to the file, starting with "./", but is: {file}', context)
f'file name must not be an absolute path starting with "/", but is: {file}', context)
)

value: str = verification_code.value
if not re.match("^[0-9a-f]{40}$", value):
validation_messages.append(
ValidationMessage(
f"value of verification_code must consist of 40 hexadecimal digits, but is: {value} (length: {len(value)} digits)",
f"value of verification_code must consist of 40 lowercase hexadecimal digits, but is: {value} (length: {len(value)} digits)",
context)
)

Expand Down
36 changes: 19 additions & 17 deletions tests/spdx/validation/test_checksum_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,40 +58,42 @@ def test_valid_checksum(checksum):

@pytest.mark.parametrize("checksum, expected_message",
[(Checksum(ChecksumAlgorithm.SHA1, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.SHA1 must consist of 40 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.SHA1 must consist of 40 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.SHA224, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.SHA224 must consist of 56 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.SHA224 must consist of 56 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.SHA256, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.SHA256 must consist of 64 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.SHA256 must consist of 64 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.SHA384, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.SHA384 must consist of 96 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.SHA384 must consist of 96 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.SHA512, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.SHA512 must consist of 128 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.SHA512 must consist of 128 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.SHA3_256, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.SHA3_256 must consist of 64 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.SHA3_256 must consist of 64 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.SHA3_384, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.SHA3_384 must consist of 96 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.SHA3_384 must consist of 96 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.SHA3_512, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.SHA3_512 must consist of 128 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.SHA3_512 must consist of 128 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.BLAKE2B_256, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.BLAKE2B_256 must consist of 64 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.BLAKE2B_256 must consist of 64 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.BLAKE2B_384, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.BLAKE2B_384 must consist of 96 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.BLAKE2B_384 must consist of 96 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.BLAKE2B_512, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.BLAKE2B_512 must consist of 128 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.BLAKE2B_512 must consist of 128 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.BLAKE3, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.BLAKE3 must consist of at least 256 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.BLAKE3 must consist of at least 256 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.MD2, "71c4025dd9897b364f3ebbb42c484ff43d00791c"),
"value of ChecksumAlgorithm.MD2 must consist of 32 hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
"value of ChecksumAlgorithm.MD2 must consist of 32 lowercase hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
(Checksum(ChecksumAlgorithm.MD4, "71c4025dd9897b364f3ebbb42c484ff43d00791c"),
"value of ChecksumAlgorithm.MD4 must consist of 32 hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
"value of ChecksumAlgorithm.MD4 must consist of 32 lowercase hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
(Checksum(ChecksumAlgorithm.MD5, "71c4025dd9897b364f3ebbb42c484ff43d00791c"),
"value of ChecksumAlgorithm.MD5 must consist of 32 hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
"value of ChecksumAlgorithm.MD5 must consist of 32 lowercase hexadecimal digits, but is: 71c4025dd9897b364f3ebbb42c484ff43d00791c (length: 40 digits)"),
(Checksum(ChecksumAlgorithm.MD6,
"a872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafed5"),
"value of ChecksumAlgorithm.MD6 must consist of between 0 and 512 hexadecimal digits, but is: a872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafed5 (length: 513 digits)"),
"value of ChecksumAlgorithm.MD6 must consist of between 0 and 512 lowercase hexadecimal digits, but is: a872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafeda872cac2efd29ed2ad8b5faa79b63f983341bea41183582b8863d952f6ac3e1cdfe0189967a13006857d3b9985174bf67239874dcec4cbbc9839496179feafed5 (length: 513 digits)"),
(Checksum(ChecksumAlgorithm.ADLER32, "af1eec2a1b18886c3f3cc244349d91d8"),
"value of ChecksumAlgorithm.ADLER32 must consist of 8 hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
"value of ChecksumAlgorithm.ADLER32 must consist of 8 lowercase hexadecimal digits, but is: af1eec2a1b18886c3f3cc244349d91d8 (length: 32 digits)"),
(Checksum(ChecksumAlgorithm.SHA1, "CE9F343C4BA371746FD7EAD9B59031AE34D8AFC4"),
"value of ChecksumAlgorithm.SHA1 must consist of 40 lowercase hexadecimal digits, but is: CE9F343C4BA371746FD7EAD9B59031AE34D8AFC4 (length: 40 digits)"),
])
def test_invalid_checksum(checksum, expected_message):
parent_id = "parent_id"
Expand Down
27 changes: 25 additions & 2 deletions tests/spdx/validation/test_package_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@
import pytest
from license_expression import Licensing

from spdx.model.relationship import Relationship, RelationshipType
from spdx.model.spdx_no_assertion import SpdxNoAssertion
from spdx.model.spdx_none import SpdxNone
from spdx.validation.package_validator import validate_package_within_document
from spdx.validation.validation_message import ValidationMessage, ValidationContext, SpdxElementType
from tests.spdx.fixtures import package_fixture, package_verification_code_fixture, document_fixture
from tests.spdx.fixtures import package_fixture, package_verification_code_fixture, document_fixture, file_fixture


def test_valid_package():
Expand All @@ -42,7 +43,7 @@ def test_valid_package():
license_info_from_files=[Licensing().parse("some_license")],
verification_code=None),
"license_info_from_files must be None if files_analyzed is False, but is: [LicenseSymbol('some_license', "
"is_exception=False)]")
"is_exception=False)]")
])
def test_invalid_package(package_input, expected_message):
validation_messages: List[ValidationMessage] = validate_package_within_document(package_input,
Expand All @@ -54,3 +55,25 @@ def test_invalid_package(package_input, expected_message):
full_element=package_input))

assert validation_messages == [expected]


@pytest.mark.parametrize("relationships",
[[Relationship("SPDXRef-Package", RelationshipType.CONTAINS, "SPDXRef-File1")],
[Relationship("SPDXRef-Package", RelationshipType.CONTAINS, "DocumentRef-external:SPDXRef-File")],
[Relationship("SPDXRef-File2", RelationshipType.CONTAINED_BY, "SPDXRef-Package")],
[Relationship("DocumentRef-external:SPDXRef-File", RelationshipType.CONTAINED_BY, "SPDXRef-Package")],
[Relationship("SPDXRef-Package", RelationshipType.CONTAINS, "SPDXRef-File2"),
Relationship("SPDXRef-File1", RelationshipType.CONTAINED_BY, "SPDXRef-Package")]])
def test_invalid_package_with_contains(relationships):
document = document_fixture(relationships=relationships,
files=[file_fixture(spdx_id="SPDXRef-File1"), file_fixture(spdx_id="SPDXRef-File2")])
package = package_fixture(files_analyzed=False, verification_code=None, license_info_from_files=[])
context = ValidationContext(spdx_id=package.spdx_id, parent_id=document.creation_info.spdx_id,
element_type=SpdxElementType.PACKAGE,
full_element=package)

validation_messages: List[ValidationMessage] = validate_package_within_document(package, document)

assert validation_messages == [
ValidationMessage(f"package must contain no elements if files_analyzed is False, but found {relationships}",
context)]
Loading