Make WordPress Core

Opened 23 months ago

Last modified 3 months ago

#57813 new defect (bug)

wp_get_missing_image_subsizes() can give error if image_meta exists, but not height/width

Reported by: donbowman's profile donbowman Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.1.1
Component: Media Keywords: has-patch
Focuses: Cc:

Description

I have an SVG file attached as image. The function throws an error:

        if ( ! empty( $imagesize ) ) {
                $full_width  = $imagesize[0];
                $full_height = $imagesize[1];
        } else {
                $full_width  = (int) $image_meta['width'];
                $full_height = (int) $image_meta['height'];
        }

on the else case. The below patch will resolve.
This is with 6.1.1

--- /app/wp-admin/includes/image.php.orig	2023-02-26 21:42:57.687046116 +0000
+++ /app/wp-admin/includes/image.php	2023-02-26 21:40:41.349180739 +0000
@@ -89,11 +89,11 @@

 	$registered_sizes = wp_get_registered_image_subsizes();
 	$image_meta       = wp_get_attachment_metadata( $attachment_id );

 	// Meta error?
-	if ( empty( $image_meta ) ) {
+	if ( empty( $image_meta ) || !array_key_exists('width', $image_meta) || !array_key_exists('height', $image_meta) ) {
 		return $registered_sizes;
 	}

 	// Use the originally uploaded image dimensions as full_width and full_height.
 	if ( ! empty( $image_meta['original_image'] ) ) {

Attachments (1)

img.patch (634 bytes) - added by donbowman 23 months ago.
patch to wp-admin/includes/image.php

Download all attachments as: .zip

Change History (8)

@donbowman
23 months ago

patch to wp-admin/includes/image.php

#1 @Soean
23 months ago

I can reproduce the error with and SVG file in WP 6.2 Beta 5 with the new Media inserter in the block editor.

#2 follow-up: @donbowman
15 months ago

This is another spot with the same issue, assuming width/height exist on svg. This is on 6.3.2

--- /app/wp-includes/media.php.orig	2023-10-28 15:19:04.671902110 +0000
+++ /app/wp-includes/media.php	2023-10-28 15:19:35.784547473 +0000
@@ -1642,6 +1642,8 @@
 	// Is it a full size image?
 	if (
 		isset( $image_meta['file'] ) &&
+		array_key_exists('width', $image_meta['file'] &&
+		array_key_exists('height', $image_meta['file'] &&
 		str_contains( $image_src, wp_basename( $image_meta['file'] ) )
 	) {
 		$dimensions = array(
Last edited 15 months ago by donbowman (previous) (diff)

#3 @apermo
3 months ago

#62293 was marked as a duplicate.

This ticket was mentioned in PR #7637 on WordPress/wordpress-develop by @apermo.


3 months ago
#4

Added a check if $image_meta['width'] and $image_meta['height'] are set, similar to other places in WordPress Core.

While this is rare case, it popped up in my sentry issues.

Trac ticket: https://core.trac.wordpress.org/ticket/57813

#5 @apermo
3 months ago

https://github.com/WordPress/wordpress-develop/pull/7637

I ran into a similar issue, as seen in #62293 and opened a PR on GitHub.

I will check @donbowman's patches and integrate them into my PR.
I think my issue and Don's are not necessarily the same but closely related. In case of Don's SVG issue, I think the solution might be a mime type check, and an early out in case of SVG?

Updated PR is coming once I'm done.

#6 in reply to: ↑ 2 @apermo
3 months ago

Replying to donbowman:

This is another spot with the same issue, assuming width/height exist on svg. This is on 6.3.2

--- /app/wp-includes/media.php.orig	2023-10-28 15:19:04.671902110 +0000
+++ /app/wp-includes/media.php	2023-10-28 15:19:35.784547473 +0000
@@ -1642,6 +1642,8 @@
 	// Is it a full size image?
 	if (
 		isset( $image_meta['file'] ) &&
+		array_key_exists('width', $image_meta['file'] &&
+		array_key_exists('height', $image_meta['file'] &&
 		str_contains( $image_src, wp_basename( $image_meta['file'] ) )
 	) {
 		$dimensions = array(

While this is definitely a related problem, I'm unsure wether this shouldn't be a seperate trac issue, to ease the discussion. The initial problem is a backend issue. This is a frontend issue. To reduce complexity I would suggest to split this part up into a second issue and a distinct PR if everyone agrees.

CC @Soean @donbowman @sabernhardt

#7 @apermo
3 months ago

After a conversation with @Soean I've created an additional ticket for the frontend related issue at https://core.trac.wordpress.org/ticket/62312#ticket

Note: See TracTickets for help on using tickets.