Skip to content

fix(state): do not check content type if no input #6794

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

Merged
merged 3 commits into from
Nov 15, 2024
Merged

fix(state): do not check content type if no input #6794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ff39f5b
fix(state): do not check content type if no input
alexndlm Nov 14, 2024
dcf2ad9
-refactoring
alexndlm Nov 14, 2024
3cfc3ce
-refactoring
alexndlm Nov 14, 2024
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
20 changes: 14 additions & 6 deletions src/State/Provider/ContentNegotiationProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ private function flattenMimeTypes(array $formats): array
*/
private function getInputFormat(HttpOperation $operation, Request $request): ?string
{
if (
false === ($input = $operation->getInput())
|| (\is_array($input) && null === $input['class'])
|| false === $operation->canDeserialize()
) {
return null;
}

$contentType = $request->headers->get('CONTENT_TYPE');
if (null === $contentType || '' === $contentType) {
return null;
Expand All @@ -108,14 +116,14 @@ private function getInputFormat(HttpOperation $operation, Request $request): ?st
return $format;
}

$supportedMimeTypes = [];
foreach ($formats as $mimeTypes) {
foreach ($mimeTypes as $mimeType) {
$supportedMimeTypes[] = $mimeType;
if (!$request->isMethodSafe() && 'DELETE' !== $request->getMethod()) {
$supportedMimeTypes = [];
foreach ($formats as $mimeTypes) {
foreach ($mimeTypes as $mimeType) {
$supportedMimeTypes[] = $mimeType;
}
}
}

if (!$request->isMethodSafe() && 'DELETE' !== $request->getMethod()) {
throw new UnsupportedMediaTypeHttpException(\sprintf('The content-type "%s" is not supported. Supported MIME types are "%s".', $contentType, implode('", "', $supportedMimeTypes)));
}

Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to not set input formats in
https://github.com/api-platform/core/blob/3.4/src/Metadata/Resource/Factory/FormatsResourceMetadataCollectionFactory.php#L79

when deserialize is false? Or do you think that the problem should be handled here? In that case can you check https://gist.github.com/soyuka/beac00bc527d2ea6e703ed27dd67532c/revisions (first is with the suggested patch, the second if we can move the handling of the formats to FormatsResourceMetadataCollectionFactory instead as there's no point having input formats if we don't deserialize imo)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to check if the operation has input in the ContentNegotiationProvider even if we do not set the input format in FormatsResourceMetadataCollectionFactory.

So, to avoid overcomplicating the code and possibly not introducing some bugs or BC, I prefer not to change the FormatsResourceMetadataCollectionFactory logic.

And I apply your suggestion.

Expand Down
60 changes: 60 additions & 0 deletions tests/State/Provider/ContentNegotiationProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\UnsupportedMediaTypeHttpException;

class ContentNegotiationProviderTest extends TestCase
{
Expand Down Expand Up @@ -60,4 +61,63 @@ public function testRequestWithEmptyContentType(): void

$this->assertSame($expectedResult, $result);
}

public function testRequestWhenNoInput(): void
{
$expectedResult = new \stdClass();

$decorated = $this->prophesize(ProviderInterface::class);
$decorated->provide(Argument::cetera())->willReturn($expectedResult);

$negotiator = new Negotiator();
$formats = ['jsonld' => ['application/ld+json']];
$errorFormats = ['jsonld' => ['application/ld+json']];

$provider = new ContentNegotiationProvider($decorated->reveal(), $negotiator, $formats, $errorFormats);

$request = new Request(
server: [
'REQUEST_METHOD' => 'POST',
'REQUEST_URI' => '/',
'CONTENT_TYPE' => 'some-not-supported/content-type',
],
content: ''
);

$operation = new Post();
$operation = $operation->withDeserialize(false);
$context = ['request' => $request];

$result = $provider->provide($operation, [], $context);

$this->assertSame($expectedResult, $result);
}

public function testRequestWithInput(): void
{
$this->expectException(UnsupportedMediaTypeHttpException::class);

$decorated = $this->prophesize(ProviderInterface::class);

$negotiator = new Negotiator();
$formats = ['jsonld' => ['application/ld+json']];
$errorFormats = ['jsonld' => ['application/ld+json']];

$provider = new ContentNegotiationProvider($decorated->reveal(), $negotiator, $formats, $errorFormats);

$request = new Request(
server: [
'REQUEST_METHOD' => 'POST',
'REQUEST_URI' => '/',
'CONTENT_TYPE' => 'some-not-supported/content-type',
],
content: ''
);

$operation = new Post();
$operation = $operation->withDeserialize();
$context = ['request' => $request];

$provider->provide($operation, [], $context);
}
}
Loading