Improper HTTP headers generated within WP_Http_Streams->request() method for URI's specifying port

[kamelkev]

Discovered an error while attempting to fetch_feed() for a URI using port :8080.

Reproduction for WordPress 3.9.1 is as follows:

• Log into admin
• Go to "Add Widgets"
• Add an RSS Widget, configure the widget with an RSS feed specifying port 8080

Receive the following error:
RSS Error: A feed could not be found at ​http://example.com:8080/feed.rss. A feed with an invalid mime type may fall victim to this error, or SimplePie was unable to auto-discover it.. Use force_feed() if you are certain this URL is a real feed.

However within your feed server's log file you can clearly see that the port is being omitted from the request, you will instead receive a request on port 80.

Some of the callbacks related to requesting an RSS feed through SimplePie are complex, but I dug down all the way to "class-http.php", where I found the following lines which construct the request header:

if ( $proxy->is_enabled() && $proxy->send_through_proxy( $url ) )
  $strHeaders .= 'Host: ' . $arrURL['host'] . ':' . $arrURL['port'] . "\r\n";
else
  $strHeaders .= 'Host: ' . $arrURL['host'] . "\r\n";

The significant part here is the second branch, which omits specifying the port entirely. The author should have included the port, I am 99.9% sure the the HTTP specification states the port number should accompany the Host: line whenever it is stated. I can clearly see my web clients doing this, so I believe this to be a notable error that needs correction, otherwise the request is effectively broken for such a case.

Proposed changeset to fix the problem is as follows:

924,926c924
< 		if ( $proxy->is_enabled() && $proxy->send_through_proxy( $url ) )
< 			$strHeaders .= 'Host: ' . $arrURL['host'] . ':' . $arrURL['port'] . "\r\n";
< 		else
---
> 		if ( $arrURL['port'] == 80 || $arrURL['port'] == 443 )
927a926,927
> 		else
> 			$strHeaders .= 'Host: ' . $arrURL['host'] . ':' . $arrURL['port'] . "\r\n";

The behavior with respect to port 443 is ambiguous to me. Web clients will actually stamp the port into the header for this case, but I am pretty sure this is a gray area.

[kamelkev]

Pasted the wrong changeset above, patch added instead. I'm sure someone else can format this better to fit the code style...

[SergeyBiryukov]

Introduced in [8620], modified in [10692].

[dd32]

This is correct, The port must be included in the Host header, unless it's the default for the protocol, in which case it's optional. ​http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23 describes that for HTTP/1.1, which is almost the same as 1.0.

Although we should be able to send the default port in 100% of cases, We should probably only send it when it's not optional to avoid any unexpected servers.

The logic needed would be:

• If proxy enabled, always include port
• If ( (http == scheme and port == 80 ) or (https == scheme and port == 443 ) ) drop port
• else: Include port

[kamelkev]

Hi Dion,

Makes sense. I omitted the portion relating to "proxy enabled" in the previous patch, I have attached another that should be closer to what you described, and matched the project whitespace style.

[dd32]

In 29852:

When making a HTTP request to a non-standard port, include the port in the Host header for the Streams HTTP transport. This bring parity to the cURL transport and respects the HTTP RFC.
Props kamelkev for the initial patch; Fixes #28982