Skip to content

Commit

Permalink
[FrameworkBundle] removed the possibility to pass a non-scalar attrib…
Browse files Browse the repository at this point in the history
…utes when calling render() to make the call works with or without a reverse proxy (closes #2941)
  • Loading branch information
fabpot committed Jan 2, 2012
1 parent 1e2d1a3 commit 254e49b
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-2.1.md
Expand Up @@ -26,6 +26,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c

### FrameworkBundle

* [BC BREAK] removed the possibility to pass a non-scalar attributes when calling render() to make the call works with or without a reverse proxy
* added a router:match command
* added kernel.event_subscriber tag
* added a way to create relative symlinks when running assets:install command (--relative option)
Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/HttpKernel.php
Expand Up @@ -105,6 +105,14 @@ public function render($controller, array $options = array())
'comment' => '',
), $options);

// enforce all attribute values to be scalars to be sure that the same
// render() call will work with or without a reverse proxy
foreach ($options['attributes'] as $key => $value) {
if (!is_scalar($value)) {

This comment has been minimized.

Copy link
@stof

stof Jan 2, 2012

Member

shouldn't arrays of scalar values be allowed too ? they can be passed through an HTTP request

This comment has been minimized.

Copy link
@lsmith77

lsmith77 Jan 2, 2012

Contributor

err right .. when i commented on the related ticket I said "scalar" which was of course totally wrong. actually it can be an object even, but we should just make sure to only pass the public properties on:
http://de3.php.net/http_build_query

This comment has been minimized.

Copy link
@fabpot

fabpot Jan 2, 2012

Author Member

I don't think it makes sense to support objects anyway as it makes the conversion not very explicit. What do you think?

This comment has been minimized.

Copy link
@lsmith77

lsmith77 Jan 2, 2012

Contributor

well the convention would be that whatever you pass .. you end up with the equivalent of what http_build_query would do.
if it wouldn't be such a big overhead i would propose to always send everything through parse_str(http_build_query($params)) .. well actually we need to check if we can make a user land implementation that does the same but faster. maybe 2 C pace functions are faster than what we can do anyway, it would certainly be easier to ensure reliable behavior.

This comment has been minimized.

Copy link
@asm89

asm89 Jan 2, 2012

Contributor

I agree. If you want to pass objects you're probably better of with some helper function for the serialization/deserialization.

As for supporting arrays, that would require to recursively check the arrays if it contains any non-scalar/array values?

Edit: Maybe the solution proposed by @lsmith77 is the best. That way it is "ensured" that you get the same data at the other end.

This comment has been minimized.

Copy link
@fabpot

fabpot Jan 2, 2012

Author Member

see 789d5ad

throw new \InvalidArgumentException(sprintf('Unable to render the "%s" controller as the "%s" attribute is not a scalar.', $controller, $key));
}
}

if (!is_array($options['alt'])) {
$options['alt'] = array($options['alt']);
}
Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/HttpKernelTest.php
Expand Up @@ -219,4 +219,17 @@ public function testExceptionInSubRequestsDoesNotMangleOutputBuffers()

$this->assertEquals('Foo', ob_get_clean());
}

/**
* @expectedException \InvalidArgumentException
*/
public function testRenderOnlyAllowScalarAttributes()
{
$dispatcher = new EventDispatcher();
$resolver = $this->getMock('Symfony\\Component\\HttpKernel\\Controller\\ControllerResolverInterface');
$container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
$kernel = new HttpKernel($dispatcher, $container, $resolver);

$kernel->render('/', array('attributes' => array('foo' => new \stdClass())));
}
}

0 comments on commit 254e49b

Please sign in to comment.