Skip to content
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

add image file name validation. #4249

Merged
merged 14 commits into from
May 7, 2024
Merged

Conversation

thefrieddan1
Copy link
Contributor

add image file name validation.

Related Issues

fixes: https://jira-dc.paloaltonetworks.com/browse/CIAC-5180

Description

Add image file name validation to validate_content_path script

add image file name validation.
@thefrieddan1 thefrieddan1 self-assigned this Apr 29, 2024
@thefrieddan1 thefrieddan1 changed the title init commit. add image file name validation. Apr 29, 2024
@thefrieddan1 thefrieddan1 marked this pull request as ready for review April 30, 2024 18:49
Copy link

Changelog(s) in markdown:

  • Added validation for image files in doc_files folder. #4249

Copy link
Contributor

@dorschw dorschw left a comment

Choose a reason for hiding this comment

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

Nice! looks good

demisto_sdk/scripts/validate_content_path.py Outdated Show resolved Hide resolved
demisto_sdk/scripts/validate_content_path.py Outdated Show resolved Hide resolved
@@ -295,6 +312,12 @@ def _validate(path: Path) -> None:
raise InvalidXDRCTemplatesFileName


def _validate_image_file_name(image_name: str):
pattern = r"[^0-9a-zA-Z-_]+"
if re.findall(pattern, image_name):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think fullmatch would suit better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fullmatch does not catch the invalid img file names from the unit test case

demisto_sdk/commands/common/constants.py Show resolved Hide resolved
demisto_sdk/commands/common/constants.py Show resolved Hide resolved
@thefrieddan1 thefrieddan1 requested a review from dorschw May 5, 2024 10:04
@@ -283,6 +290,9 @@ def _validate(path: Path) -> None:
):
raise InvalidXSIAMReportFileName

if first_level_folder == DOC_FILES_DIR:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if first_level_folder == DOC_FILES_DIR:
if first_level_folder == DOC_FILES_DIR and path.suffix in (".png", ".jpg", ".jpeg", ".gif"):
  1. Have I missed any suffixes?
  2. Looks like we call it on all files under DOC_FILES_DIR. Please insert this change and then modify the unit tests to match (make sure that it only fails on image files, test it with image suffixes and with non-image and make sure non-image files can contain chars that images are not allowed to).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The constant ALLOWED_SUFFIXES support only jpg and svg

@thefrieddan1 thefrieddan1 requested a review from dorschw May 6, 2024 06:55
@thefrieddan1 thefrieddan1 merged commit 9312161 into master May 7, 2024
24 checks passed
@thefrieddan1 thefrieddan1 deleted the validate_image_file_names branch May 7, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants