Skip to content

fix(state): only emit Content-Type charset for media types that define it #8226

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

Open
soyuka wants to merge 1 commit into
base: 4.3
Choose a base branch
from
+337 −314
Open

fix(state): only emit Content-Type charset for media types that define it #8226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Laravel/Tests/AuthTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function testGetCollection(): void
{
$response = $this->get('/api/vaults', ['accept' => ['application/ld+json']]);
$this->assertArraySubset(['detail' => 'Unauthenticated.'], $response->json());
$response->assertHeader('content-type', 'application/problem+json; charset=utf-8');
$response->assertHeader('content-type', 'application/problem+json');
$response->assertStatus(401);
}

Expand Down
8 changes: 4 additions & 4 deletions src/Laravel/Tests/DocsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,28 @@ public function testOpenApi(): void
{
$res = $this->get('/api/docs.jsonopenapi');
$this->assertArrayHasKey('openapi', $res->json());
$this->assertSame('application/vnd.openapi+json; charset=utf-8', $res->headers->get('content-type'));
$this->assertSame('application/vnd.openapi+json', $res->headers->get('content-type'));
}

public function testOpenApiAccept(): void
{
$res = $this->get('/api/docs', headers: ['accept' => 'application/vnd.openapi+json']);
$this->assertArrayHasKey('openapi', $res->json());
$this->assertSame('application/vnd.openapi+json; charset=utf-8', $res->headers->get('content-type'));
$this->assertSame('application/vnd.openapi+json', $res->headers->get('content-type'));
}

public function testJsonLd(): void
{
$res = $this->get('/api/docs.jsonld');
$this->assertArrayHasKey('@context', $res->json());
$this->assertSame('application/ld+json; charset=utf-8', $res->headers->get('content-type'));
$this->assertSame('application/ld+json', $res->headers->get('content-type'));
}

public function testJsonLdAccept(): void
{
$res = $this->get('/api/docs', headers: ['accept' => 'application/ld+json']);
$this->assertArrayHasKey('@context', $res->json());
$this->assertSame('application/ld+json; charset=utf-8', $res->headers->get('content-type'));
$this->assertSame('application/ld+json', $res->headers->get('content-type'));
}

public function testHtmlDocsRendersSwaggerUiByDefault(): void
Expand Down
6 changes: 3 additions & 3 deletions src/Laravel/Tests/HalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function testGetEntrypoint(): void
{
$response = $this->get('/api/', ['accept' => ['application/hal+json']]);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/hal+json; charset=utf-8');
$response->assertHeader('content-type', 'application/hal+json');

$this->assertJsonContains(
[
Expand All @@ -67,7 +67,7 @@ public function testGetCollection(): void
BookFactory::new()->has(AuthorFactory::new())->count(10)->create();
$response = $this->get('/api/books', ['accept' => 'application/hal+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/hal+json; charset=utf-8');
$response->assertHeader('content-type', 'application/hal+json');
$this->assertJsonContains(
[
'_links' => [
Expand All @@ -88,7 +88,7 @@ public function testGetBook(): void
$iri = $this->getIriFromResource($book);
$response = $this->get($iri, ['accept' => ['application/hal+json']]);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/hal+json; charset=utf-8');
$response->assertHeader('content-type', 'application/hal+json');
$this->assertJsonContains(
[
'name' => $book->name, // @phpstan-ignore-line
Expand Down
12 changes: 6 additions & 6 deletions src/Laravel/Tests/JsonApiTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function testGetEntrypoint(): void
{
$response = $this->get('/api/', ['accept' => ['application/vnd.api+json']]);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/vnd.api+json; charset=utf-8');
$response->assertHeader('content-type', 'application/vnd.api+json');
$this->assertJsonContains(
[
'links' => [
Expand All @@ -78,7 +78,7 @@ public function testGetCollection(): void
BookFactory::new()->has(AuthorFactory::new())->count(10)->create();
$response = $this->get('/api/books', ['accept' => ['application/vnd.api+json']]);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/vnd.api+json; charset=utf-8');
$response->assertHeader('content-type', 'application/vnd.api+json');
$response->assertJsonFragment([
'links' => [
'self' => '/api/books?page=1',
Expand All @@ -98,7 +98,7 @@ public function testGetBook(): void
$iri = $this->getIriFromResource($book);
$response = $this->get($iri, ['accept' => ['application/vnd.api+json']]);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/vnd.api+json; charset=utf-8');
$response->assertHeader('content-type', 'application/vnd.api+json');

$this->assertJsonContains([
'data' => [
Expand Down Expand Up @@ -141,7 +141,7 @@ public function testCreateBook(): void
);

$response->assertStatus(201);
$response->assertHeader('content-type', 'application/vnd.api+json; charset=utf-8');
$response->assertHeader('content-type', 'application/vnd.api+json');
$this->assertJsonContains([
'data' => [
'type' => 'Book',
Expand Down Expand Up @@ -243,7 +243,7 @@ public function testValidateJsonApi(): void
);

$response->assertStatus(422);
$response->assertHeader('content-type', 'application/vnd.api+json; charset=utf-8');
$response->assertHeader('content-type', 'application/vnd.api+json');
$json = $response->json();
$this->assertJsonContains([
'errors' => [
Expand Down Expand Up @@ -289,7 +289,7 @@ public function testNotFound(): void
{
$response = $this->get('/api/books/notfound', headers: ['accept' => 'application/vnd.api+json']);
$response->assertStatus(404);
$response->assertHeader('content-type', 'application/vnd.api+json; charset=utf-8');
$response->assertHeader('content-type', 'application/vnd.api+json');

$this->assertJsonContains([
'links' => ['type' => '/errors/404'],
Expand Down
28 changes: 14 additions & 14 deletions src/Laravel/Tests/JsonLdTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function testGetCollection(): void
BookFactory::new()->has(AuthorFactory::new())->count(10)->create();
$response = $this->get('/api/books', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$response->assertJsonFragment([
'@context' => '/api/contexts/Book',
'@id' => '/api/books',
Expand All @@ -66,7 +66,7 @@ public function testGetBook(): void
$book = Book::first();
$response = $this->get($this->getIriFromResource($book), ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$response->assertJsonFragment([
'@context' => '/api/contexts/Book',
'@id' => $this->getIriFromResource($book),
Expand Down Expand Up @@ -94,7 +94,7 @@ public function testCreateBook(): void
);

$response->assertStatus(201);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$response->assertJsonFragment([
'@context' => '/api/contexts/Book',
'@type' => 'Book',
Expand Down Expand Up @@ -184,7 +184,7 @@ public function testSkolemIris(): void
{
$response = $this->get('/api/outputs', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$response->assertJsonFragment([
'@type' => 'NotAResource',
'name' => 'test',
Expand All @@ -198,7 +198,7 @@ public function testSubresourceCollection(): void
PostFactory::new()->has(CommentFactory::new()->count(10))->count(10)->create();
$response = $this->get('/api/posts', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');

$response->assertJsonFragment([
'@context' => '/api/contexts/Post',
Expand Down Expand Up @@ -244,7 +244,7 @@ public function testCreateNotValid(): void
);

$response->assertStatus(422);
$response->assertHeader('content-type', 'application/problem+json; charset=utf-8');
$response->assertHeader('content-type', 'application/problem+json');
$response->assertJsonFragment([
'@context' => '/api/contexts/ValidationError',
'@type' => 'ValidationError',
Expand All @@ -269,7 +269,7 @@ public function testCreateNotValidPost(): void
);

$response->assertStatus(422);
$response->assertHeader('content-type', 'application/problem+json; charset=utf-8');
$response->assertHeader('content-type', 'application/problem+json');
$response->assertJsonFragment([
'@context' => '/api/contexts/ValidationError',
'@type' => 'ValidationError',
Expand All @@ -286,7 +286,7 @@ public function testSluggable(): void
SluggableFactory::new()->count(10)->create();
$response = $this->get('/api/sluggables', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$response->assertJsonFragment([
'@context' => '/api/contexts/Sluggable',
'@id' => '/api/sluggables',
Expand All @@ -308,23 +308,23 @@ public function testJsonLdContextHasCorrectContentType(): void
{
$response = $this->get('/api/contexts/Entrypoint', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$this->assertArrayHasKey('@context', $response->json());
}

public function testJsonLdResourceContextHasCorrectContentType(): void
{
$response = $this->get('/api/contexts/Book', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$this->assertArrayHasKey('@context', $response->json());
}

public function testJsonLdContextDefaultsToEntrypoint(): void
{
$response = $this->get('/api/contexts/', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$this->assertArrayHasKey('@context', $response->json());
}

Expand All @@ -333,7 +333,7 @@ public function testHidden(): void
PostFactory::new()->has(CommentFactory::new()->count(10))->count(10)->create();
$response = $this->get('/api/posts/1/comments/1', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$response->assertJsonMissingPath('internalNote');
}

Expand All @@ -342,7 +342,7 @@ public function testVisible(): void
BookFactory::new()->has(AuthorFactory::new())->count(10)->create();
$response = $this->get('/api/books', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$this->assertStringNotContainsString('internalNote', (string) $response->getContent());
}

Expand Down Expand Up @@ -386,7 +386,7 @@ public function testResourceWithOptionModel(): void
{
$response = $this->get('/api/resource_with_models?page=1', ['accept' => 'application/ld+json']);
$response->assertStatus(200);
$response->assertHeader('content-type', 'application/ld+json; charset=utf-8');
$response->assertHeader('content-type', 'application/ld+json');
$response->assertJsonFragment([
'@context' => '/api/contexts/ResourceWithModel',
'@id' => '/api/resource_with_models',
Expand Down
4 changes: 2 additions & 2 deletions src/Laravel/Tests/JsonProblemTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function testNotFound(): void
{
$response = $this->get('/api/books/notfound', headers: ['accept' => 'application/ld+json']);
$response->assertStatus(404);
$response->assertHeader('content-type', 'application/problem+json; charset=utf-8');
$response->assertHeader('content-type', 'application/problem+json');
$response->assertJsonFragment([
'@context' => '/api/contexts/Error',
'@id' => '/api/errors/404',
Expand Down Expand Up @@ -89,7 +89,7 @@ public function testProblemExceptionInterface(): void
{
$response = $this->get('/api/teapot', headers: ['accept' => 'application/json']);
$response->assertStatus(418);
$response->assertHeader('content-type', 'application/problem+json; charset=utf-8');
$response->assertHeader('content-type', 'application/problem+json');
$response->assertJsonFragment([
'type' => '/problem/teapot',
'title' => 'I\'m a teapot',
Expand Down
25 changes: 24 additions & 1 deletion src/State/Util/HttpResponseHeadersTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ private function getHeaders(Request $request, HttpOperation $operation, array $c
];

if ($hasBody) {
$headers['Content-Type'] = \sprintf('%s; charset=utf-8', $request->getMimeType($request->getRequestFormat()));
$headers['Content-Type'] = $this->formatContentType($request->getMimeType($request->getRequestFormat()));
}

$exception = $request->attributes->get('exception');
Expand Down Expand Up @@ -148,6 +148,29 @@ private function getHeaders(Request $request, HttpOperation $operation, array $c
return $headers;
}

/**
* Appends `; charset=utf-8` only for media types whose IANA registration defines a charset parameter.
*
* JSON-based media types (RFC 8259, RFC 6839 `+json` suffix) do not define charset and MUST always be UTF-8;
* sending the parameter can break strict clients.
*/
private function formatContentType(?string $mimeType): string
{
if (null === $mimeType || '' === $mimeType) {
return '';
}

if (str_starts_with($mimeType, 'text/')) {
return $mimeType.'; charset=utf-8';
}

if ('application/xml' === $mimeType) {
return $mimeType.'; charset=utf-8';
}

return $mimeType;
}

private function addLinkedDataPlatformHeaders(array &$headers, HttpOperation $operation): void
{
if (!$this->resourceMetadataCollectionFactory) {
Expand Down
2 changes: 1 addition & 1 deletion tests/Fixtures/TestBundle/Controller/CustomController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ class CustomController extends AbstractController
{
public function customAction(int $id): JsonResponse
{
return new JsonResponse(\sprintf('This is a custom action for %d.', $id), 200, ['Content-Type' => 'application/ld+json; charset=utf-8']);
return new JsonResponse(\sprintf('This is a custom action for %d.', $id), 200, ['Content-Type' => 'application/ld+json']);
}
}
8 changes: 4 additions & 4 deletions tests/Functional/AttributeResourceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function testGetAttributeResourcesCollection(): void
]);

$this->assertResponseStatusCodeSame(200);
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json; charset=utf-8');
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json');
$this->assertJsonEquals([
'@context' => '/contexts/AttributeResources',
'@id' => '/attribute_resources',
Expand Down Expand Up @@ -87,7 +87,7 @@ public function testAliasedResourceRedirectsAndShowsTarget(): void

$this->assertResponseStatusCodeSame(301);
$this->assertResponseHeaderSame('Location', '/attribute_resources/2');
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json; charset=utf-8');
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json');
$this->assertJsonEquals([
'@context' => '/contexts/AttributeResource',
'@id' => '/attribute_resources/2',
Expand All @@ -107,7 +107,7 @@ public function testPatchAliasedResource(): void

$this->assertResponseStatusCodeSame(301);
$this->assertResponseHeaderSame('Location', '/attribute_resources/2');
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json; charset=utf-8');
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json');
$this->assertJsonEquals([
'@context' => '/contexts/AttributeResource',
'@id' => '/attribute_resources/2',
Expand All @@ -123,7 +123,7 @@ public function testIncompleteUriVariableConfigurationProducesProblem(): void
$response = self::createClient()->request('GET', '/photos/1/resize/300/100');

$this->assertResponseStatusCodeSame(400);
$this->assertResponseHeaderSame('Content-Type', 'application/problem+json; charset=utf-8');
$this->assertResponseHeaderSame('Content-Type', 'application/problem+json');
$linkHeader = $response->getHeaders(false)['link'][0] ?? '';
$this->assertStringContainsString('<http://www.w3.org/ns/hydra/error>; rel="http://www.w3.org/ns/json-ld#error"', $linkHeader);
$this->assertJsonContains(['detail' => 'Unable to generate an IRI for the item of type "ApiPlatform\\Tests\\Fixtures\\TestBundle\\Entity\\IncompleteUriVariableConfigured"']);
Expand Down
2 changes: 1 addition & 1 deletion tests/Functional/Authorization/DenyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public function testAuthenticatedUserGetCollectionReturns200(): void
'headers' => ['Accept' => 'application/ld+json'],
]);
$this->assertResponseIsSuccessful();
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json; charset=utf-8');
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json');
}

public function testCustomDataProviderGeneratorReturns200(): void
Expand Down
2 changes: 1 addition & 1 deletion tests/Functional/Authorization/LegacyDenyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function testAuthenticatedUserGetCollectionReturns200(): void
'headers' => ['Accept' => 'application/ld+json'],
]);
$this->assertResponseIsSuccessful();
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json; charset=utf-8');
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json');
}

public function testStandardUserCannotCreate(): void
Expand Down
2 changes: 1 addition & 1 deletion tests/Functional/CircularReferenceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function testSelfReferencingCircularReference(): void
]);

$this->assertResponseStatusCodeSame(200);
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json; charset=utf-8');
$this->assertResponseHeaderSame('Content-Type', 'application/ld+json');
$this->assertJsonEquals([
'@context' => '/contexts/CircularReference',
'@id' => '/circular_references/1',
Expand Down
Loading
Loading