WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#37122 closed defect (bug) (fixed)

Error out when xml_create_parser is not available

Reported by: kraftbj Owned by: pento
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: XML-RPC Keywords: has-patch
Focuses: Cc:
PR Number:

Description

When PHP XML is not available, we fatal with an PHP Fatal error: Call to undefined function xml_parser_create() in wp-includes/class-IXR.php and other files.

We already error out when that function is not available in wp-includes/rss.php since [3083].

Background: The PHP 7 packages on Ubuntu, unlike PHP 5, does not include php7.0-xml by default, so there may be an increase in failures of this type as folks switch over.

Attachments (4)

37122.diff (2.6 KB) - added by kraftbj 3 years ago.
Check for xml_parser_create before use
37122.2.diff (4.5 KB) - added by markoheijnen 3 years ago.
37122.3.diff (4.5 KB) - added by markoheijnen 3 years ago.
37122.4.diff (3.6 KB) - added by kraftbj 3 years ago.
updated patch

Download all attachments as: .zip

Change History (23)

#1 @kraftbj
3 years ago

We use that function in the following files, seemingly without a check at first glance:

  • /wp-admin/link-parse-opml.php (long live Links)
  • /wp-includes/atomlib.php
  • /wp-includes/class-simplepie.php
  • /wp-includes/feed.php
  • /wp-includes/SimplePie/Parser.php

and a couple of unit tests.

This ticket was mentioned in Slack in #core by kraft. View the logs.


3 years ago

#3 @kraftbj
3 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.7

@kraftbj
3 years ago

Check for xml_parser_create before use

#4 @kraftbj
3 years ago

  • Keywords has-patch added; needs-patch removed

Attached a patch. SimplePie appears to check for the xml extension directly before running, which likely covers it.

#5 @kraftbj
3 years ago

  • Owner set to kraftbj
  • Status changed from new to accepted

#6 @jorbin
3 years ago

  • Keywords commit added

Nice catch @kraftbj and agree that this makes sense.

#7 @GaryJ
3 years ago

  • Keywords needs-refresh added; has-patch commit removed

The check before the solitary call to xml_parser_create_ns() is a check for xml_parser_create(). While presumably the former will be available when the latter is also available, why not actually check for the function that's about to be called?

For the cases when duplicate code is created (check + call to xml_parser_create()), I think it may make more sense to add in a wp_xml_parser_create() that can handle this in a cleaner way. i.e.

function wp_xml_parser_create( $encoding = null ) {
    if ( ! function_exists( 'xml_parser_create' ) ) { 
        trigger_error( __( 'The XML extension for PHP is not available. Please contact your hosting provider to enable it." ) ); 
    }

    return xml_parser_create( $encoding );
}

Then use wp_xml_parser_create() in the instances where non-library code in WP currently uses xml_parser_create().

Regardless of my suggestion, I think the patch needs a refresh, as some files have been moved around.

#8 @kraftbj
3 years ago

Good call wrt xml_parser_create_ns. I'm neutral on the helper function. I'm about to hit the road, but can refresh it later this week.

@markoheijnen
3 years ago

@markoheijnen
3 years ago

#9 @markoheijnen
3 years ago

  • Keywords has-patch added; needs-refresh removed

I have uploaded a new patch that fixes the check for xml_parser_create_ns but mainly fixed the problem with previous code and first patch is still you still need to return something otherwise it's still fatal errors.

This ticket was mentioned in Slack in #core by stevenkword. View the logs.


3 years ago

#11 @stevenkword
3 years ago

@kraftbj to review the most recent patches.

@kraftbj
3 years ago

updated patch

#12 @kraftbj
3 years ago

I updated the patch some:

  • src/wp-admin/link-parse-opml.php -- I kill processing using wp_die instead of the if/else structure. This file isn't called by Core, seemingly only called by the OPML Importer plugins (perhaps others).
  • src/wp-includes/feed.php - Confirming that the given return makes enough sense. We aren't able to confirm it is XML, so returning as a looser type should be fine. prep_atom_text_construct() doesn't seem to be called by Core either.
  • src/wp-includes/rss.php - I updated the error messages to be consistent with the others (and the php.net link isn't helpful for the PHP 7 issue). Previously, it called the parser while suppressing errors, then checking if it is a php resource. Changed it to be consistent with the other checks.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 years ago

#15 @markoheijnen
3 years ago

Looks good to me but haven't been able to test it on my end as this fixes edge cases (removed functionality)

#16 @pento
3 years ago

  • Owner changed from kraftbj to pento
  • Status changed from accepted to assigned

#17 @pento
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38883:

General: Check to see that the PHP-XML module is enabled before using XML functions.

There are a handful of places where we don't check that the XML functions exist before we use them. Ubuntu's PHP 7 packages don't include PHP-XML by default, increasing the chance of this causing issues.

Props kraftbj, markoheijnen.
Fixes #37122.

#18 @ocean90
3 years ago

In 39007:

XML-RPC: Fix truncated warning message added in [38883].

See #37122.

#19 in reply to: ↑ description @t0nnyh0ang
3 years ago

Replying to kraftbj:

When PHP XML is not available, we fatal with an PHP Fatal error: Call to undefined function xml_parser_create() in wp-includes/class-IXR.php and other files.

We already error out when that function is not available in wp-includes/rss.php since [3083].

Background: The PHP 7 packages on Ubuntu, unlike PHP 5, does not include php7.0-xml by default, so there may be an increase in failures of this type as folks switch over.

Note: See TracTickets for help on using tickets.