From dd347c8a9aafcd993676c7a9f82c1fea1c54c96c Mon Sep 17 00:00:00 2001 From: Lode Claassen Date: Sat, 15 May 2021 15:40:58 +0200 Subject: [PATCH 1/5] add test coverage for broken json request body --- tests/helpers/RequestParserTest.php | 11 +++++++++++ tests/helpers/TestableNonInterfaceStreamInterface.php | 5 ++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/helpers/RequestParserTest.php b/tests/helpers/RequestParserTest.php index 6410c78b..efc3db23 100644 --- a/tests/helpers/RequestParserTest.php +++ b/tests/helpers/RequestParserTest.php @@ -178,6 +178,17 @@ public function testFromPsrRequest_WithEmptyDocument() { $this->assertSame([], $requestParser->getDocument()); } + public function testFromPsrRequest_WithBrokenDocument() { + $selfLink = ''; + $queryParameters = []; + $document = '{"data": {'; + + $request = new TestableNonInterfaceRequestInterface($selfLink, $queryParameters, $document); + $requestParser = RequestParser::fromPsrRequest($request); + + $this->assertSame([], $requestParser->getDocument()); + } + public function testFromPsrRequest_WithServerRequestInterface() { $queryParameters = [ 'sort' => 'name,-location', diff --git a/tests/helpers/TestableNonInterfaceStreamInterface.php b/tests/helpers/TestableNonInterfaceStreamInterface.php index d5739880..87a1d256 100644 --- a/tests/helpers/TestableNonInterfaceStreamInterface.php +++ b/tests/helpers/TestableNonInterfaceStreamInterface.php @@ -19,8 +19,11 @@ public function getContents() { if ($this->document === null) { return ''; } + if (is_string($this->document) === false) { + return (string) json_encode($this->document); + } - return (string) json_encode($this->document); + return $this->document; } // not used in current implementation From 0cc8b5f0544421086ac64086137e515cab3d82f2 Mon Sep 17 00:00:00 2001 From: Lode Claassen Date: Sat, 15 May 2021 15:43:27 +0200 Subject: [PATCH 2/5] actually throw exceptions on broken json request body --- src/helpers/RequestParser.php | 8 +++++--- tests/helpers/RequestParserTest.php | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/helpers/RequestParser.php b/src/helpers/RequestParser.php index 5ae8f250..7ba2ebf0 100644 --- a/src/helpers/RequestParser.php +++ b/src/helpers/RequestParser.php @@ -3,6 +3,7 @@ namespace alsvanzelf\jsonapi\helpers; use alsvanzelf\jsonapi\Document; +use alsvanzelf\jsonapi\exceptions\Exception; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ServerRequestInterface; @@ -73,6 +74,8 @@ public static function fromSuperglobals() { /** * @param ServerRequestInterface|RequestInterface $request * @return self + * + * @throws Exception if the body's json is invalid */ public static function fromPsrRequest(RequestInterface $request) { $selfLink = (string) $request->getUri(); @@ -90,9 +93,8 @@ public static function fromPsrRequest(RequestInterface $request) { } else { $document = json_decode($request->getBody()->getContents(), true); - - if ($document === null) { - $document = []; + if (json_last_error() !== JSON_ERROR_NONE) { + throw new Exception('error parsing request body: '.json_last_error_msg()); } } diff --git a/tests/helpers/RequestParserTest.php b/tests/helpers/RequestParserTest.php index efc3db23..6db8560f 100644 --- a/tests/helpers/RequestParserTest.php +++ b/tests/helpers/RequestParserTest.php @@ -3,6 +3,7 @@ namespace alsvanzelf\jsonapiTests\helpers; use alsvanzelf\jsonapi\Document; +use alsvanzelf\jsonapi\exceptions\Exception; use alsvanzelf\jsonapi\helpers\RequestParser; use alsvanzelf\jsonapiTests\helpers\TestableNonInterfaceRequestInterface; use alsvanzelf\jsonapiTests\helpers\TestableNonInterfaceServerRequestInterface; @@ -182,11 +183,12 @@ public function testFromPsrRequest_WithBrokenDocument() { $selfLink = ''; $queryParameters = []; $document = '{"data": {'; + $request = new TestableNonInterfaceRequestInterface($selfLink, $queryParameters, $document); - $request = new TestableNonInterfaceRequestInterface($selfLink, $queryParameters, $document); - $requestParser = RequestParser::fromPsrRequest($request); + $this->expectException(Exception::class); + $this->expectExceptionMessage('error parsing request body: unexpected end of data'); - $this->assertSame([], $requestParser->getDocument()); + RequestParser::fromPsrRequest($request); } public function testFromPsrRequest_WithServerRequestInterface() { From fcd9ee5bff68f7f7f849133bacdc1dbeced2afa8 Mon Sep 17 00:00:00 2001 From: Lode Claassen Date: Sat, 15 May 2021 15:43:58 +0200 Subject: [PATCH 3/5] also throw on invalid json in superglobals this is not testable as php://input isn't, thus reducing coverage again :( --- src/helpers/RequestParser.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/helpers/RequestParser.php b/src/helpers/RequestParser.php index 7ba2ebf0..7d851138 100644 --- a/src/helpers/RequestParser.php +++ b/src/helpers/RequestParser.php @@ -45,6 +45,8 @@ public function __construct($selfLink='', array $queryParameters=[], array $docu /** * @return self + * + * @throws Exception if the body's json is invalid */ public static function fromSuperglobals() { $selfLink = ''; @@ -61,6 +63,9 @@ public static function fromSuperglobals() { if ($documentIsJsonapi || $documentIsJson) { $document = json_decode(file_get_contents('php://input'), true); + if (json_last_error() !== JSON_ERROR_NONE) { + throw new Exception('error parsing request body: '.json_last_error_msg()); + } if ($document === null) { $document = []; From 0faceab1249e1b722a315c8eab9bb74afd12f09a Mon Sep 17 00:00:00 2001 From: Lode Claassen Date: Sat, 15 May 2021 15:47:05 +0200 Subject: [PATCH 4/5] use another json error to test which works better across env --- tests/helpers/RequestParserTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/helpers/RequestParserTest.php b/tests/helpers/RequestParserTest.php index 6db8560f..e2b237d1 100644 --- a/tests/helpers/RequestParserTest.php +++ b/tests/helpers/RequestParserTest.php @@ -182,11 +182,11 @@ public function testFromPsrRequest_WithEmptyDocument() { public function testFromPsrRequest_WithBrokenDocument() { $selfLink = ''; $queryParameters = []; - $document = '{"data": {'; + $document = '{"data": {foo: "bar"}}'; $request = new TestableNonInterfaceRequestInterface($selfLink, $queryParameters, $document); $this->expectException(Exception::class); - $this->expectExceptionMessage('error parsing request body: unexpected end of data'); + $this->expectExceptionMessage('error parsing request body: quoted object property name expected'); RequestParser::fromPsrRequest($request); } From 2a721605e3993496ba90b8cefc787576e29ed773 Mon Sep 17 00:00:00 2001 From: Lode Claassen Date: Sat, 15 May 2021 15:52:43 +0200 Subject: [PATCH 5/5] test only the generic part of the exception --- tests/helpers/RequestParserTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/helpers/RequestParserTest.php b/tests/helpers/RequestParserTest.php index e2b237d1..7e186cc1 100644 --- a/tests/helpers/RequestParserTest.php +++ b/tests/helpers/RequestParserTest.php @@ -186,7 +186,7 @@ public function testFromPsrRequest_WithBrokenDocument() { $request = new TestableNonInterfaceRequestInterface($selfLink, $queryParameters, $document); $this->expectException(Exception::class); - $this->expectExceptionMessage('error parsing request body: quoted object property name expected'); + $this->expectExceptionMessage('error parsing request body: '); RequestParser::fromPsrRequest($request); }