-
-
Notifications
You must be signed in to change notification settings - Fork 47
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.
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
feat: add CPE format validation in property setter #706
Conversation
- Implemented regex-based validation for CPE format in the model. - Added tests to verify handling of invalid CPE strings. Signed-off-by: Saquib Saifee <[email protected]>
@madpah and @jkowalleck please advise if these changes are enough to add this validation. |
Signed-off-by: Saquib Saifee <[email protected]>
@@ -1457,6 +1467,8 @@ def cpe(self) -> Optional[str]: | |||
|
|||
@cpe.setter | |||
def cpe(self, cpe: Optional[str]) -> None: | |||
if cpe and not CPE_REGEX.fullmatch(cpe): | |||
raise ValueError(f'Invalid CPE format: {cpe}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This behavioral change is a breaking change.
This is a remark, not a blocker. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CPE is a complex, external spec - outside the domain of CycloneDX.
This fact leads me to the architectural decision: we do not want to maintain an implementation of this external spec in the domain of CycloneDX python library.
meaning: this PR will not be accepted in its current form.
we might consider a usage of an external library, like https://pypi.org/project/cpe/.
Thank you for the PR, @saquibsaifee . I will close this PR for reasons detailed in #706 (review) For a different approach, please open a new pullrequest. |
Fixes #580