cabrerahector / wordpress-popular-posts

WordPress Popular Posts - A highly customizable WordPress widget that displays your most popular posts.
https://wordpress.org/plugins/wordpress-popular-posts/
GNU General Public License v2.0
279 stars 83 forks source link

Image urls don't support Unicode characters in filenames #275

Closed dimnks closed 3 years ago

dimnks commented 3 years ago

Describe the bug

When an image thumbnail filename contains Unicode characters (e.g. Greek), the plugin default thumbnail shows up instead.

To Reproduce

Steps to reproduce the behavior:

  1. Go to 'Setings / WordPress Popular Posts'
  2. Click on 'Tools'
  3. Scroll down to 'Thumbnails'
  4. Current Settings: Pick Image from: Custom field / Resize image: No
  5. Upload a featured image with Unicode characters to a post and make some visits to it so it shows up in the popular list.

Expected behavior

The image should be picked regardless the filename character encoding.

Environment:

Additional context

Digging into the plugin fileset I spotted that is_image_url() method in Image class, uses filter_var($url, FILTER_VALIDATE_URL) to validate the image url which is buggy regarding urls containing Unicode characters. To bypass the issue, trying to keep some kind of url validation, I changed the specific block to:

if ( parse_url( $url, PHP_URL_SCHEME ) && parse_url( $url, PHP_URL_HOST ) ) {
    return false;
}

I know that this approach doesn't replicate the FILTER_VALIDATE_URL flag but still it provides some kind of control and thumbnails seem to work now. Would you suggest another approach?

cabrerahector commented 3 years ago

Thanks for the heads-up. I'll have a look into this as soon as I can.

However:

  1. Current Settings: Pick Image from: Custom field / Resize image: No
  2. Upload a featured image with Unicode characters to a post and make some visits to it so it shows up in the popular list.

So according to this you set WPP to pick your image from a custom field and then you're expecting WPP to pick up your Featured Image instead?

dimnks commented 3 years ago

Hey @cabrerahector!

You are right! Sorry about that, I mentioned the featured image by mistake. No, I'm not expecting that. I use a custom field to pick images through WPP. The issue occures with featured images as well though. As long as there are Unicode characters in the filename.

Thanks!

cabrerahector commented 3 years ago

Thanks for the update, @dimnks.

Alright, so I uploaded an image using some random greek text I found on the web as filename to a dev WP instance using the same settings as you and WPP was able to display the Custom Field image just fine (I used ACF for this since I didn't feel like registering a custom field manually):

Then I assigned the same image as featured image for that post, reloaded the page and got the same result:

dimnks commented 3 years ago

Hi @cabrerahector

Thanks for the follow up! I'm not sure what is going on, on my side. This is not the behavior I'm facing.

Without my code modification mentioned above, what I see is:

not_ok

Applying the modification makes the thumbnail work:

ok

cabrerahector commented 3 years ago

That's odd. I wonder why the filter_var validation passes on my set up and not in yours. Must be a server config issue.

Anyways, I can change that validation. That's just to make sure that the function receives an actual URL and not something else before checking whether that URL leads to an actual image or not. I'll play around with your suggestion (or try a different approach) and get back to you later.

cabrerahector commented 3 years ago

Hey @dimnks,

This seems to work (based on this SO answer):

/**
 * Checks whether an URL points to an actual image.
 *
 * @since   5.0.0
 * @access  private
 * @param   string
 * @return  array|bool
 */
private function is_image_url($url)
{
    $path = parse_url($url, PHP_URL_PATH);
    $encoded_path = array_map('urlencode', explode('/', $path));
    $parse_url = str_replace($path, implode('/', $encoded_path), $url);

    if ( ! filter_var($parse_url, FILTER_VALIDATE_URL) )
        return false;

    // sanitize URL, just in case
    $image_url = esc_url($url);
    // remove querystring
    preg_match('/[^\?]+\.(jpg|JPG|jpe|JPE|jpeg|JPEG|gif|GIF|png|PNG)/', $image_url, $matches);

    return ( is_array($matches) && ! empty($matches) ) ? $matches : false;
}

Could you please test that and report back your results?

dimnks commented 3 years ago

Hi @cabrerahector

Yeap! I confirm that images are showing up now. Sweet!

Thanks!

cabrerahector commented 3 years ago

Awesome! I'll test that code some more and push it to the repository once I'm sure it doesn't break anything.