Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#28982 closed defect (bug) (fixed)

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

Reported by: kamelkev's profile kamelkev Owned by: dd32's profile dd32
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.9.1
Component: HTTP API Keywords: needs-patch
Focuses: Cc:


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 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";
  $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:

< 		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 )
> 		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.

Attachments (2)

proposed.patch (688 bytes) - added by kamelkev 10 years ago.
patch.txt (596 bytes) - added by kamelkev 10 years ago.

Download all attachments as: .zip

Change History (9)

10 years ago

#1 @kamelkev
10 years ago

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

#2 @SergeyBiryukov
10 years ago

  • Component changed from Feeds to HTTP API

#3 @SergeyBiryukov
10 years ago

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

#4 @dd32
10 years ago

  • Keywords needs-patch added

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. 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
Last edited 10 years ago by dd32 (previous) (diff)

10 years ago

#5 @kamelkev
10 years ago

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.

Last edited 10 years ago by kamelkev (previous) (diff)

#6 @dd32
10 years ago

  • Milestone changed from Awaiting Review to 4.1

#7 @dd32
10 years ago

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

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

Note: See TracTickets for help on using tickets.