undefined index warnings due to Jetpack_Media_Summary::get()

  • This bug affects the Jetpack 3.9.1 modules
    * Twitter Cards (open graph)
    * Publicize (open graph)
    * Related Posts (maybe)

    Symptom you may see:

    Notice: Undefined offset: -1 in ***/wp-includes/post-template.php on line 268

    (In error_log, on screen with WP_DEBUG true, or not at all depending on server config and other plugins.)

    Jetpack_Media_Summary is used by the modules to create “og:” open graph meta tags for Facebook, and for whatever Related Posts does.

    It tries to get a post/page excerpt and fails. It would be easy to fix this bug, because the data it tries to get is never used.

    The problem is that get_excerpt() on line 261 of class.media-summary.php does this:

    $post_excerpt = apply_filters( ‘get_the_excerpt’, $post_excerpt );

    which in turn calls get_the_content(), an extremely touchy Worpdress core function. Then get_the_content() fails depending on the state of the loop, queries done by other plugins, etc., because it relies on the practically insane globals $page and $pages. The behavior of the code that emits the warning above in wp-includes/post-template.php is actually worse than just that warning. For example, $page=0 and count($pages)=1 will create the warning at the line:

    $content = $pages[$page – 1]

    I de-activated another plugin and the warning stopped, but only because then count($pages) was 0. PHP did not complain about an invalid index in an empty array! So the code is really a massive failure in both cases.

    Once your theme is in its loop and displaying the post/page, everything is fine. Typically $page=1 and count($pages)=1. But the modules do their work before that loop, in order to generate meta tags.

    I admit I have not tried our site with only Jetpack enabled, but even if it were just a plugin conflict, and the code is completely unnecessary, why keep it? By inspection, I don’t see $excerpt[‘extract’] is ever used. It’s just overhead in the data structure. So the fix is easy.

    (Debugging detail: $page was set to zero in register_globals(). Using wp_reset_query() does not help.)

    https://www.ads-software.com/plugins/jetpack/

Viewing 6 replies - 1 through 6 (of 6 total)
  • Plugin Author Jeremy Herve

    (@jeherve)

    Jetpack Mechanic ??

    While that excerpt isn’t used in the Jetpack codebase, we do use the generated string on WordPress.com, in the Publicize module, to generate the string of text that is sent to Facebook. We consequently can’t remove this from Jetpack.

    We also can’t afford not to go through that filter, as we need to make sure the excerpt has the proper formatting.

    It’d be interesting to look at what plugin triggered the error, and see in what way they’re customizing the excerpt. We could then see about adding an additional check after running the filter in Jetpack.

    Let me know what you find!

    Thread Starter kitchin

    (@kitchin)

    STR:

    1. Install WordPress.

    2. Install and activate and connect Jetpack.

    3. Configure Publicize: Twitter, approve Twitter credentials.

    4. Optional: Put define(‘WP_DEBUG’, true) in wp-config.php.

    5. Edit wp-includes/post-template.php.
    Around line 269 is this line:
    $content = $pages[$page - 1]

    After that line, add

    print("<pre style='background: white; color: black;'>DEBUG: page=$page, count(pages)=" . count($pages) ."</pre>\n");
print("<pre style='background: white; color: black'>DEBUG: strlen(content)=" . strlen($content) ."</pre>\n");

    6. Go to the Hello World post WP created on installation.

    Expected results:
    DEBUG: page=1, count(pages)=1
    DEBUG: strlen(content)= [ about 85? ]

    Actual results:
    DEBUG: page=0, count(pages)=0
    DEBUG: strlen(content)=0

    You’ll never get an excerpt out of that.

    I think it’s bad practice to ask PHP to access an element of an empty array, even if it does not raise a warning. In my particular case, there was a plugin that ran a query that put the state into page=0, count(pages)=1, so a warning was raised. But it’s bad/useless in either case.

    If your post has a post_excerpt, then it will be OK and you won’t reach that code in wp-includes/post-template.php. If you have another filter that sets the excerpt, you’re also OK. But in the default case, you need to create your own excerpt. You are in the header, not in a Loop iteration.

    WordPress uses paging in get_the_content(), called by the default excerpt filter when post_excerpt is empty. Paging is not well-defined in the header.

    Plugin Author Jeremy Herve

    (@jeherve)

    Jetpack Mechanic ??

    I’m afraid I can’t seem to reproduce the issue on my end:
    https://i.wpne.ws/f1ro

    When creating the error, did you use that other plugin you mentioned, or a theme that would change the $page and $pages globals?

    there was a plugin that ran a query that put the state into page=0, count(pages)=1, so a warning was raised.

    I would assume that’s to be expected when $page is 0.

    If you have an proposal to fix this issue in Jetpack without getting rid of get_excerpt() since we use it for Publicize, feel free to submit a patch! We’ll be happy to review it and get it merged and shipped with the next version of Jetpack.

    Thread Starter kitchin

    (@kitchin)

    I’m using no plugins and Twenty Sixteen. I trred it again, this time a manual install without Softalicious. I uploaded a wp-config with WP_DEBUG true (actually does not matter for this test). Then I installed Jetpack, activated, connected to WordPress.com. It asked to me to “Jump Start your site” and I clicked “skip.” This time I did not connect to Twitter (Publicize was already active). Visited home page and clicked on the Hello World post. Then I uploaded the modified wp-includes/post-template.php. Same error. The page shows this four times:

    DEBUG: page=0, count(pages)=0
    DEBUG: strlen(content)=0

    Then “Just another WordPress site.”

    Then something like your screenshot with

    DEBUG: page=1, count(pages)=1
    DEBUG: strlen(content)=85

    Thanks for looking at this. I should have been more clear, the correct debug output is in the Loop. The incorrect debugs, 4 times, are in the html header. They may not be visible depending on styling or perhaps a browser that does not think display belongs in the html header. View source then! ??

    I notice your screen shot does not include “Just another WordPress site.” Is there anything up top?

    Plugin Author Jeremy Herve

    (@jeherve)

    Jetpack Mechanic ??

    Oh, right! I can see the issue now.

    I created a GitHub issue to track this. We’ll see what we can do to address this in the next few weeks:
    https://github.com/Automattic/jetpack/issues/3370

    Thanks again for the report!

    Thread Starter kitchin

    (@kitchin)

    Thanks Jeremy, that get_the_excerpt filter sure does go some odd places.

Viewing 6 replies - 1 through 6 (of 6 total)
  • The topic ‘undefined index warnings due to Jetpack_Media_Summary::get()’ is closed to new replies.