Skip to content

Commit 10449cb

Browse files
fabpotfabpot
committed
feature #32344 [HttpFoundation][HttpKernel] Improving the request/response format autodetection (yceruto)
This PR was merged into the 4.4 branch. Discussion ---------- [HttpFoundation][HttpKernel] Improving the request/response format autodetection | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Mainly for API-based apps, currently the response header `Content-Type` (if no provided) is guessed based on the request format (`_format` attribute), falling back to `html` by default. Especially for the new error renderer system, where any kind of error can occur and it becomes an http response, this PR improves this guesser mechanism by taking into account also the `Content-type` of the request. Example: ```bash $ curl -X POST -H 'Content-Type: application/json' -i 'https://127.0.0.1:8000/login' ``` **before:** ```bash HTTP/2 500 cache-control: no-cache, private content-type: text/html; charset=UTF-8 # <- inaccurate ... {"title":"Internal Server Error","status":500,"detail":"Invalid credentials!"} ``` Most of the 3rd-party bundles that I know (`api-platform/core`, `FOSRestBundle`) need a dedicated listener to achieve it right. **after:** ```bash HTTP/2 500 cache-control: no-cache, private content-type: application/json ... {"title":"Internal Server Error","status":500,"detail":"Invalid credentials!"} ``` Of course, this applies to all kind of responses, as long as the `Content-Type` is not explicitly provided. So, as a last chance, the `Accept` heading of the request is also taken into account to detect the preferred format: ```bash $ curl -H 'Accept: application/json' -i 'https://127.0.0.1:8000/userinfo' HTTP/2 404 cache-control: no-cache, private content-type: application/json ... {"title":"Not Found","status":404,"detail":"No route found for \"GET \/userinfo\""} ``` They could be other places in the code where this new method could also be useful, please advise :) WDYT? Commits ------- 1952928 Improving the request/response format autodetection
2 parents d65a5e2 + 1952928 commit 10449cb

File tree

4 files changed

+51
-2
lines changed

4 files changed

+51
-2
lines changed

‎src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,10 @@ class Request
192192

193193
protected static $requestFactory;
194194

195+
/**
196+
* @var string|null
197+
*/
198+
private $preferredFormat;
195199
private $isHostValid = true;
196200
private $isForwardedValid = true;
197201

@@ -1559,6 +1563,25 @@ public function isNoCache()
15591563
return $this->headers->hasCacheControlDirective('no-cache') || 'no-cache' == $this->headers->get('Pragma');
15601564
}
15611565

1566+
public function getPreferredFormat(?string $default = 'html'): ?string
1567+
{
1568+
if (null !== $this->preferredFormat) {
1569+
return $this->preferredFormat;
1570+
}
1571+
1572+
$this->preferredFormat = $this->getRequestFormat($this->getContentType());
1573+
1574+
if (null === $this->preferredFormat) {
1575+
foreach ($this->getAcceptableContentTypes() as $contentType) {
1576+
if (null !== $this->preferredFormat = $this->getFormat($contentType)) {
1577+
break;
1578+
}
1579+
}
1580+
}
1581+
1582+
return $this->preferredFormat ?: $default;
1583+
}
1584+
15621585
/**
15631586
* Returns the preferred language.
15641587
*

‎src/Symfony/Component/HttpFoundation/Response.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ public function prepare(Request $request)
270270
} else {
271271
// Content-type based on the Request
272272
if (!$headers->has('Content-Type')) {
273-
$format = $request->getRequestFormat();
273+
$format = $request->getPreferredFormat();
274274
if (null !== $format && $mimeType = $request->getMimeType($format)) {
275275
$headers->set('Content-Type', $mimeType);
276276
}

‎src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,32 @@ public function testDuplicateWithFormat()
399399
$this->assertEquals('xml', $dup->getRequestFormat());
400400
}
401401

402+
public function testGetPreferredFormat()
403+
{
404+
$request = new Request();
405+
$this->assertNull($request->getPreferredFormat(null));
406+
$this->assertSame('html', $request->getPreferredFormat());
407+
$this->assertSame('json', $request->getPreferredFormat('json'));
408+
409+
$request->setRequestFormat('atom');
410+
$request->headers->set('Content-Type', 'application/json');
411+
$request->headers->set('Accept', 'application/xml');
412+
$this->assertSame('atom', $request->getPreferredFormat());
413+
414+
$request = new Request();
415+
$request->headers->set('Content-Type', 'application/json');
416+
$request->headers->set('Accept', 'application/xml');
417+
$this->assertSame('json', $request->getPreferredFormat());
418+
419+
$request = new Request();
420+
$request->headers->set('Accept', 'application/xml');
421+
$this->assertSame('xml', $request->getPreferredFormat());
422+
423+
$request = new Request();
424+
$request->headers->set('Accept', 'application/json;q=0.8,application/xml;q=0.9');
425+
$this->assertSame('xml', $request->getPreferredFormat());
426+
}
427+
402428
/**
403429
* @dataProvider getFormatToMimeTypeMapProviderWithAdditionalNullFormat
404430
*/

‎src/Symfony/Component/HttpKernel/EventListener/DebugHandlersListener.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public function onKernelException(GetResponseForExceptionEvent $event)
174174
$e = $request->attributes->get('exception');
175175

176176
try {
177-
return new Response($this->errorFormatter->render($e, $request->getRequestFormat()), $e->getStatusCode(), $e->getHeaders());
177+
return new Response($this->errorFormatter->render($e, $request->getPreferredFormat()), $e->getStatusCode(), $e->getHeaders());
178178
} catch (ErrorRendererNotFoundException $_) {
179179
return new Response($this->errorFormatter->render($e), $e->getStatusCode(), $e->getHeaders());
180180
}

0 commit comments

Comments
 (0)