Skip to content

Commit

Permalink
fixed request format when forwarding a request
Browse files Browse the repository at this point in the history
  • Loading branch information
fabpot committed Aug 22, 2013
1 parent 1ad64ee commit 7e87eb1
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\FrameworkBundle\Tests\Controller;

use Symfony\Bundle\FrameworkBundle\Tests\TestCase;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;

class ControllerTest extends TestCase
{
public function testForward()
{
$request = Request::create('/');
$request->setLocale('fr');
$request->setRequestFormat('xml');

$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
$kernel->expects($this->once())->method('handle')->will($this->returnCallback(function (Request $request) {
return new Response($request->getRequestFormat().'--'.$request->getLocale());
}));

$container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
$container->expects($this->at(0))->method('get')->will($this->returnValue($request));
$container->expects($this->at(1))->method('get')->will($this->returnValue($kernel));

$controller = new Controller();
$controller->setContainer($container);

$response = $controller->forward('a_controller');
$this->assertEquals('xml--fr', $response->getContent());
}
}
4 changes: 4 additions & 0 deletions src/Symfony/Component/HttpFoundation/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,10 @@ public function duplicate(array $query = null, array $request = null, array $att
$dup->method = null;
$dup->format = null;

if (!$dup->get('_format')) {
$dup->setRequestFormat($this->getRequestFormat());
}

return $dup;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public function onKernelException(GetResponseForExceptionEvent $event)
'_controller' => $this->controller,
'exception' => FlattenException::create($exception),
'logger' => $this->logger instanceof DebugLoggerInterface ? $this->logger : null,
'_format' => $request->getRequestFormat(),
// keep for BC -- as $format can be an argument of the controller callable
// see src/Symfony/Bundle/TwigBundle/Controller/ExceptionController.php
// @deprecated in 2.4, to be removed in 3.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,25 @@ public function provider()
array($event, $event2)
);
}

public function testSubRequestFormat()
{
$listener = new ExceptionListener('foo', $this->getMock('Psr\Log\LoggerInterface'));

$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
$kernel->expects($this->once())->method('handle')->will($this->returnCallback(function (Request $request) {
return new Response($request->getRequestFormat());
}));

$request = Request::create('/');
$request->setRequestFormat('xml');

$event = new GetResponseForExceptionEvent($kernel, $request, 'foo', new \Exception('foo'));
$listener->onKernelException($event);

$response = $event->getResponse();
$this->assertEquals('xml', $response->getContent());
}
}

class TestLogger extends Logger implements DebugLoggerInterface
Expand Down

5 comments on commit 7e87eb1

@Taluu
Copy link
Contributor

@Taluu Taluu commented on 7e87eb1 Sep 2, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I'm kinda late to comment this commit, but I think that this is a regression. When you forward a request, you may need to interpolate the format on your own ; here, you set it by default to "html" if no format was given (when it could be handled in the forwarded controller or whatever). When doing a getRequestFormat on the forwarded part, you then get the default format.

I know this is easily manageable (through in the forwarded controller setting this value to null again), but I still think this is a regression.

@fabpot
Copy link
Member Author

@fabpot fabpot commented on 7e87eb1 Sep 2, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Taluu Can you open a new issue?

@hhamon
Copy link
Contributor

@hhamon hhamon commented on 7e87eb1 Sep 10, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm this is a regression.

It should be _format as the controller variable is also called $_format. If the _format variable is set in my URL with a json value, then I'm getting an html stack trace instead of a json stack trace.

@rsahlstrom
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reported #9083 that details how the removal of _format in ExceptionListener effects the ExceptionController in the TwigBundle.

@Tobion
Copy link
Member

@Tobion Tobion commented on 7e87eb1 Oct 28, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the same problem as @hhamon in the current master branch!

Please sign in to comment.