Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Assert values according to PSR standard #250

Merged
merged 6 commits into from Jun 5, 2019
Merged

Assert values according to PSR standard #250

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9ceb5db
Assert values according to PSR standard
gmponos Dec 8, 2018
53c3278
Added more assertions
gmponos Dec 8, 2018
df60ffa
Add tests for changes of assertions
gmponos Dec 9, 2018
42ad61c
Merge branch 'master' into issue#249_tests
gmponos Dec 29, 2018
7b128a3
Removed docblocks
gmponos Dec 29, 2018
d67db57
Improve code by removing duplication
gmponos Dec 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/MessageTrait.php
View file
Open in desktop
Expand Up @@ -66,8 +66,17 @@ public function getHeaderLine($header)

public function withHeader($header, $value)
{
$this->assertHeader($header);
if (!is_array($value)) {
if (!is_string($value)) {
throw new \InvalidArgumentException('Header value must be a string or an array of strings.');
}

$value = [$value];
} else {
if (count($value) === 0){
throw new \InvalidArgumentException('Header value can not be an empty array.');
}
}

$value = $this->trimHeaderValues($value);
Expand All @@ -85,8 +94,17 @@ public function withHeader($header, $value)

public function withAddedHeader($header, $value)
{
$this->assertHeader($header);
if (!is_array($value)) {
if (!is_string($value)) {
throw new \InvalidArgumentException('Header value must be a string or an array of strings.');
}

$value = [$value];
} else {
if (count($value) === 0){
throw new \InvalidArgumentException('Header value can not be an empty array.');
}
}

$value = $this->trimHeaderValues($value);
Expand Down Expand Up @@ -144,8 +162,17 @@ private function setHeaders(array $headers)
{
$this->headerNames = $this->headers = [];
foreach ($headers as $header => $value) {
$this->assertHeader($header);
if (!is_array($value)) {
if (!is_string($value)) {
throw new \InvalidArgumentException('Header value must be a string or an array of strings.');
}

$value = [$value];
} else {
if (count($value) === 0){
throw new \InvalidArgumentException('Header value can not be an empty array.');
}
}

$value = $this->trimHeaderValues($value);
Expand Down Expand Up @@ -177,7 +204,20 @@ private function setHeaders(array $headers)
private function trimHeaderValues(array $values)
{
return array_map(function ($value) {
if (!is_string($value)) {
throw new \InvalidArgumentException('Header value must be a string.');
}
return trim($value, " \t");
}, $values);
}

/**
* @param string $header
sagikazarmark marked this conversation as resolved.
Show resolved Hide resolved
*/
private function assertHeader($header)
{
if (!is_string($header) || $header === '') {
throw new \InvalidArgumentException('Header must be a non empty string.');
Copy link
Member

Choose a reason for hiding this comment

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

typo: non-empty

}
}
}
12 changes: 12 additions & 0 deletions src/Request.php
View file
Open in desktop
Expand Up @@ -36,6 +36,7 @@ public function __construct(
$body = null,
$version = '1.1'
) {
$this->assertMethod($method);
if (!($uri instanceof UriInterface)) {
$uri = new Uri($uri);
}
Expand Down Expand Up @@ -91,6 +92,7 @@ public function getMethod()

public function withMethod($method)
{
$this->assertMethod($method);
$new = clone $this;
$new->method = strtoupper($method);
Copy link
Member Author

@gmponos gmponos Dec 8, 2018 •
edited

Choose a reason for hiding this comment

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

According to the tests the strtoupper should not be here.. the current PR targets 1,x.. Can this be fixed on v1. and add the change on the current PR or it should be fixed on version 2.x?

return $new;
Expand Down Expand Up @@ -139,4 +141,14 @@ private function updateHostFromUri()
// See: http://tools.ietf.org/html/rfc7230#section-5.4
$this->headers = [$header => [$host]] + $this->headers;
}

/**
* @param string $method
*/
private function assertMethod($method)
{
if (!is_string($method) || $method === '') {
throw new \InvalidArgumentException('Method should be a non empty string.');
Copy link
Member

Choose a reason for hiding this comment

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

👍 method is defined as token = 1*tchar

}
}
}
32 changes: 28 additions & 4 deletions src/Response.php
View file
Open in desktop
Expand Up @@ -93,9 +93,9 @@ public function __construct(
$version = '1.1',
$reason = null
) {
if (filter_var($status, FILTER_VALIDATE_INT) === false) {
throw new \InvalidArgumentException('Status code must be an integer value.');
}
$this->assertStatusCodeIsInteger($status);
$status = (int) $status;
$this->assertStatusCodeRange($status);

$this->statusCode = (int) $status;
Copy link
Member

Choose a reason for hiding this comment

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

you can remove this cast since you casted already a few lines above


Expand Down Expand Up @@ -125,12 +125,36 @@ public function getReasonPhrase()

public function withStatus($code, $reasonPhrase = '')
{
$this->assertStatusCodeIsInteger($code);
$code = (int) $code;
$this->assertStatusCodeRange($code);

$new = clone $this;
$new->statusCode = (int) $code;
$new->statusCode = $code;
if ($reasonPhrase == '' && isset(self::$phrases[$new->statusCode])) {
$reasonPhrase = self::$phrases[$new->statusCode];
}
$new->reasonPhrase = $reasonPhrase;
return $new;
}

/**
* @param int $statusCode
*/
private function assertStatusCodeIsInteger($statusCode)
{
if (filter_var($statusCode, FILTER_VALIDATE_INT) === false) {
throw new \InvalidArgumentException('Status code must be an integer value.');
}
}

/**
* @param int $statusCode
*/
private function assertStatusCodeRange($statusCode)
{
if ($statusCode < 100 || $statusCode >= 600) {
Copy link
Member

Choose a reason for hiding this comment

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

👍 according to https://tools.ietf.org/html/rfc7231#section-6

throw new \InvalidArgumentException('Status code must be an integer value between 1xx and 5xx.');
}
}
}
30 changes: 30 additions & 0 deletions tests/RequestTest.php
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@
use GuzzleHttp\Psr7\Uri;

/**
* @covers GuzzleHttp\Psr7\MessageTrait
* @covers GuzzleHttp\Psr7\Request
*/
class RequestTest extends BaseTest
Expand Down Expand Up @@ -90,6 +91,35 @@ public function testWithUri()
$this->assertSame($u1, $r1->getUri());
}

/**
* @dataProvider invalidMethodsProvider
*/
public function testConstructWithInvalidMethods($method)
{
$this->expectException('InvalidArgumentException');
new Request($method, '/');
}

/**
* @dataProvider invalidMethodsProvider
*/
public function testWithInvalidMethods($method)
{
$r = new Request('get', '/');
$this->expectException('InvalidArgumentException');
$r->withMethod($method);
}

public function invalidMethodsProvider()
{
return [
[null],
[false],
[['foo']],
[new \stdClass()],
];
}

public function testSameInstanceWhenSameUri()
{
$r1 = new Request('GET', 'http://foo.com');
Expand Down
91 changes: 88 additions & 3 deletions tests/ResponseTest.php
View file
Open in desktop
Expand Up @@ -237,6 +237,51 @@ public function testSameInstanceWhenRemovingMissingHeader()
$this->assertSame($r, $r->withoutHeader('foo'));
}

/**
* @dataProvider invalidHeaderProvider
*/
public function testConstructResponseInvalidHeader($header, $headerValue, $expectedMessage)
{
$this->expectException('InvalidArgumentException', $expectedMessage);
new Response(200, [$header => $headerValue]);
}

public function invalidHeaderProvider()
{
return [
['foo', [], 'Header value can not be an empty array.'],
['', '', 'Header must be a non empty string.'],
['foo', false, 'Header value must be a string or an array of strings.'],
[false, 'foo', 'Header must be a non empty string.'],
['foo', new \stdClass(), 'Header value must be a string or an array of strings.'],
['foo', new \ArrayObject(), 'Header value must be a string or an array of strings.'],
];
}

/**
* @dataProvider invalidWithHeaderProvider
*/
public function testWithInvalidHeader($header, $headerValue, $expectedMessage)
{
$r = new Response();
$this->expectException('InvalidArgumentException', $expectedMessage);
$r->withHeader($header, $headerValue);
}

public function invalidWithHeaderProvider()
{
return [
[[], 'foo', 'Header must be a non empty string.'],
['foo', [], 'Header value can not be an empty array.'],
['', '', 'Header must be a non empty string.'],
['foo', false, 'Header value must be a string or an array of strings.'],
[false, 'foo', 'Header must be a non empty string.'],
['foo', new \stdClass(), 'Header value must be a string or an array of strings.'],
['foo', new \ArrayObject(), 'Header value must be a string or an array of strings.'],
[new \stdClass(), 'foo', 'Header must be a non empty string.'],
];
}

public function testHeaderValuesAreTrimmed()
{
$r1 = new Response(200, ['OWS' => " \t \tFoo\t \t "]);
Expand All @@ -251,16 +296,27 @@ public function testHeaderValuesAreTrimmed()
}

/**
* @dataProvider responseInitializedWithNonIntegerStatusCodeProvider
* @dataProvider nonIntegerStatusCodeProvider
* @param mixed $invalidValues
*/
public function testResponseInitializedWithNonIntegerStatusCodeProvider($invalidValues)
public function testConstructResponseWithNonIntegerStatusCode($invalidValues)
{
$this->expectException('InvalidArgumentException', 'Status code must be an integer value.');
new Response($invalidValues);
}

public function responseInitializedWithNonIntegerStatusCodeProvider()
/**
* @dataProvider nonIntegerStatusCodeProvider
* @param mixed $invalidValues
*/
public function testResponseChangeStatusCodeWithNonInteger($invalidValues)
{
$response = new Response();
$this->expectException('InvalidArgumentException', 'Status code must be an integer value.');
$response->withStatus($invalidValues);
}

public function nonIntegerStatusCodeProvider()
{
return [
['whatever'],
Expand All @@ -269,4 +325,33 @@ public function responseInitializedWithNonIntegerStatusCodeProvider()
[new \stdClass()],
];
}

/**
* @dataProvider invalidStatusCodeRangeProvider
* @param mixed $invalidValues
*/
public function testConstructResponseWithInvalidRangeStatusCode($invalidValues)
{
$this->expectException('InvalidArgumentException', 'Status code must be an integer value between 1xx and 5xx.');
new Response($invalidValues);
}

/**
* @dataProvider invalidStatusCodeRangeProvider
* @param mixed $invalidValues
*/
public function testResponseChangeStatusCodeWithWithInvalidRange($invalidValues)
{
$response = new Response();
$this->expectException('InvalidArgumentException', 'Status code must be an integer value between 1xx and 5xx.');
$response->withStatus($invalidValues);
}

public function invalidStatusCodeRangeProvider()
{
return [
[600],
[99],
];
}
}