Fixed case sensitivity issue Signed-off-by: Vibhinn Singhal <[email protected]> #15
Conversation
|
@VibhinnS, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
VibhinnS
changed the title
|
@VibhinnS Thank you for contributing to the VMware Build Inspector project. It looks like there may be a few issues with this pull request: Firstly, all commits included a pull request must be signed with a Developer Certificate of Origin. Details of this policy can be found here: https://cla.vmware.com/dco .Typically when the CLAbot warning fires, it means that one or more of the commits are missing this signoff. The easiest way to fix this usually is to roll-up any unsigned commits into a new branch, and sign the commit there, then use that new branch/commit as the source of the Pull Request. Secondly, From the description of this PR it looks like you were attempting to resolve issue #12 with this commit. Unfortunately from the commit, it looks like you are making changes in the Yum parser (https://github.com/vmware-labs/build-inspector/blob/main/code/parsers/yum.py) rather than the Warning rule (https://github.com/vmware-labs/build-inspector/blob/main/code/rules/warning.yar) where the issue has been noted. Please let me know if you need assistance in understanding or resolving either of these issues. |
|
@VibhinnS, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
| sha1_hash = sha1(namespace.encode('utf-8')).hexdigest() | ||
| sha512_hash = sha512(namespace.encode('utf-8')).hexdigest() | ||
| # return a dictionary containing both hash values | ||
| return {'sha1': sha1_hash, 'sha512': sha512_hash} |
There was a problem hiding this comment.
I'm not quite sure I understand why this change was made. This section of code does not require any sort of SHA hashing. This change should be reverted before merging. Please reference the Namespace section of the SPDX spec here: https://spdx.dev/spdx-specification-21-web-version/#h.1gdfkutofa90
| dep_document.package = Package(name=dependency.name, spdx_id=dep_document.spdx_id, download_location=dependency.download_location, version=dependency.version) | ||
| dep_document.package = Package(name=dependency.name, spdx_id=dep_document.spdx_id, | ||
| download_location=dependency.download_location, version=dependency.version) | ||
| checksums = create_spdx_namespace(dependency.name) |
There was a problem hiding this comment.
Again I am unclear why this is being used for hashing, as it was already an existing function with a specific purpose. Please revert this change.
| checksums = create_spdx_namespace(dependency.name) | ||
| # set SHA1 checksum | ||
| dep_document_checksum = Algorithm(identifier="SHA1", value=checksums['sha1']) | ||
| Algorithm(identifier="SHA-512", value=hashlib.sha512(cls.__TO_METHOD__(dep_document).encode()).hexdigest()) |
There was a problem hiding this comment.
It is unclear why this line is here, as the output from this statement is never saved or used.
| @@ -16,6 +16,8 @@ | |||
| from json import dumps, loads | |||
| from uuid import uuid4 | |||
| from io import StringIO | |||
| from spdx.checksum import Algorithm, sha1, sha512 | |||
There was a problem hiding this comment.
These imports should be included with the existing spdx.checksum imports
| @@ -16,6 +16,8 @@ | |||
| from json import dumps, loads | |||
| from uuid import uuid4 | |||
| from io import StringIO | |||
| from spdx.checksum import Algorithm, sha1, sha512 | |||
| import hashlib | |||
There was a problem hiding this comment.
Any functions used from Hashlib should be included in the existing Hashlib import on line 15
| dep_document_checksum = Algorithm(identifier="SHA1", value=checksums['sha1']) | ||
| Algorithm(identifier="SHA-512", value=hashlib.sha512(cls.__TO_METHOD__(dep_document).encode()).hexdigest()) | ||
| # set SHA512 checksum | ||
| dep_document_checksum.add_algorithm(Algorithm(identifier="SHA512", value=checksums['sha512'])) |
There was a problem hiding this comment.
I do not believe the spdx.checksum.Algorithm class has an "add_algorithm" model. Please confirm that this code is correct.
|
Closing this PR as it has been abandoned. |
Fixed the case sensitivity issue in Warning parser
Signed-off by: Vibhinn Singhal [email protected]