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

Fix some html.screenshot usages #1031

Merged
merged 10 commits into from
Nov 29, 2023
Merged

Fix some html.screenshot usages #1031

merged 10 commits into from
Nov 29, 2023

Conversation

rw-access
Copy link
Contributor

FYI @aidenmitchell.

I didn't look at any other rules to check for invalid usage of .. inside file.explode(.).
Remember that .. inside a file.explode is referring to the attachment, not the exploded file. In the archive case, the attachment is the archive file.

@rw-access rw-access requested a review from a team November 22, 2023 19:11
and any(file.explode(.),
any(ml.logo_detect(file.html_screenshot(..)).brands,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will evaluate the exact same every time so shouldn't be in the loop.

or ..file_type == "html"
or ..content_type == "text/html"
)
and any(ml.logo_detect(file.html_screenshot(..)).brands,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this entirely. We currently can't look at raw files in archives. This one is referencing the archive attachment.

@rw-access rw-access changed the title Ross.html screenshot fixes Fix some html.screenshot usages Nov 22, 2023
@rw-access rw-access enabled auto-merge (squash) November 22, 2023 20:02
@rw-access rw-access merged commit c797e4f into main Nov 29, 2023
3 checks passed
@rw-access rw-access deleted the ross.html-screenshot-fixes branch November 29, 2023 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants