Commit 92b1f920efa for woocommerce
commit 92b1f920efa5d033e71e9b1bbc53723a7b013269
Author: Allison Levine <1689238+allilevine@users.noreply.github.com>
Date: Thu Jul 16 14:37:12 2026 -0400
[Email Editor] Fix gallery crop misclassifying ampersand-src images as server-cropped (#66739)
Fix gallery crop misclassifying ampersand-src images as server-cropped
The server-crop check compared esc_url($filtered_url) against $image_url from
get_attribute('src'). get_attribute returns the entity-decoded src (raw '&')
while esc_url re-encodes '&' to '&', so any image whose src had a
multi-param query string (e.g. a CDN URL like '?w=600&h=600') compared as
different and was wrongly treated as server-cropped — stamping concrete
width/height on an uncropped file and distorting it in clients that honor those
attributes but not object-fit (Outlook desktop).
Detect the crop by comparing the raw filter result to the original src before
escaping, and escape only for output. Branch on the escaped URL being non-empty
so a hostile URL reduced to empty by esc_url also falls back to CSS cropping.
Add a regression test with an ampersand-containing src.
Bug introduced in PR #66570.
diff --git a/packages/php/email-editor/changelog/fix-nl-738-gallery-crop-server-detect-encoding b/packages/php/email-editor/changelog/fix-nl-738-gallery-crop-server-detect-encoding
new file mode 100644
index 00000000000..a2647ee4369
--- /dev/null
+++ b/packages/php/email-editor/changelog/fix-nl-738-gallery-crop-server-detect-encoding
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Fix email gallery aspect-ratio crop misclassifying images whose src contains a query-string ampersand (e.g. CDN URLs) as server-cropped, which stamped distorting fixed dimensions on uncropped images. The renderer now detects a server crop by comparing the filter result to the original URL before escaping.
diff --git a/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-gallery.php b/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-gallery.php
index 5fb4843af19..35f57498a57 100644
--- a/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-gallery.php
+++ b/packages/php/email-editor/src/Integrations/Core/Renderer/Blocks/class-gallery.php
@@ -236,18 +236,23 @@ class Gallery extends Abstract_Block_Renderer {
*/
$filtered_url = apply_filters( 'woocommerce_email_editor_gallery_cropped_image_url', $image_url, $aspect_ratio, $width, $height, $image_attrs );
- // Extensions can return anything (arrays, WP_Error, objects). Only accept a string, and
- // sanitize it before we compare or use it so an invalid value can't emit a warning, be
- // misclassified as a server crop, or become an empty src.
- $cropped_url = is_string( $filtered_url ) ? esc_url( $filtered_url ) : '';
-
- $is_server_cropped = '' !== $image_url && '' !== $cropped_url && $cropped_url !== $image_url;
+ // Extensions can return anything (arrays, WP_Error, objects). A crop happened only when an
+ // integration returned a *different*, non-empty string. Compare the raw filter result against
+ // the original src here, BEFORE escaping: get_attribute() returns the entity-decoded src (raw
+ // "&"), while esc_url() re-encodes "&" to "&", so comparing an escaped URL against the
+ // decoded original would misclassify any image whose src has a multi-param query string (e.g.
+ // "?w=600&h=600", common with image CDNs) as server-cropped.
+ $is_server_cropped = is_string( $filtered_url ) && '' !== $image_url && '' !== $filtered_url && $filtered_url !== $image_url;
+
+ // Escape for output only after the decision. If esc_url() reduces a hostile/invalid crop URL
+ // to an empty string, fall through to the CSS branch rather than emit an empty src.
+ $cropped_url = $is_server_cropped ? esc_url( (string) $filtered_url ) : '';
// These crop styles are appended after Html_Processing_Helper::sanitize_image_html() has run
// (its style allowlist would otherwise strip object-fit), so they intentionally bypass that
// sanitizer. Only the regex-validated $aspect_ratio may be interpolated here — every other
// token is a literal. Do not add dynamic values to $crop_styles without validating them.
- if ( $is_server_cropped ) {
+ if ( '' !== $cropped_url ) {
// The file is already cropped to the requested ratio, so we can give it concrete
// dimensions. This renders the crop correctly even in clients without CSS crop support.
$html->set_attribute( 'src', $cropped_url );
diff --git a/packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Blocks/Gallery_Test.php b/packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Blocks/Gallery_Test.php
index 194c699c408..e4ca0de4deb 100644
--- a/packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Blocks/Gallery_Test.php
+++ b/packages/php/email-editor/tests/integration/Integrations/Core/Renderer/Blocks/Gallery_Test.php
@@ -519,6 +519,35 @@ class Gallery_Test extends \Email_Editor_Integration_Test_Case {
$this->assertSame( 3, $image_count, 'All three images are present.' );
}
+ /**
+ * Test an image whose src has a multi-param query string (e.g. a CDN URL with "&") is not
+ * misclassified as server-cropped when no integration rewrites it.
+ *
+ * Regression: get_attribute() returns the entity-decoded src while esc_url() re-encodes "&" to
+ * "&", so comparing an escaped URL against the decoded original wrongly flagged any "&"-in-src
+ * image as cropped and stamped it with distorting fixed dimensions.
+ */
+ public function testItDoesNotMisclassifyAmpersandSrcAsServerCropped(): void {
+ $parsed_gallery = $this->parsed_gallery;
+ $parsed_gallery['attrs']['aspectRatio'] = '1';
+ // A CDN-style src with more than one query parameter, so the URL contains an ampersand.
+ $parsed_gallery['innerBlocks'][0]['innerHTML'] = '<figure class="wp-block-image size-large"><img src="https://example.com/image1.jpg?w=600&h=600" alt="Image 1" class="wp-image-1"/></figure>';
+
+ // No filter is attached, so nothing is server-cropped.
+ $rendered = $this->gallery_renderer->render( '', $parsed_gallery, $this->rendering_context );
+
+ // The ampersand image keeps CSS-only cropping: no fixed width/height that would distort it.
+ $image_count = 0;
+ $html = new \WP_HTML_Tag_Processor( $rendered );
+ while ( $html->next_tag( array( 'tag_name' => 'img' ) ) ) {
+ ++$image_count;
+ $this->assertNull( $html->get_attribute( 'width' ), 'An uncropped ampersand-src image must not get a fixed width.' );
+ $this->assertNull( $html->get_attribute( 'height' ), 'An uncropped ampersand-src image must not get a fixed height.' );
+ }
+ $this->assertSame( 3, $image_count, 'All three images are present.' );
+ $this->assertStringContainsString( 'object-fit: cover', $rendered, 'Images still get CSS cropping.' );
+ }
+
/**
* Test the crop is sized to the actual rendered cell width, so an incomplete final row
* (e.g. a lone trailing image spanning the full width) is not served an undersized file.