EnxDev commented on code in PR #44465:
URL: https://github.com/apache/superset/pull/44465#discussion_r4059904867
##########
superset/utils/screenshot_utils.py:
##########
@@ -145,6 +148,31 @@ class ScreenshotBlankCaptureError(RuntimeError):
"""Raised when Chromium repeatedly returns a perceptually blank capture."""
+def validate_report_screenshot(
+ screenshot: bytes,
+ context: ReportExecutionContext,
+) -> None:
+ """Validate exact image bytes independently of browser or cache
provenance."""
+ if context.capture_was_rejected:
+ raise ScreenshotBlankCaptureError("Capture was already rejected")
+ if context.artifact_was_validated(screenshot):
+ return
+ try:
+ with Image.open(io.BytesIO(screenshot)) as image:
+ image.verify()
+ blankness = get_screenshot_blankness_metrics(screenshot)
Review Comment:
Could we make pixel-decoding failures propagate here? Image.verify() checks
the PNG structure and CRCs, but it does not decode the IDAT stream.
get_screenshot_blankness_metrics() then catches a decoding OSError and returns
is_blank=False, so these bytes are approved below. I reproduced this with a PNG
whose IDAT payload was corrupted and whose CRC was recomputed: verify() passed,
convert("RGB").load() raised OSError: broken data stream, and
validate_report_screenshot() still called approve_artifact(). For this final
delivery gate, a decode failure should reject the artifact as invalid_image,
for example by letting the helper surface decode errors to this caller.
##########
superset/utils/report_execution.py:
##########
@@ -245,6 +257,14 @@ def reject_capture(self, reason: str) -> None:
self._capture_rejection_reasons.append(reason)
+ def approve_artifact(self, artifact: bytes) -> None:
+ """Record exact bytes validated by the capture or PDF assembly
stage."""
+ self._validated_artifacts.add(hashlib.sha256(artifact).hexdigest())
Review Comment:
Could we namespace approvals by artifact type? _get_pdf() validates the
source screenshots first, so their hashes are already in this same set. If
notification_content.pdf is later replaced with one of those PNG byte strings,
the PDF gate sees artifact_was_validated(...) == True and accepts bytes that
were never produced by PDF assembly. Storing (kind, digest) or separate
screenshot/PDF sets would preserve the exact-PDF provenance this check is meant
to enforce.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
