Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/Header/ContentLength.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,21 @@ public static function fromString($headerLine)
return $header;
}

public function __construct($value = null)
public function __construct($value)
{
if ($value) {
HeaderValue::assertValid($value);
$this->value = $value;
$this->setFieldValue($value);
}

public function setFieldValue($value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a setFieldValue() function is considered a new feature and it should be done with a new PR.

{
$value = (string) $value;
if (!HeaderValue::isValid($value) || !preg_match('/^(0|[1-9][0-9]*)$/', $value)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check for digit number using false !== filter_var($value, FILTER_VALIDATE_INT) instead of regex.

throw new Exception\InvalidArgumentException(sprintf(
'Invalid header value for Content-Length: "%s"',
$value
));
}
$this->value = $value;
}

public function getFieldName()
Expand Down
79 changes: 56 additions & 23 deletions test/Header/ContentLengthTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,56 +13,89 @@

class ContentLengthTest extends \PHPUnit_Framework_TestCase
{
public function testContentLengthFromStringCreatesValidContentLengthHeader()
public function testFromStringCreatesValidContentLengthHeader()
{
$contentLengthHeader = ContentLength::fromString('Content-Length: xxx');
$contentLengthHeader = ContentLength::fromString('Content-Length: 123');
$this->assertInstanceOf('Zend\Http\Header\HeaderInterface', $contentLengthHeader);
$this->assertInstanceOf('Zend\Http\Header\ContentLength', $contentLengthHeader);
}

public function testContentLengthGetFieldNameReturnsHeaderName()
public function testGetFieldNameReturnsHeaderName()
{
$contentLengthHeader = new ContentLength();
$contentLengthHeader = new ContentLength('123');
$this->assertEquals('Content-Length', $contentLengthHeader->getFieldName());
}

public function testContentLengthGetFieldValueReturnsProperValue()
public function testGetFieldValueReturnsProperValue()
{
$this->markTestIncomplete('ContentLength needs to be completed');

$contentLengthHeader = new ContentLength();
$this->assertEquals('xxx', $contentLengthHeader->getFieldValue());
$contentLengthHeader = new ContentLength('123');
$this->assertEquals('123', $contentLengthHeader->getFieldValue());
}

public function testContentLengthToStringReturnsHeaderFormattedString()
public function testToStringReturnsHeaderFormattedString()
{
$this->markTestIncomplete('ContentLength needs to be completed');
$contentLengthHeader = new ContentLength('12345');
$this->assertSame('Content-Length: 12345', $contentLengthHeader->toString());
}

$contentLengthHeader = new ContentLength();
public function testCastValueToString()
{
$contentLengthHeader = new ContentLength(12345);
$this->assertEquals('12345', $contentLengthHeader->getFieldValue());
}

// @todo set some values, then test output
$this->assertEmpty('Content-Length: xxx', $contentLengthHeader->toString());
public function testAllowZeroValue()
{
$contentLengthHeader = new ContentLength('0');
$this->assertEquals('0', $contentLengthHeader->getFieldValue());
}

/** Implementation specific tests here */
public function testAllowBigIntValue()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you removed this test? Is still relevant for reference.

{
$power95of2 = '39614081257132168796771975168';
$contentLengthHeader = new ContentLength($power95of2);
$this->assertEquals($power95of2, $contentLengthHeader->getFieldValue());
}

/**
* @see http://en.wikipedia.org/wiki/HTTP_response_splitting
* @group ZF2015-04
* @dataProvider invalidValues
*/
public function testPreventsCRLFAttackViaFromString()
public function testInvalidValueThrowsExceptionOnConstructor($invalidValue)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, see comment above.

{
$this->setExpectedException('Zend\Http\Header\Exception\InvalidArgumentException');
$header = ContentLength::fromString("Content-Length: xxx\r\n\r\nevilContent");
new ContentLength($invalidValue);
}

/**
* @see http://en.wikipedia.org/wiki/HTTP_response_splitting
* @group ZF2015-04
* @dataProvider invalidValues
*/
public function testPreventsCRLFAttackViaConstructor()
public function testInvalidValueThrowsExceptionOnSetFieldValue($invalidValue)
{
$this->setExpectedException('Zend\Http\Header\Exception\InvalidArgumentException');
$header = new ContentLength("Content-Length: xxx\r\n\r\nevilContent");
$header = new ContentLength('123');
$header->setFieldValue($invalidValue);
}

/**
* Data provider of invalid values
* @return array
*/
public function invalidValues()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what are you doing here and it's fine but we prefer to add only unit tests relevant for the PR.

{
return [
// invalid number
['abc'],
['1a1'],
['a1a'],

// invalid integer
['123.456'],
['0x12'],
['xFF'],
[' 1 '],

// http://en.wikipedia.org/wiki/HTTP_response_splitting
['123\r\n\r\nevilContent'],
];
}
}