Skip to content

Commit ba8a7e6

Browse files
soyukadunglas
soyuka
authored and
dunglas
committed
fix: exception message leak
1 parent b1b4f14 commit ba8a7e6

File tree

9 files changed

+104
-27
lines changed

9 files changed

+104
-27
lines changed

‎src/ApiResource/Error.php

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,14 @@
5858
class Error extends \Exception implements ProblemExceptionInterface, HttpExceptionInterface
5959
{
6060
public function __construct(
61-
private readonly string $title,
62-
private readonly string $detail,
61+
private string $title,
62+
private string $detail,
6363
#[ApiProperty(identifier: true)] private int $status,
6464
array $originalTrace = null,
6565
private ?string $instance = null,
6666
private string $type = 'about:blank',
67-
private array $headers = []
67+
private array $headers = [],
68+
private ?\Exception $previous = null
6869
) {
6970
parent::__construct();
7071

@@ -85,21 +86,21 @@ public function __construct(
8586

8687
#[SerializedName('hydra:title')]
8788
#[Groups(['jsonld', 'legacy_jsonld'])]
88-
public function getHydraTitle(): string
89+
public function getHydraTitle(): ?string
8990
{
9091
return $this->title;
9192
}
9293

9394
#[SerializedName('hydra:description')]
9495
#[Groups(['jsonld', 'legacy_jsonld'])]
95-
public function getHydraDescription(): string
96+
public function getHydraDescription(): ?string
9697
{
9798
return $this->detail;
9899
}
99100

100101
#[SerializedName('description')]
101102
#[Groups(['jsonapi', 'legacy_jsonapi'])]
102-
public function getDescription(): string
103+
public function getDescription(): ?string
103104
{
104105
return $this->detail;
105106
}
@@ -108,7 +109,7 @@ public static function createFromException(\Exception|\Throwable $exception, int
108109
{
109110
$headers = ($exception instanceof SymfonyHttpExceptionInterface || $exception instanceof HttpExceptionInterface) ? $exception->getHeaders() : [];
110111

111-
return new self('An error occurred', $exception->getMessage(), $status, $exception->getTrace(), type: '/errors/'.$status, headers: $headers);
112+
return new self('An error occurred', $exception->getMessage(), $status, $exception->getTrace(), type: '/errors/'.$status, headers: $headers, previous: $exception->getPrevious());
112113
}
113114

114115
#[Ignore]
@@ -123,6 +124,9 @@ public function getStatusCode(): int
123124
return $this->status;
124125
}
125126

127+
/**
128+
* @param array<string, string> $headers
129+
*/
126130
public function setHeaders(array $headers): void
127131
{
128132
$this->headers = $headers;
@@ -134,15 +138,20 @@ public function getType(): string
134138
return $this->type;
135139
}
136140

141+
public function setType(string $type): void
142+
{
143+
$this->type = $type;
144+
}
145+
137146
#[Groups(['jsonld', 'legacy_jsonproblem', 'jsonproblem', 'jsonapi', 'legacy_jsonapi'])]
138147
public function getTitle(): ?string
139148
{
140149
return $this->title;
141150
}
142151

143-
public function setType(string $type): void
152+
public function setTitle(string $title = null): void
144153
{
145-
$this->type = $type;
154+
$this->title = $title;
146155
}
147156

148157
#[Groups(['jsonld', 'jsonproblem', 'legacy_jsonproblem'])]
@@ -162,9 +171,19 @@ public function getDetail(): ?string
162171
return $this->detail;
163172
}
164173

174+
public function setDetail(string $detail = null): void
175+
{
176+
$this->detail = $detail;
177+
}
178+
165179
#[Groups(['jsonld', 'jsonproblem', 'legacy_jsonproblem'])]
166180
public function getInstance(): ?string
167181
{
168182
return $this->instance;
169183
}
184+
185+
public function setInstance(string $instance = null): void
186+
{
187+
$this->instance = $instance;
188+
}
170189
}

‎src/Hydra/Serializer/ErrorNormalizer.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
/**
2525
* Converts {@see \Exception} or {@see FlattenException} to a Hydra error representation.
2626
*
27-
* @deprecated we use ItemNormalizer instead
27+
* @deprecated we will use the ItemNormalizer in 4.x instead
2828
*
2929
* @author Kévin Dunglas <dunglas@gmail.com>
3030
* @author Samuel ROZE <samuel.roze@gmail.com>
@@ -47,8 +47,6 @@ public function __construct(private readonly UrlGeneratorInterface|LegacyUrlGene
4747
*/
4848
public function normalize(mixed $object, string $format = null, array $context = []): array|string|int|float|bool|\ArrayObject|null
4949
{
50-
trigger_deprecation('api-platform', '3.2', sprintf('The class "%s" is deprecated in favor of using an Error resource. We fallback on "api_platform.serializer.normalizer.item".', __CLASS__));
51-
5250
if ($this->itemNormalizer) {
5351
return $this->itemNormalizer->normalize($object, $format, $context);
5452
}

‎src/JsonApi/Serializer/ErrorNormalizer.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
/**
2323
* Converts {@see \Exception} or {@see FlattenException} or to a JSON API error representation.
2424
*
25-
* @deprecated we use ItemNormalizer instead
25+
* @deprecated we will use the ItemNormalizer in 4.x instead
2626
*
2727
* @author Héctor Hurtarte <hectorh30@gmail.com>
2828
*/
@@ -46,8 +46,6 @@ public function __construct(private readonly bool $debug = false, array $default
4646
*/
4747
public function normalize(mixed $object, string $format = null, array $context = []): array
4848
{
49-
trigger_deprecation('api-platform', '3.2', sprintf('The class "%s" is deprecated in favor of using an Error resource. We fallback on "api_platform.serializer.normalizer.item".', __CLASS__));
50-
5149
if ($this->itemNormalizer) {
5250
return $this->itemNormalizer->normalize($object, $format, $context);
5351
}

‎src/Problem/Serializer/ErrorNormalizer.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
* Normalizes errors according to the API Problem spec (RFC 7807).
2323
*
2424
* @see https://tools.ietf.org/html/rfc7807
25-
* @deprecated we use ItemNormalizer instead
25+
* @deprecated we will use the ItemNormalizer in 4.x instead
2626
*
2727
* @author Kévin Dunglas <dunglas@gmail.com>
2828
*/
@@ -47,8 +47,6 @@ public function __construct(private readonly bool $debug = false, array $default
4747
*/
4848
public function normalize(mixed $object, string $format = null, array $context = []): array
4949
{
50-
trigger_deprecation('api-platform', '3.2', sprintf('The class "%s" is deprecated in favor of using an Error resource. We fallback on "api_platform.serializer.normalizer.item".', __CLASS__));
51-
5250
if ($this->itemNormalizer) {
5351
return $this->itemNormalizer->normalize($object, $format, $context);
5452
}

‎src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ private function registerCommonConfiguration(ContainerBuilder $container, array
251251
}
252252
$container->setParameter('api_platform.asset_package', $config['asset_package']);
253253
$container->setParameter('api_platform.defaults', $this->normalizeDefaults($config['defaults'] ?? []));
254+
$container->setParameter('api_platform.rfc_7807_compliant_errors', $config['defaults']['extra_properties']['rfc_7807_compliant_errors'] ?? false);
254255

255256
if ($container->getParameter('kernel.debug')) {
256257
$container->removeDefinition('api_platform.serializer.mapping.cache_class_metadata_factory');

‎src/Symfony/Bundle/Resources/config/api.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@
198198
<argument key="$exceptionToStatus">%api_platform.exception_to_status%</argument>
199199
<argument key="$identifiersExtractor" type="service" id="api_platform.api.identifiers_extractor"/>
200200
<argument key="$resourceClassResolver" type="service" id="api_platform.resource_class_resolver"/>
201+
<argument key="$problemCompliantErrors">%api_platform.rfc_7807_compliant_errors%</argument>
201202
</service>
202203
</services>
203204
</container>

‎src/Symfony/EventListener/ErrorListener.php

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ public function __construct(
5757
private readonly array $exceptionToStatus = [],
5858
private readonly null|IdentifiersExtractorInterface|LegacyIdentifiersExtractorInterface $identifiersExtractor = null,
5959
private readonly null|ResourceClassResolverInterface|LegacyResourceClassResolverInterface $resourceClassResolver = null,
60-
Negotiator $negotiator = null
60+
Negotiator $negotiator = null,
61+
private readonly bool $problemCompliantErrors = true
6162
) {
6263
parent::__construct($controller, $logger, $debug, $exceptionsMapping);
6364
$this->resourceMetadataCollectionFactory = $resourceMetadataCollectionFactory;
@@ -66,16 +67,31 @@ public function __construct(
6667

6768
protected function duplicateRequest(\Throwable $exception, Request $request): Request
6869
{
69-
$dup = parent::duplicateRequest($exception, $request);
70-
7170
$apiOperation = $this->initializeOperation($request);
71+
if (false === $this->problemCompliantErrors) {
72+
// TODO: deprecate in API Platform 3.3
73+
$this->controller = 'api_platform.action.exception';
74+
$dup = parent::duplicateRequest($exception, $request);
75+
$dup->attributes->set('_api_operation', $apiOperation);
76+
77+
return $dup;
78+
}
7279

7380
if ($this->debug) {
7481
$this->logger?->error('An exception occured, transforming to an Error resource.', ['exception' => $exception, 'operation' => $apiOperation]);
7582
}
7683

7784
$format = $this->getRequestFormat($request, $this->errorFormats, false);
7885

86+
// Let the error handler take this we don't handle HTML
87+
if ('html' === $format) {
88+
$this->controller = 'error_controller';
89+
$dup = parent::duplicateRequest($exception, $request);
90+
91+
return $dup;
92+
}
93+
94+
$dup = parent::duplicateRequest($exception, $request);
7995
if ($this->resourceMetadataCollectionFactory) {
8096
if ($this->resourceClassResolver?->isResourceClass($exception::class)) {
8197
$resourceCollection = $this->resourceMetadataCollectionFactory->create($exception::class);
@@ -126,9 +142,8 @@ protected function duplicateRequest(\Throwable $exception, Request $request): Re
126142
$operation = $operation->withProvider([self::class, 'provide']);
127143
}
128144

129-
// For our swagger Ui errors
130-
if ('html' === $format) {
131-
$operation = $operation->withOutputFormats(['html' => ['text/html']]);
145+
if (!$this->debug && $operation->getStatus() >= 500) {
146+
$errorResource->setDetail('Internal Server Error');
132147
}
133148

134149
$identifiers = [];
@@ -144,7 +159,7 @@ protected function duplicateRequest(\Throwable $exception, Request $request): Re
144159
]);
145160
}
146161

147-
if ('jsonld' === $format && !($apiOperation?->getExtraProperties()['rfc_7807_compliant_errors'] ?? false)) {
162+
if ($apiOperation && 'jsonld' === $format && !($apiOperation?->getExtraProperties()['rfc_7807_compliant_errors'] ?? false)) {
148163
$operation = $operation->withOutputFormats(['jsonld' => ['application/ld+json']])
149164
->withLinks([])
150165
->withExtraProperties(['rfc_7807_compliant_errors' => false] + $operation->getExtraProperties());
@@ -154,7 +169,7 @@ protected function duplicateRequest(\Throwable $exception, Request $request): Re
154169
$dup->attributes->set('_api_previous_operation', $apiOperation);
155170
$dup->attributes->set('_api_operation', $operation);
156171
$dup->attributes->set('_api_operation_name', $operation->getName());
157-
$dup->attributes->set('exception', $errorResource);
172+
$dup->attributes->set('exception', $exception);
158173
// These are for swagger
159174
$dup->attributes->set('_api_original_route', $request->attributes->get('_route'));
160175
$dup->attributes->set('_api_original_route_params', $request->attributes->get('_route_params'));

‎tests/Symfony/Bundle/Test/ApiTestCaseTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ private function recreateSchema(array $options = []): void
290290
*/
291291
public function testExceptionNormalizer(): void
292292
{
293-
$this->expectDeprecation('Since api-platform 3.2: The class "ApiPlatform\Problem\Serializer\ErrorNormalizer" is deprecated in favor of using an Error resource. We fallback on "api_platform.serializer.normalizer.item".');
293+
// $this->expectDeprecation('Since api-platform 3.2: The class "ApiPlatform\Problem\Serializer\ErrorNormalizer" is deprecated in favor of using an Error resource. We fallback on "api_platform.serializer.normalizer.item".');
294294

295295
$response = self::createClient()->request('GET', '/issue5921', [
296296
'headers' => [

‎tests/Symfony/EventListener/ErrorListenerTest.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,4 +108,51 @@ public function testDuplicateExceptionWithErrorResource(): void
108108
$errorListener = new ErrorListener('action', null, true, [], $resourceMetadataCollectionFactory->reveal(), ['jsonld' => ['application/ld+json']], [], $identifiersExtractor->reveal(), $resourceClassResolver->reveal());
109109
$errorListener->onKernelException($exceptionEvent);
110110
}
111+
112+
public function testDisableErrorResourceHandling(): void
113+
{
114+
$exception = Error::createFromException(new \Exception(), 400);
115+
$operation = new Get(name: '_api_errors_hydra', priority: 0, status: 400, outputFormats: ['jsonld' => ['application/ld+json']]);
116+
$resourceMetadataCollectionFactory = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class);
117+
$resourceClassResolver = $this->prophesize(ResourceClassResolverInterface::class);
118+
$kernel = $this->prophesize(KernelInterface::class);
119+
$kernel->handle(Argument::that(function ($request) {
120+
$this->assertEquals($request->attributes->get('_api_operation'), null);
121+
122+
return true;
123+
}), HttpKernelInterface::SUB_REQUEST, false)->willReturn(new Response());
124+
$exceptionEvent = new ExceptionEvent($kernel->reveal(), Request::create('/'), HttpKernelInterface::SUB_REQUEST, $exception);
125+
$identifiersExtractor = $this->prophesize(IdentifiersExtractorInterface::class);
126+
$identifiersExtractor->getIdentifiersFromItem($exception, Argument::any())->willReturn(['id' => 1]);
127+
$errorListener = new ErrorListener('action', null, true, [], $resourceMetadataCollectionFactory->reveal(), ['jsonld' => ['application/ld+json']], [], $identifiersExtractor->reveal(), $resourceClassResolver->reveal(), null, false);
128+
$errorListener->onKernelException($exceptionEvent);
129+
}
130+
131+
public function testDuplicateExceptionWithErrorResourceProduction(): void
132+
{
133+
$exception = new \Exception();
134+
$operation = new Get(name: '_api_errors_hydra', priority: 0, outputFormats: ['jsonld' => ['application/ld+json']]);
135+
$resourceMetadataCollectionFactory = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class);
136+
$resourceMetadataCollectionFactory->create(Error::class)
137+
->willReturn(new ResourceMetadataCollection(Error::class, [new ApiResource(operations: [$operation])]));
138+
$resourceClassResolver = $this->prophesize(ResourceClassResolverInterface::class);
139+
$resourceClassResolver->isResourceClass(\Exception::class)->willReturn(false);
140+
$kernel = $this->prophesize(KernelInterface::class);
141+
$kernel->handle(Argument::that(function ($request) {
142+
$this->assertTrue($request->attributes->has('_api_original_route'));
143+
$this->assertTrue($request->attributes->has('_api_original_route_params'));
144+
$this->assertTrue($request->attributes->has('_api_requested_operation'));
145+
$this->assertTrue($request->attributes->has('_api_previous_operation'));
146+
$this->assertEquals('_api_errors_hydra', $request->attributes->get('_api_operation_name'));
147+
$operation = $request->attributes->get('_api_operation');
148+
$this->assertEquals(\call_user_func($operation->getProvider())->getDetail(), 'Internal Server Error');
149+
150+
return true;
151+
}), HttpKernelInterface::SUB_REQUEST, false)->willReturn(new Response());
152+
$exceptionEvent = new ExceptionEvent($kernel->reveal(), Request::create('/'), HttpKernelInterface::SUB_REQUEST, $exception);
153+
$identifiersExtractor = $this->prophesize(IdentifiersExtractorInterface::class);
154+
$identifiersExtractor->getIdentifiersFromItem(Argument::cetera())->willThrow(new \Exception());
155+
$errorListener = new ErrorListener('action', null, false, [], $resourceMetadataCollectionFactory->reveal(), ['jsonld' => ['application/ld+json']], [], $identifiersExtractor->reveal(), $resourceClassResolver->reveal());
156+
$errorListener->onKernelException($exceptionEvent);
157+
}
111158
}

0 commit comments

Comments
 (0)