Skip to content
Merged
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
4 changes: 2 additions & 2 deletions features/security/validate_response_types.feature
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ Feature: Validate response types
And I send a "GET" request to "/dummies/1.invalid"
Then the response status code should be 404
Copy link
Member

Choose a reason for hiding this comment

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

Should be 406 right? (same below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope after all

And the header "Content-Type" should be equal to "application/problem+json; charset=utf-8"
And the JSON node "detail" should be equal to "Not Found"
And the JSON node "detail" should be equal to 'Format "invalid" is not supported'

Scenario: Requesting an invalid format in the Accept header and in the URL should throw an error
When I add "Accept" header equal to "text/invalid"
And I send a "GET" request to "/dummies/1.invalid"
Then the response status code should be 404
And the header "Content-Type" should be equal to "application/problem+json; charset=utf-8"
And the JSON node "detail" should be equal to "Not Found"
And the JSON node "detail" should be equal to 'Format "invalid" is not supported'
2 changes: 1 addition & 1 deletion src/EventListener/AddFormatListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function onKernelRequest(GetResponseEvent $event)
if (null === $routeFormat = $request->attributes->get('_format') ?: null) {
$mimeTypes = array_keys($this->mimeTypes);
} elseif (!isset($this->formats[$routeFormat])) {
throw new NotFoundHttpException('Not Found');
throw new NotFoundHttpException(sprintf('Format "%s" is not supported', $routeFormat));
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say "415 Unsupported Media Type" or "406 Not Acceptable" but definitely not 404 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://httpstatuses.com/415 is about the payload of the request, so let's go with https://httpstatuses.com/406, which looks like a perfect fit for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sroze in fact, 404 was right, and extensions shouldn't be used (in production, in development it's ok although I'm starting to think this causes more issues than it solves).

} else {
$mimeTypes = Request::getMimeTypes($routeFormat);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/EventListener/AddFormatListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public function testAcceptHeaderTakePrecedenceOverRequestFormat()

/**
* @expectedException \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
* @expectedExceptionMessage Not Found
* @expectedExceptionMessage Format "invalid" is not supported
*/
public function testInvalidRouteFormat()
{
Expand Down