"Invalid header line detected" error if HTTP header value is empty

[weierophinney]

In Version 1.12.13 is a bug which is not in Version 1.12.11.

If an HTTP header value is empty I get the Exception "Invalid header line detected" in Zend_Http_Response line 573

This happens because the regex in line 528 does not match if the value is empty.
As far as I know empty values are allowed in HTTP 1.1.

If I change the regex in line 528 from
"|^([\w-]+):\s_(.+)|s"
to
"|^([\w-]+):\s_(.*)|s"
it works fine. So just replacing the + by a *.

It would be nice if this could be fixed in the code, because this breaks existing code.
For me it broke the feed reader, which is fetching and parsing an RSS feed where one HTTP response header value is empty.

[froschdesign]

This happens because the regex in line 528 does not match if the value is empty.

Are you sure?

This is the old version:

if (preg_match("|^([\w-]+):\s*(.+)|", $line, $m)) {

This is the new version:

if (preg_match("|^([\w-]+):\s*(.+)|s", $line, $m)) {

More informations to the changes in version 1.12.12: ZF2015-04: Potential CRLF injection attacks in mail and HTTP headers

[PHPGangsta]

Hi Frank,

yes, that's what I'm seeing, I'm sure ;-)

Maybe the difference is that the explode() in the past (<=1.12.11) was done with
$lines = explode("\n", $parts[0]);

So there maybe was a \r left, and that \r was matching the (.+) in the old version.

In the current version it's
$lines = explode("\r\n", $parts[0]);

I just tested it again, 1.12.11 is working fine. If I switch to 1.12.13 it's not working and I get the Exception. If I change the + to * it's working again.

This is an example URL that fails:
http://rss.sueddeutsche.de/rss/Politik

You can try it yourself with this simple stripped down example:
Zend_Http_Response::extractHeaders("Expires:300\r\nImagetoolbar:\r\nPragma:no-cache");

[froschdesign]

Hallo Michael,

yes, that's what I'm seeing, I'm sure ;-)

😉

This is an example URL that fails:
http://rss.sueddeutsche.de/rss/Politik

Good example!

You can try it yourself…

I trust you.

[LaneNelsonHD]

Don't know if this is the right place to put this, but we just upgraded to 1.12.13 (from 1.12.3), and found that the Regex cited above would not recognize an Http Header "zipi.step: 0".
Our analysis indicates that the regex "|^([\w-]+):\s*(.+)|s" used to match headers does not recognize the dot/period in zipi.step, and the header is rejected. The field name zipi.step appears to comply with RFC7230 (see the definition of a token on p 83), and that there might be additional characters (tchars in the specification, also on p 83) that may be acceptable when used in header field names but which would be rejected by the Regex. As long as you are fixing the blank space problem, perhaps you could expand the acceptable characters to at least include the dot/period?

[akrabat]

ping @weierophinney for comment.

[weierophinney]

Indeed, the regex for header names should be:

/^[a-zA-Z0-9\'`#$%&*+.^_|~!-]+$/

to ensure that any valid token is allowed.

Is anybody working on a patch at this point? If so, where?

[akrabat]

@weierophinney No one is working on this at the moment as the last time it was touched was for a security fix.

[weierophinney]

I've converted this issue to a pull request, with an attached patch that:

• Defines tests for the original reported issue (empty header values were causing the parser to skip the header line)
• Defines a test for the issue reported by @LaneNelsonHD (valid header names per RFC 7230 were not being matched).
• Adds code to correct the behavior.

Please review, and let me know if the solutions work for you.

[weierophinney]

Tests on 5.2 fail currently as they are blocked on #591; however, relevant test results start here:

• https://travis-ci.org/zendframework/zf1/jobs/74285816#L695

Tests pass on other required platforms, and the HTTP tests pass on both PHP 7 and HHVM. (Not sure why PHP 7 shows failing status; I do not see any test failures in the run.)

[PHPGangsta]

Looks good to me, Matthew! Thanks for the fixes!

[akrabat]

Thanks Matthew.

[LaneNelsonHD]

Looks like it solves our problems as well. Many thanks!