Skip to content

Commit

Permalink
Merge branch '4.4' into 5.0
Browse files Browse the repository at this point in the history
* 4.4:
  [HttpFoundation] Do not set the default Content-Type based on the Accept header
  [Security] Fix access_control behavior with unanimous decision strategy
  • Loading branch information
nicolas-grekas committed Mar 30, 2020
2 parents 6b0ad43 + dca3434 commit a92ffff
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 16 deletions.
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\ErrorHandler\ErrorRenderer;

use Symfony\Component\ErrorHandler\Exception\FlattenException;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Serializer\Exception\NotEncodableValueException;
use Symfony\Component\Serializer\SerializerInterface;
Expand All @@ -30,6 +31,7 @@ class SerializerErrorRenderer implements ErrorRendererInterface

/**
* @param string|callable(FlattenException) $format The format as a string or a callable that should return it
* formats not supported by Request::getMimeTypes() should be given as mime types
* @param bool|callable $debug The debugging mode as a boolean or a callable that should return it
*/
public function __construct(SerializerInterface $serializer, $format, ErrorRendererInterface $fallbackErrorRenderer = null, $debug = false)
Expand Down Expand Up @@ -57,11 +59,16 @@ public function render(\Throwable $exception): FlattenException

try {
$format = \is_string($this->format) ? $this->format : ($this->format)($flattenException);
$headers = [
'Content-Type' => Request::getMimeTypes($format)[0] ?? $format,
'Vary' => 'Accept',
];

return $flattenException->setAsString($this->serializer->serialize($flattenException, $format, [
'exception' => $exception,
'debug' => \is_bool($this->debug) ? $this->debug : ($this->debug)($exception),
]));
]))
->setHeaders($flattenException->getHeaders() + $headers);
} catch (NotEncodableValueException $e) {
return $this->fallbackErrorRenderer->render($exception);
}
Expand Down
4 changes: 3 additions & 1 deletion src/Symfony/Component/HttpFoundation/Request.php
Expand Up @@ -1561,7 +1561,9 @@ public function isNoCache()
* Gets the preferred format for the response by inspecting, in the following order:
* * the request format set using setRequestFormat
* * the values of the Accept HTTP header
* * the content type of the body of the request.
*
* Note that if you use this method, you should send the "Vary: Accept" header
* in the response to prevent any issues with intermediary HTTP caches.
*/
public function getPreferredFormat(?string $default = 'html'): ?string
{
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpFoundation/Response.php
Expand Up @@ -266,7 +266,7 @@ public function prepare(Request $request)
} else {
// Content-type based on the Request
if (!$headers->has('Content-Type')) {
$format = $request->getPreferredFormat(null);
$format = $request->getRequestFormat(null);
if (null !== $format && $mimeType = $request->getMimeType($format)) {
$headers->set('Content-Type', $mimeType);
}
Expand Down
15 changes: 14 additions & 1 deletion src/Symfony/Component/HttpFoundation/Tests/ResponseTest.php
Expand Up @@ -497,12 +497,25 @@ public function testPrepareDoesNothingIfRequestFormatIsNotDefined()
$this->assertEquals('text/html; charset=UTF-8', $response->headers->get('content-type'));
}

/**
* Same URL cannot produce different Content-Type based on the value of the Accept header,
* unless explicitly stated in the response object.
*/
public function testPrepareDoesNotSetContentTypeBasedOnRequestAcceptHeader()
{
$response = new Response('foo');
$request = Request::create('/');
$request->headers->set('Accept', 'application/json');
$response->prepare($request);

$this->assertSame('text/html; charset=UTF-8', $response->headers->get('content-type'));
}

public function testPrepareSetContentType()
{
$response = new Response('foo');
$request = Request::create('/');
$request->setRequestFormat('json');
$request->headers->remove('accept');

$response->prepare($request);

Expand Down
Expand Up @@ -54,11 +54,16 @@ public function __construct(iterable $voters = [], string $strategy = self::STRA
}

/**
* @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array
*
* {@inheritdoc}
*/
public function decide(TokenInterface $token, array $attributes, $object = null)
public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/)
{
if (\count($attributes) > 1) {
$allowMultipleAttributes = 3 < func_num_args() && func_get_arg(3);

// Special case for AccessListener, do not remove the right side of the condition before 6.0
if (\count($attributes) > 1 && !$allowMultipleAttributes) {
throw new InvalidArgumentException(sprintf('Passing more than one Security attribute to "%s()" is not supported.', __METHOD__));
}

Expand Down
10 changes: 1 addition & 9 deletions src/Symfony/Component/Security/Http/Firewall/AccessListener.php
Expand Up @@ -85,15 +85,7 @@ public function authenticate(RequestEvent $event)
$this->tokenStorage->setToken($token);
}

$granted = false;
foreach ($attributes as $key => $value) {
if ($this->accessDecisionManager->decide($token, [$key => $value], $request)) {
$granted = true;
break;
}
}

if (!$granted) {
if (!$this->accessDecisionManager->decide($token, $attributes, $request, true)) {
$exception = new AccessDeniedException();
$exception->setAttributes($attributes);
$exception->setSubject($request);
Expand Down
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
use Symfony\Component\Security\Http\AccessMapInterface;
Expand Down Expand Up @@ -227,4 +228,44 @@ public function testHandleWhenTheSecurityTokenStorageHasNoToken()

$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
}

public function testHandleMWithultipleAttributesShouldBeHandledAsAnd()
{
$request = new Request();

$accessMap = $this->getMockBuilder('Symfony\Component\Security\Http\AccessMapInterface')->getMock();
$accessMap
->expects($this->any())
->method('getPatterns')
->with($this->equalTo($request))
->willReturn([['foo' => 'bar', 'bar' => 'baz'], null])
;

$authenticatedToken = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
$authenticatedToken
->expects($this->any())
->method('isAuthenticated')
->willReturn(true)
;

$tokenStorage = new TokenStorage();
$tokenStorage->setToken($authenticatedToken);

$accessDecisionManager = $this->getMockBuilder('Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface')->getMock();
$accessDecisionManager
->expects($this->once())
->method('decide')
->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request), true)
->willReturn(true)
;

$listener = new AccessListener(
$tokenStorage,
$accessDecisionManager,
$accessMap,
$this->createMock('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')
);

$listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MASTER_REQUEST));
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/Security/Http/composer.json
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": "^7.2.5",
"symfony/security-core": "^4.4|^5.0",
"symfony/security-core": "^4.4.7|^5.0.7",
"symfony/http-foundation": "^4.4.7|^5.0.7",
"symfony/http-kernel": "^4.4|^5.0",
"symfony/property-access": "^4.4|^5.0"
Expand Down

0 comments on commit a92ffff

Please sign in to comment.