Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#28001 closed defect (bug) (fixed)

WP_Http still fails on some requests with relative url redirects

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

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
12 years ago

  • Summary WP_Http still fails on some requests with relative urls → WP_Http still fails on some requests with relative url redirects

#2 @SergeyBiryukov
12 years ago

  • Description modified (diff)

#3 @dd32
12 years ago

  • Milestone Awaiting Review → 4.1

#4 @dd32
12 years ago

  • Owner set to dd32
  • Resolution → fixed
  • Status new → 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
12 years ago

  • Resolution fixed
  • Status closed → reopened

#6 @dd32
12 years ago

  • Resolution → fixed
  • Status reopened → 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
12 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
12 years ago

  • Resolution fixed
  • Status closed → 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
12 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
12 years ago

  • Keywords dev-feedback removed

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

#11 @johnbillion
12 years ago

  • Resolution → fixed
  • Status reopened → closed
Note: See TracTickets for help on using tickets.