Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#28001 closed defect (bug) (fixed)

WP_Http still fails on some requests with relative url redirects

Reported by: drlightman's profile DrLightman Owned by: dd32's profile dd32
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.9
Component: HTTP API Keywords:
Focuses: Cc:

Description (last modified by SergeyBiryukov)

This is a follow-up to #20434.

WP_Http still fails (A valid URL was not provided.) with some urls with relative url redirects.

I was scraping web.archive.org with a plugin.

The combination of the relative redirects plus this kind of url:

http://web.archive.org/web/20131001160642/http://www.spiegel.de/thema/deutschland_nach_der_bundestagswahl/

is the culprit.

I think the problem is the check on the scheme in ::make_absolute_url where it does:

	// Check for a scheme
	if ( false !== strpos( $maybe_relative_path, '://' ) )
		return $maybe_relative_path;

so that a relative url like /web/20131001160642/http://www.spiegel.de/thema/deutschland_nach_der_bundestagswahl/ it's evaluated as absolute.

Change History (11)

#1 @DrLightman
9 years ago

  • Summary changed from WP_Http still fails on some requests with relative urls to WP_Http still fails on some requests with relative url redirects

#2 @SergeyBiryukov
9 years ago

  • Description modified (diff)

#3 @dd32
9 years ago

  • Milestone changed from Awaiting Review to 4.1

#4 @dd32
9 years ago

  • Owner set to dd32
  • Resolution set to fixed
  • Status changed from new to closed

In 29850:

Correctly handle url's containing url's in WP_HTTP::make_absolute_url().
A valid relative URL could be mistaken for an absolute url if it contained a : in any position of the url.
Fixes #28001

#5 @jorbin
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#6 @dd32
9 years ago

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

In 29861:

Handle deficiencies in PHP's parse_url in older versions of PHP (<5.4.7) in WP_HTTP::make_absolute_url().

In older versions of PHP:

  • parse_url() will fail to parse a url where the scheme break (:) is present in a relative URL's path
  • parse_url() will include the hostname of a schemeless URL in the path component

This handles those two types of URL's by correcting the response from parse_url().

Fixes #28001, #29886

#7 @dd32
9 years ago

In 29864:

Add some unit tests for WP_HTTP::parse_url() to cover the <PHP 5.4.7 compatibility alterations.

These unit tests cover the expected vehaviour of certain combinations of URL's, but makes no attempt to test invalid URL structures, as PHP's behavious for invalid URL's is undefined (Some will be treated as paths, others fail, and it varies between PHP 5.4.7+ and <5.4.7).
This change also makes WP_HTTP::parse_url() protected in order to allow unit testing.
See #28001, #29886

#8 @dd32
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This method should also be used in the helpers in wp-includes/http.php such as wp_http_validate_url(). This probably makes the function better exposed as wp_parse_url() to work around all the different PHP parse_url oddities.

#9 @DrewAPicture
8 years ago

  • Keywords dev-feedback added

@dd32: From your comment:8 seems like there are some leftover cases you wanted to handle here. Were you wanting to still do that in 4.1 or punt for anything left?

#10 @johnbillion
8 years ago

  • Keywords dev-feedback removed

Closing as fixed for 4.1. dd32 will be looking at additional improvements in 4.2.

#11 @johnbillion
8 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.