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
-------

1952928471 Improving the request/response format autodetection
This commit is contained in:
Fabien Potencier 2019-07-03 19:09:44 +02:00
commit 10449cb493
4 changed files with 51 additions and 2 deletions

View File

@ -192,6 +192,10 @@ class Request
protected static $requestFactory;
/**
* @var string|null
*/
private $preferredFormat;
private $isHostValid = true;
private $isForwardedValid = true;
@ -1559,6 +1563,25 @@ class Request
return $this->headers->hasCacheControlDirective('no-cache') || 'no-cache' == $this->headers->get('Pragma');
}
public function getPreferredFormat(?string $default = 'html'): ?string
{
if (null !== $this->preferredFormat) {
return $this->preferredFormat;
}
$this->preferredFormat = $this->getRequestFormat($this->getContentType());
if (null === $this->preferredFormat) {
foreach ($this->getAcceptableContentTypes() as $contentType) {
if (null !== $this->preferredFormat = $this->getFormat($contentType)) {
break;
}
}
}
return $this->preferredFormat ?: $default;
}
/**
* Returns the preferred language.
*

View File

@ -270,7 +270,7 @@ class Response
} else {
// Content-type based on the Request
if (!$headers->has('Content-Type')) {
$format = $request->getRequestFormat();
$format = $request->getPreferredFormat();
if (null !== $format && $mimeType = $request->getMimeType($format)) {
$headers->set('Content-Type', $mimeType);
}

View File

@ -399,6 +399,32 @@ class RequestTest extends TestCase
$this->assertEquals('xml', $dup->getRequestFormat());
}
public function testGetPreferredFormat()
{
$request = new Request();
$this->assertNull($request->getPreferredFormat(null));
$this->assertSame('html', $request->getPreferredFormat());
$this->assertSame('json', $request->getPreferredFormat('json'));
$request->setRequestFormat('atom');
$request->headers->set('Content-Type', 'application/json');
$request->headers->set('Accept', 'application/xml');
$this->assertSame('atom', $request->getPreferredFormat());
$request = new Request();
$request->headers->set('Content-Type', 'application/json');
$request->headers->set('Accept', 'application/xml');
$this->assertSame('json', $request->getPreferredFormat());
$request = new Request();
$request->headers->set('Accept', 'application/xml');
$this->assertSame('xml', $request->getPreferredFormat());
$request = new Request();
$request->headers->set('Accept', 'application/json;q=0.8,application/xml;q=0.9');
$this->assertSame('xml', $request->getPreferredFormat());
}
/**
* @dataProvider getFormatToMimeTypeMapProviderWithAdditionalNullFormat
*/

View File

@ -174,7 +174,7 @@ class DebugHandlersListener implements EventSubscriberInterface
$e = $request->attributes->get('exception');
try {
return new Response($this->errorFormatter->render($e, $request->getRequestFormat()), $e->getStatusCode(), $e->getHeaders());
return new Response($this->errorFormatter->render($e, $request->getPreferredFormat()), $e->getStatusCode(), $e->getHeaders());
} catch (ErrorRendererNotFoundException $_) {
return new Response($this->errorFormatter->render($e), $e->getStatusCode(), $e->getHeaders());
}