Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Commit

Permalink
[zendframework/zendframework#5616] Revert code from zendframework/zen…
Browse files Browse the repository at this point in the history 
…dframework#5608

Based on new tests and links to relevant specifications, the code from zendframework/zendframework#5608
was invalid; header lines may never start with whitespace.

This patch reverts that code, and updates the `fromString()` logic to
capture whitespace on subsequent lines to include in the multi-line
header value. Tests have been updated where necessary to use a regex for
testing the existence of whitespace between multiline values.
  • Loading branch information
@weierophinney
weierophinney committed Mar 5, 2014
1 parent 4812ec5 commit 9775572
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 76 deletions.
16 changes: 4 additions & 12 deletions src/Headers.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public static function fromString($string)
foreach (explode("\r\n", $string) as $line) {

// check if a header name is present
if (preg_match('/^\s*(?P<name>[^()><@,;:\"\\/\[\]?=}{ \t]+):.*$/', $line, $matches)) {
if (preg_match('/^(?P<name>[^()><@,;:\"\\/\[\]?=}{ \t]+):.*$/', $line, $matches)) {
if ($current) {
// a header name was present, then store the current complete line
$headers->headersKeys[] = static::createKey($current['name']);
Expand All @@ -68,17 +68,9 @@ public static function fromString($string)
'name' => $matches['name'],
'line' => trim($line)
);
} elseif (preg_match('/^\s+.*$/', $line, $matches)) {
if ($current) {
// continuation: append to current line
$current['line'] .= trim($line);
} else {
// Line does not match header format!
throw new Exception\RuntimeException(sprintf(
'Line "%s" does not match header format!',
$line
));
}
} elseif (preg_match('/^(?P<ws>\s+).*$/', $line, $matches)) {
// continuation: append to current line
$current['line'] .= "\r\n" . $matches['ws'] . trim($line);
} elseif (preg_match('/^\s*$/', $line)) {
// empty line indicates end of headers
break;
Expand Down
60 changes: 2 additions & 58 deletions test/HeadersTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,6 @@ public function testHeadersFromStringFactoryCreatesSingleObject()
$this->assertEquals('foo-bar', $header->getFieldValue());
}

public function testHeadersFromStringFactoryCreateSingleObjectWithLeadingWhitespaces()
{
$headers = Headers::fromString(" Fake: foo-bar");
$this->assertEquals(1, $headers->count());

$header = $headers->get('fake');
$this->assertInstanceOf('Zend\Http\Header\GenericHeader', $header);
$this->assertEquals('Fake', $header->getFieldName());
$this->assertEquals('foo-bar', $header->getFieldValue());
}

public function testHeadersFromStringFactoryCreatesSingleObjectWithContinuationLine()
{
$headers = Headers::fromString("Fake: foo-bar,\r\n blah-blah");
Expand All @@ -58,19 +47,7 @@ public function testHeadersFromStringFactoryCreatesSingleObjectWithContinuationL
$this->assertInstanceOf('Zend\Http\Header\GenericHeader', $header);
$this->assertEquals('Fake', $header->getFieldName());
// any leading space MAY be replaced by a single space @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html
$this->assertEquals('foo-bar, blah-blah', $header->getFieldValue());
}

public function testHeadersFromStringFactoryCreatesSingleObjectWithContinuationLineAndLeadingWhitespaces()
{
$headers = Headers::fromString(" Fake: foo-bar,\r\n blah-blah");
$this->assertEquals(1, $headers->count());

$header = $headers->get('fake');
$this->assertInstanceOf('Zend\Http\Header\GenericHeader', $header);
$this->assertEquals('Fake', $header->getFieldName());
// any leading space MAY be replaced by a single space @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html
$this->assertEquals('foo-bar, blah-blah', $header->getFieldValue());
$this->assertRegexp("#foo-bar,\r\n\s+blah-blah#", $header->getFieldValue());
}

public function testHeadersFromStringFactoryCreatesSingleObjectWithHeaderBreakLine()
Expand All @@ -84,17 +61,6 @@ public function testHeadersFromStringFactoryCreatesSingleObjectWithHeaderBreakLi
$this->assertEquals('foo-bar', $header->getFieldValue());
}

public function testHeadersFromStringFactoryCreatesSingleObjectWithHeaderBreakLineAndLeadingWhitespaces()
{
$headers = Headers::fromString(" Fake: foo-bar\r\n\r\n");
$this->assertEquals(1, $headers->count());

$header = $headers->get('fake');
$this->assertInstanceOf('Zend\Http\Header\GenericHeader', $header);
$this->assertEquals('Fake', $header->getFieldName());
$this->assertEquals('foo-bar', $header->getFieldValue());
}

public function testHeadersFromStringFactoryRespectsSpecAllowedMultiLineHeaders()
{
$headers = Headers::fromString("Foo: foo-bar\r\nX-Another: another\r\n X-Actually-A-Continuation:ofSomeKindOfValue\r\nX-Another: another\r\n");
Expand All @@ -103,7 +69,7 @@ public function testHeadersFromStringFactoryRespectsSpecAllowedMultiLineHeaders(
// check continued header
$header = $headers->get('X-Another');
$this->assertEquals('X-Another', $header->getFieldName());
$this->assertEquals('another X-Actually-A-Continuation:ofSomeKindOfValue', $header->getFieldValue());
$this->assertRegexp("#another\r\n\s+X-Actually-A-Continuation:ofSomeKindOfValue#", $header->getFieldValue());
}

public function testHeadersFromStringFactoryThrowsExceptionOnMalformedHeaderLine()
Expand All @@ -112,12 +78,6 @@ public function testHeadersFromStringFactoryThrowsExceptionOnMalformedHeaderLine
Headers::fromString("Fake = foo-bar\r\n\r\n");
}

public function testHeadersFromStringFactoryThrowsExceptionOnMalformedHeaderLineAndLeadingWhitespaces()
{
$this->setExpectedException('Zend\Http\Exception\RuntimeException', 'does not match');
Headers::fromString(" Fake = foo-bar\r\n\r\n");
}

public function testHeadersFromStringFactoryCreatesMultipleObjects()
{
$headers = Headers::fromString("Fake: foo-bar\r\nAnother-Fake: boo-baz");
Expand All @@ -134,22 +94,6 @@ public function testHeadersFromStringFactoryCreatesMultipleObjects()
$this->assertEquals('boo-baz', $header->getFieldValue());
}

public function testHeadersFromStringFactoryCreatesMultipleObjectsWithLeadingWhitespaces()
{
$headers = Headers::fromString(" Fake: foo-bar\r\nAnother-Fake: boo-baz");
$this->assertEquals(2, $headers->count());

$header = $headers->get('fake');
$this->assertInstanceOf('Zend\Http\Header\GenericHeader', $header);
$this->assertEquals('Fake', $header->getFieldName());
$this->assertEquals('foo-bar', $header->getFieldValue());

$header = $headers->get('anotherfake');
$this->assertInstanceOf('Zend\Http\Header\GenericHeader', $header);
$this->assertEquals('Another-Fake', $header->getFieldName());
$this->assertEquals('boo-baz', $header->getFieldValue());
}

public function testHeadersFromStringMultiHeaderWillAggregateLazyLoadedHeaders()
{
$headers = new Headers();
Expand Down
4 changes: 2 additions & 2 deletions test/Response/ResponseStreamTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ public function testMultilineHeader()
$this->assertEquals(6, count($response->getHeaders()), 'Header count is expected to be 6');

// Check header integrity
$this->assertEquals('timeout=15,max=100', $response->getHeaders()->get('keep-alive')->getFieldValue());
$this->assertEquals('text/html;charset=iso-8859-1', $response->getHeaders()->get('content-type')->getFieldValue());
$this->assertRegexp("#timeout=15,\r\n\s+max=100#", $response->getHeaders()->get('keep-alive')->getFieldValue());
$this->assertRegexp("#text/html;\r\n\s+charset=iso-8859-1#", $response->getHeaders()->get('content-type')->getFieldValue());
}


Expand Down
8 changes: 4 additions & 4 deletions test/ResponseTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,8 @@ public function testMultilineHeaderNoSpaces()
$this->assertEquals(6, count($response->getHeaders()), 'Header count is expected to be 6');

// Check header integrity
$this->assertEquals('timeout=15,max=100', $response->getHeaders()->get('keep-alive')->getFieldValue());
$this->assertEquals('text/html;charset=iso-8859-1', $response->getHeaders()->get('content-type')->getFieldValue());
$this->assertRegexp("#timeout=15,\r\n\s+max=100#", $response->getHeaders()->get('keep-alive')->getFieldValue());
$this->assertRegexp("#text/html;\r\n\s+charset=iso-8859-1#", $response->getHeaders()->get('content-type')->getFieldValue());
}

public function testMultilineHeader()
Expand All @@ -339,8 +339,8 @@ public function testMultilineHeader()
$this->assertEquals(6, count($response->getHeaders()), 'Header count is expected to be 6');

// Check header integrity
$this->assertEquals('timeout=15,max=100', $response->getHeaders()->get('keep-alive')->getFieldValue());
$this->assertEquals('text/html;charset=iso-8859-1', $response->getHeaders()->get('content-type')->getFieldValue());
$this->assertRegexp("#timeout=15,\r\n\s+max=100#", $response->getHeaders()->get('keep-alive')->getFieldValue());
$this->assertRegexp("#text/html;\r\n\s+charset=iso-8859-1#", $response->getHeaders()->get('content-type')->getFieldValue());
}

/**
Expand Down

0 comments on commit 9775572

Please sign in to comment.