From 52903df41cdd9c92d6f007d802f5b299388565b9 Mon Sep 17 00:00:00 2001 From: Ralph Schindler Date: Wed, 5 Mar 2014 11:56:54 -0600 Subject: [PATCH 1/2] Zend\Http: Unit tests for multi-line headers --- test/HeadersTest.php | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/test/HeadersTest.php b/test/HeadersTest.php index a2cefc6c43..875d3e2980 100644 --- a/test/HeadersTest.php +++ b/test/HeadersTest.php @@ -57,7 +57,8 @@ public function testHeadersFromStringFactoryCreatesSingleObjectWithContinuationL $header = $headers->get('fake'); $this->assertInstanceOf('Zend\Http\Header\GenericHeader', $header); $this->assertEquals('Fake', $header->getFieldName()); - $this->assertEquals('foo-bar,blah-blah', $header->getFieldValue()); + // 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() @@ -68,7 +69,8 @@ public function testHeadersFromStringFactoryCreatesSingleObjectWithContinuationL $header = $headers->get('fake'); $this->assertInstanceOf('Zend\Http\Header\GenericHeader', $header); $this->assertEquals('Fake', $header->getFieldName()); - $this->assertEquals('foo-bar,blah-blah', $header->getFieldValue()); + // 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 testHeadersFromStringFactoryCreatesSingleObjectWithHeaderBreakLine() @@ -93,6 +95,17 @@ public function testHeadersFromStringFactoryCreatesSingleObjectWithHeaderBreakLi $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"); + $this->assertEquals(3, $headers->count()); + + // check continued header + $header = $headers->get('X-Another'); + $this->assertEquals('X-Another', $header->getFieldName()); + $this->assertEquals('another X-Actually-A-Continuation:ofSomeKindOfValue', $header->getFieldValue()); + } + public function testHeadersFromStringFactoryThrowsExceptionOnMalformedHeaderLine() { $this->setExpectedException('Zend\Http\Exception\RuntimeException', 'does not match'); From 9775572b4665966c7a6b392961d7db675a3b5791 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 5 Mar 2014 12:47:03 -0600 Subject: [PATCH 2/2] [zendframework/zf2#5616] Revert code from zendframework/zf2#5608 Based on new tests and links to relevant specifications, the code from zendframework/zf2#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. --- src/Headers.php | 16 ++------ test/HeadersTest.php | 60 +--------------------------- test/Response/ResponseStreamTest.php | 4 +- test/ResponseTest.php | 8 ++-- 4 files changed, 12 insertions(+), 76 deletions(-) diff --git a/src/Headers.php b/src/Headers.php index cc6c435282..440b5be327 100644 --- a/src/Headers.php +++ b/src/Headers.php @@ -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[^()><@,;:\"\\/\[\]?=}{ \t]+):.*$/', $line, $matches)) { + if (preg_match('/^(?P[^()><@,;:\"\\/\[\]?=}{ \t]+):.*$/', $line, $matches)) { if ($current) { // a header name was present, then store the current complete line $headers->headersKeys[] = static::createKey($current['name']); @@ -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\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; diff --git a/test/HeadersTest.php b/test/HeadersTest.php index 875d3e2980..32cb27f6ed 100644 --- a/test/HeadersTest.php +++ b/test/HeadersTest.php @@ -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"); @@ -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() @@ -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"); @@ -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() @@ -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"); @@ -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(); diff --git a/test/Response/ResponseStreamTest.php b/test/Response/ResponseStreamTest.php index 0590200918..5d7bf93884 100644 --- a/test/Response/ResponseStreamTest.php +++ b/test/Response/ResponseStreamTest.php @@ -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()); } diff --git a/test/ResponseTest.php b/test/ResponseTest.php index bfc072ad7b..b106b2ae75 100644 --- a/test/ResponseTest.php +++ b/test/ResponseTest.php @@ -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() @@ -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()); } /**