WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 2 years ago

#40784 new defect (bug)

WP_HTTP_IXR_Client constructor needs to check that $server is valid

Reported by: chrispecoraro Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.7.4
Component: HTTP API Keywords: has-patch has-unit-tests
Focuses: Cc:
PR Number:

Description

ErrorException is thrown: Undefined index: host and Undefined index: scheme

When WP_HTTP_IXR_Client is instantiated in pingback(), $path is checked, but $server may still be invalid or false, which sends an invalid URL to parse_url().

Instantiation:
$client = new WP_HTTP_IXR_Client($pingback_server_url);

Excerpt from class:

<?php
class WP_HTTP_IXR_Client extends IXR_Client {
        public $scheme;
        /**
         * @var IXR_Error
         */
        public $error;

        /**
         * @param string $server
         * @param string|bool $path
         * @param int|bool $port
         * @param int $timeout
         */
        public function __construct($server, $path = false, $port = false, $timeout = 15) {
                if ( ! $path ) {
                        // Assume we have been given a URL instead
                        $bits = parse_url($server);
                        $this->scheme = $bits['scheme'];
                        $this->server = $bits['host'];
                        ...

In the code that instantiates WP_HTTP_IXR_Client, it is clear that discover_pingback_server_uri could return false, which then gets passed as a constructor parameter.

Relevant code block:

$pingback_server_url = discover_pingback_server_uri( $pagelinkedto );

		if ( $pingback_server_url ) {
			@ set_time_limit( 60 );
			// Now, the RPC call
			$pagelinkedfrom = get_permalink( $post );

			// using a timeout of 3 seconds should be enough to cover slow servers
			$client = new WP_HTTP_IXR_Client($pingback_server_url);
			$client->timeout = 3;

Possible resolution:

filter_var($server, FILTER_VALIDATE_URL) should be used to test the URL prior to parse_url($server)

Attachments (1)

40784.diff (2.1 KB) - added by tfrommen 2 years ago.

Download all attachments as: .zip

Change History (3)

#1 @drebbits.web
2 years ago

  • Component changed from General to HTTP API

Welcome @chrispecoraro. Thanks for reporting.

Although in the example code block, I'm seeing $pingback_server_url is being checked out around this line if ( $pingback_server_url ) {...}, I'm still convince that we need a check on the $server if it's the type of data we expect it to be here.

@tfrommen
2 years ago

#2 @tfrommen
2 years ago

  • Keywords has-patch has-unit-tests added

Interesting that this exists already. I was about to open a pretty similar ticket.

In my opinion, the problem is not specifically that the URL might be invalid. It could be a valid URL, given in an unexpected format (e.g., protocol-less, or even relative).

I just created a small patch that fixes that, and also confirms that the fix is working by new tests.

Note: See TracTickets for help on using tickets.