Skip to content

Commit 17cbc8f

Browse files
committed
[zfcampus/zf-apigility#118] Ensure non-HTTP exception codes are cast to 500
Per zfcampus/zf-apigility#118, this PR modifies `ApiProblem` to cast non-HTTP exception codes to 500 to ensure that valid HTTP status codes are returned for caught exceptions.
1 parent 36dbbbc commit 17cbc8f

File tree

4 files changed

+33
-11
lines changed

4 files changed

+33
-11
lines changed

src/ApiProblem.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,14 @@ public function __construct($status, $detail, $type = null, $title = null, array
140140
}
141141
}
142142

143+
// Ensure a valid HTTP status
144+
if (! is_numeric($status)
145+
|| ($status < 100)
146+
|| ($status > 599)
147+
) {
148+
$status = 500;
149+
}
150+
143151
$this->status = $status;
144152
$this->detail = $detail;
145153
$this->title = $title;

src/Listener/ApiProblemListener.php

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -157,21 +157,13 @@ public function onDispatchError(MvcEvent $e)
157157

158158
// Marshall an ApiProblem and view model based on the exception
159159
$exception = $e->getParam('exception');
160-
if ($exception instanceof ProblemExceptionInterface) {
161-
$problem = new ApiProblem($exception->getCode(), $exception);
162-
} elseif ($exception instanceof \Exception) {
163-
$status = $exception->getCode();
164-
if (0 === $status) {
165-
$status = 500;
166-
}
167-
$problem = new ApiProblem($status, $exception);
168-
} else {
160+
if (! $exception instanceof \Exception) {
169161
// If it's not an exception, do not know what to do.
170162
return;
171163
}
172164

173165
$e->stopPropagation();
174-
$response = new ApiProblemResponse($problem);
166+
$response = new ApiProblemResponse(new ApiProblem($exception->getCode(), $exception));
175167
$e->setResponse($response);
176168
return $response;
177169
}

test/ApiProblemResponseTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,6 @@ public function testComposeApiProblemIsAccessible()
6363
public function testOverridesReasonPhraseIfStatusCodeIsUnknown()
6464
{
6565
$response = new ApiProblemResponse(new ApiProblem(7, 'Random error'));
66-
$this->assertContains('Unknown', $response->getReasonPhrase());
66+
$this->assertContains('Internal Server Error', $response->getReasonPhrase());
6767
}
6868
}

test/ApiProblemTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,4 +215,26 @@ public function testUsesAdditionalDetailsFromExceptionWhenProvided()
215215
$this->assertArrayHasKey('foo', $payload);
216216
$this->assertEquals('bar', $payload['foo']);
217217
}
218+
219+
public function invalidStatusCodes()
220+
{
221+
return array(
222+
'-1' => array(-1),
223+
'0' => array(0),
224+
'7' => array(7), // reported
225+
'14' => array(14), // observed
226+
'600' => array(600),
227+
);
228+
}
229+
230+
/**
231+
* @dataProvider invalidStatusCodes
232+
* @group zf-apigility-118
233+
*/
234+
public function testInvalidHttpStatusCodesAreCastTo500($code)
235+
{
236+
$e = new \Exception('Testing', $code);
237+
$problem = new ApiProblem($code, $e);
238+
$this->assertEquals(500, $problem->status);
239+
}
218240
}

0 commit comments

Comments
 (0)