fitzee commented on code in PR #44465:
URL: https://github.com/apache/superset/pull/44465#discussion_r4060292971
##########
superset/utils/webdriver.py:
##########
@@ -333,13 +336,16 @@ def _get_validated_screenshot(
else REPORT_HAS_RENDERED_CHART_HOLDERS_JS
)
)
- except PlaywrightError:
- has_rendered_content = False
+ except PlaywrightError as ex:
+
report_execution_context.reject_capture("content_state_unknown")
logger.warning(
"report_capture_content_state_failed capture=standard%s",
context_suffix,
exc_info=True,
)
+ raise ScreenshotBlankCaptureError(
Review Comment:
Fixed in 857769fed8fe5d4cb2c0c5419cabb8310723cb61. Bootstrap-only captures
no longer receive the delivery-validation context, preserving the existing
CSV/XLSX/TEXT query-context bootstrap behavior.
##########
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)
+ except (SoftTimeLimitExceeded, ReportExecutionBudgetExceededError):
+ context.reject_capture("validation_interrupted")
+ raise
+ except Exception as ex:
+ context.reject_capture("invalid_image")
+ raise ScreenshotBlankCaptureError("Unable to validate screenshot
bytes") from ex
+ if blankness.is_blank:
Review Comment:
Fixed in 857769fed8fe5d4cb2c0c5419cabb8310723cb61. A blank capture is
approved as a screenshot when browser state positively establishes that no
holder rendered content, so empty/error-only results pass the final gate
without weakening validation of other blank captures.
##########
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:
Fixed in 857769fed8fe5d4cb2c0c5419cabb8310723cb61. Final screenshot
validation now propagates decoding failures from the blankness helper and
rejects the artifact as `invalid_image`. A regression test covers a PNG whose
IDAT payload is corrupted with a recomputed CRC.
##########
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:
Fixed in 857769fed8fe5d4cb2c0c5419cabb8310723cb61. Artifact approvals are
now keyed by both kind and digest, preventing screenshot approval from
authorizing identical bytes as a PDF.
--
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]
