rankmath / seo-by-rank-math

Rank Math is a revolutionary WordPress SEO Plugin that combines the features of many SEO tools and lets you multiply your traffic in the easiest way possible :bulb: :chart_with_upwards_trend: →
https://rankmath.com
107 stars 52 forks source link

Hard crash using OpenGraph image change filter #130

Closed jorams closed 1 year ago

jorams commented 1 year ago

Describe the bug Using the filter to change an OpenGraph Image crashes the page.

A workaround is to configure a default OpenGraph thumbnail for the entire site. Only configuring an OpenGraph image for the specific page does not work.

To Reproduce

  1. Install on a fresh WordPress site
  2. Add the following code to a theme or plugin:
    add_filter('rank_math/opengraph/facebook/image', function() {
        return 'https://example.com/';
    });
  3. Load any page

Expected behavior A rendered page with a twitter:image meta tag containing the URL https://example.com/.

Instead, the page crashes with the following error:

Fatal error: Uncaught TypeError: Cannot access offset of type string on string in /var/www/html/wp-content/plugins/seo-by-rank-math/includes/opengraph/class-image.php:226
Stack trace:
#0 /var/www/html/wp-content/plugins/seo-by-rank-math/includes/opengraph/class-image.php(358): RankMath\OpenGraph\Image->add_image()
#1 /var/www/html/wp-content/plugins/seo-by-rank-math/includes/opengraph/class-image.php(81): RankMath\OpenGraph\Image->set_images()
#2 /var/www/html/wp-content/plugins/seo-by-rank-math/includes/opengraph/class-facebook.php(266): RankMath\OpenGraph\Image->__construct(Object(RankMath\OpenGraph\Facebook), Object(RankMath\OpenGraph\Facebook))
#3 /var/www/html/wp-includes/class-wp-hook.php(307): RankMath\OpenGraph\Facebook->image(Object(RankMath\OpenGraph\Facebook))
...

Additional context

The call to add_image was added in version 1.0.100 (not in this repository yet, it's line 349 here)

The change that breaks it was added in version 1.0.100.1 (not in this repository yet, it's line 226 here).

This last change assumes $attachment is an array with a key url, while it defaults to being an empty string.

The following patch fixes it:

diff --git a/class-image.php b/class-image.php
index 0cbf438..56418a0 100755
--- a/class-image.php
+++ b/class-image.php
@@ -223,7 +223,7 @@ class Image {
         * @param string $img The image we are about to add.
         */
        $filter_image_url = trim( $this->do_filter( "opengraph/{$this->network}/image", isset( $attachment['url'] ) ? $attachment['url'] : '' ) );
-       if ( ! empty( $filter_image_url ) && $filter_image_url !== $attachment['url'] ) {
+       if ( ! empty( $filter_image_url ) && (!isset( $attachment['url'] ) || $filter_image_url !== $attachment['url']) ) {
            $attachment = [ 'url' => $filter_image_url ];
        }
pratikrm commented 1 year ago

Hi @jorams

The image link you added to the filter is invalid. Can you please try adding a valid image link like https://example.com/image.png?

jorams commented 1 year ago

@pratikrm I have, the specific string is not relevant as long as it's not empty nor whitespace-only.

The code that effectively gets run is this:

$filter_image_url = trim( 'https://example.com' );
if ( ! empty( $filter_image_url ) && $filter_image_url !== $attachment['url'] ) {

The crash happens on the right side of the && in the use of the variable $attachment, which is not affected by the filter.

pratikrm commented 1 year ago

Hi @jorams

Thanks for sharing the details. I have logged it as a bug and we'll make sure to include the changes you mentioned in the next update.

someguy9 commented 1 year ago

@pratikrm is there a plan to revert how the open graph image filter function works? There seems to be no way to bypass how RankMath now validates if it's a valid image extension and strips the query string. My plugin that I made generates urls using query strings with no file extension and ends up returning nothing now when using the open graph filter settings.

pratikrm commented 1 year ago

@someguy9

Yeah, we are going to release an update soon to allow query strings added using the filter in opengraph image.

For now, to fix the issue please remove this block of the code in the plugin: https://github.com/rankmath/seo-by-rank-math/blob/master/includes/opengraph/class-image.php#L243-L251

someguy9 commented 1 year ago

@pratikrm thank you! I'll await the fix! (it's for a user of my plugin so an actual release is preferred).

surajv commented 1 year ago

Closing as it is fixed in v1.0.101