Responsive images not added when effective scheme differs from image src scheme

[webaware]

wp_image_add_srcset_and_sizes() checks for base URL matches between available files and the src URL from the image tag in content. When the protocol scheme is different, the base URL doesn't match the src URL and no responsive images are added.

This can occur in such circumstances as:

• image added with protocol http: and page viewed with protocol https:
• image added with protocol https: and page viewed with protocol http:
• image added with no protocol (i.e. protocol-relative URL) and page viewed with either protocol http: or https:
• upload_dir filter changes baseurl to https: to prevent insecure content

[webaware]

patch to fix

[johnbillion]

Thanks for the patch @webaware. Some unit tests for this would be great.

[webaware]

@johnbillion will see what I can do. Was hoping for some non-keyboard time today :) (Sunday)

[jaspermdegroot]

This seems to be a duplicate of #34945

[webaware]

Unit test

[webaware]

@jaspermdegroot related, but not the same. #34945 is more of a problem with upload_dir and while related, is different to this issue which is really all about matching available responsive images to an image from the HTML tag in content.

[azaozz]

image_base_url.diff wouldn't work properly. This shouldn't be changed:

-	$all_sizes[] = $base_url . $image_meta['file'];
+	$all_sizes[] = $image_base_url . $image_meta['file'];

as $image_meta['file'] already contains the year/month sub-dirs. If we want to keep it, the first block should be moved right under $base_url = trailingslashit( _wp_upload_dir_baseurl() );.

On the other hand we only use these for comparison. Would be better to just strip https?: from both.

[azaozz]

In 35045.patch:

• Remove the protocols from $image_src and $base_url before comparing them to be able to match http, https, and protocol relative URLs.
• Add the unit tests to the same patch.

[joemcgill]

This bug was introduced in r35820 because we build a full URL for each size and then compare them against the src of the image. Instead, we could just compare filenames, which would avoid trying to match schemes in the first place.

35045.diff changes the logic introduced in r35820 to compare file names instead of full URLs.

[webaware]

@joemcgill sounds good, makes sense.

[azaozz]

Replying to joemcgill:

Instead, we could just compare filenames...

Don't forget that you can have the same file name in different year/month sub-directories. That seems to happen regularly when uploading directly from some cameras because of their auto-naming schemes.

[joemcgill]

It seems like it would be fairly rare when an ID and a filename match, but result in a false positive. Event so, it's a good point so we can add the rest of the path as well, just to be safer.

[joemcgill]

35045.2.diff prepends the upload path from the full size image in the metadata to all the intermediate sizes before testing to see if one of the image sizes matches the path of the image src before adding srcset and sizes attributes.

[joemcgill]

Related: #35102

[azaozz]

Replying to joemcgill:

I'm a bit "uneasy" to only match the year/month/filename.ext part of the image src. Not matching the whole URL might bring other edge cases.

On the other hand that is the simplest way to support CDN plugins that replace all image src in post_content, so probably better to use that for now.

In 35045.3.diff: optimize/refine 35045.2.diff a bit. We can check if $image_meta['file'] is contained in the image src instead of adding it to the all_sizes array and then looking for it.

[azaozz]

In 35045.3.2.diff: A little more cleanup.

[azaozz]

Was also thinking we can move the "URL exists in image meta" check to wp_calculate_image_srcset() as @joemcgill mentions here. It will introduce very little overhead when we are certain the attachment ID matches the image (for galleries, etc.).

As far as I see we need to check if the image src matches one of the srcset URLs after the 'wp_calculate_image_srcset' filter at the end of wp_calculate_image_srcset(). Are there any legitimate use cases where this won't happen?

[joemcgill]

Move src match to wp_calculate_image_srcset()

[joemcgill]

Was also thinking we can move the "URL exists in image meta" check to wp_calculate_image_srcset()

I was thinking about this again earlier today while discussion #35102. Moving this logic to wp_calculate_image_srcset() streamlines the code a little bit and has the added benefit allowing the filter to still be reachable by plugin authors who have rewritten image URLs for whatever reason (e.g., CDNs, etc.).

35045.4.diff is a first pass at what this would look like if we moved the logic from 35045.2.diff (et al.) into wp_calculate_image_srcset().

[azaozz]

In 35045.5.diff:

• Move the false !== strpos( $image_src, $dirname . $image['file'] ) at the beginning of the loop. Otherwise it won't match when the image src is for a size wider than $max_srcset_image_width.
• Simplify/cleanup a bit.

[polevaultweb]

In 35045.6.diff:

Added filters for $dirname and $image_baseurl. Plugins like ours may store the files in a different location than on the server. For example, WP Offload S3 has an option for object versioning to bust CDN caches so images would go from the server as 2015/12/image.jpg to S3 as 2015/12/15154454/image.jpg.

Without this filter, the check of $dirname . $image['file'] will still fail.

Adding the filter on $image_baseurl has the extra benefit that we can rewrite the URL for the image size files instead of doing it later on the 'wp_calculate_image_srcset' hook.

[azaozz]

Replying to polevaultweb:

Added filters for $dirname and $image_baseurl.

Don't think we should be adding filters there. That will limit the update possibilities by "locking" us in back-compat. The wp_calculate_image_srcset filter is powerful enough. If a plugin moves all images and doesn't implement the same directory structure, it seems it should be overriding/replacing wp_image_add_srcset_and_sizes() and/or wp_calculate_image_srcset().

What we can try doing is check (again?) after the wp_calculate_image_srcset filter if the image src matches one of the srcset URLs. That will make it possible to set the srcset for all cases.

[azaozz]

In 35045.7.diff: add wp_calculate_image_srcset_dirname_match filter for the sake of plugins that add external image sources but do not maintain the year/month sub-dirs. This allows them to easily manipulate the image path before we check for a match.

This patch also fixes #35102.

[azaozz]

In 35045.8.diff: same as 35045.7.diff but instead of filtering only the year/month bit it lets plugins fix all inconsistencies in the attachment meta by introducing wp_calculate_image_srcset_meta filter. This can be used to fix/normalize all sorts of changes made by plugins over the years.

[azaozz]

In 36121:

Responsive images: fix the check whether the attachment meta matches the image src to work with http/https and CDNs.

Props webaware, joemcgill, azaozz.
Fixes #35045 and #35102 for trunk.

[azaozz]

Reopen for 4.4.

[dd32]

In 36152:

Responsive images: fix the check whether the attachment meta matches the image src to work with http/https and CDNs.

Merges [36121] to the 4.4 branch.
Props webaware, joemcgill, azaozz.
Fixes #35045 and #35102.