Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 798 forks source link

Photon: filter attachment_url_to_postid() to support Photon CDN URLs #23405

Open jbeales opened 2 years ago

jbeales commented 2 years ago

Impacted plugin

Jetpack

Steps to Reproduce

  1. Install Jetpack.
  2. Turn on the Photon image CDN, (Jetpack > Settings > Performance > Performance & Speed and turn on the "Enable site accelerator" and "Speed up image load times" toggles).
  3. Pass the CDN URL of any image in the media gallery to WordPress's attachment_url_to_postid() function.
  4. Inspect the return value from the function. It should be an integer post ID, but it'll be zero.

A clear and concise description of what you expected to happen.

I expect the post ID of the attachment to be returned by attachment_url_to_postid() even with Photon active.

What actually happened

attachment_url_to_postid() returns a zero, which means it failed to look up the post ID of the image.

Other information

This bug makes sense, because attachment_url_to_postid() queries the database for the URL that was passed in, and the CDN URL is not in the database.

However, there is a attachment_url_to_postid filter, and I expect that Jetpack would be a good WordPress citizen and, when Photon is on, use that filter to extract the original from the CDN URL and look up the post ID based on the original URL.

Operating System

macOS, Linux

OS Version

Confirmed on both Linux webserver and macOS localhost.

Browser

Other / Not applicable

Browser Version(s)

Not a browser problem.

jeherve commented 2 years ago

there is a attachment_url_to_postid filter, and I expect that Jetpack would be a good WordPress citizen and, when Photon is on, use that filter to extract the original from the CDN URL and look up the post ID based on the original URL.

That's a good idea indeed, that'd be a nice way to extend attachment_url_to_postid() so it can work with Photon. 👍

amustaque97 commented 2 years ago

Hi there, I would like to work on this issue. Could you please highlight some important files/documentation?

jbeales commented 2 years ago

Hi,

This is the code I'm using to remove Photon stuff from URLs before they are passed to attachment_url_to_post_id():

/**
 * If this is a Jetpack Photon URL, remove the CDN URL parts and return the origin URL.
 * If this isn't a photon URL it simply returns $image_url
 *
 * @see https://github.com/Automattic/jetpack/issues/23405
 *
 * @param string $image_url The URL which may be a Photon CDN URL.
 *
 * @return string  The source URL.
 */
function maybe_de_photonize_image_url( $image_url ) {
    if ( mb_stripos( $image_url, '.wp.com/' ) !== false ) {
        $source_url = wp_parse_url( $image_url, PHP_URL_PATH );

        if ( wp_is_using_https() ) {
            // Only one slash because the $source_url starts with a slash.
            $image_url = 'https:/' . $source_url;
        } else {
            // Only one slash because the $source_url starts with a slash.
            $image_url = 'http:/' . $source_url;
        }
    }
    return $image_url;
}

But it's incomplete - it only removes the CDN prefix, it doesn't remove any of the image manipulation query string.

attachment_url_to_post_id() is defined here: https://core.trac.wordpress.org/browser/tags/5.9/src/wp-includes/media.php#L4965 The filter is at the end.

I don't remember exactly where the Photon files are in Jetpack, but a search for 'Photon' will find them.