Merge pull request #642 from willdurand/jsonp-validator

Use JsonpCallbackValidator lib

Author: lsmith77

DependencyInjection/Configuration.php

@@ -82,7 +82,7 @@ public function getConfigTreeBuilder()
                 ->end()
                 ->arrayNode('serializer')
                     ->validate()
-                        ->ifTrue(function($v) { return !empty($v['version']) && !empty($v['groups']); })
+                        ->ifTrue(function ($v) { return !empty($v['version']) && !empty($v['groups']); })
                         ->thenInvalid('Only either a version or a groups exclusion strategy can be set')
                     ->end()
                     ->addDefaultsIfNotSet()
@@ -197,7 +197,7 @@ private function addFormatListenerSection(ArrayNodeDefinition $rootNode)
                                     ->booleanNode('prefer_extension')->defaultTrue()->end()
                                     ->scalarNode('fallback_format')->defaultValue('html')->end()
                                     ->arrayNode('priorities')
-                                        ->beforeNormalization()->ifString()->then(function($v) { return preg_split('/\s*,\s*/', $v); })->end()
+                                        ->beforeNormalization()->ifString()->then(function ($v) { return preg_split('/\s*,\s*/', $v); })->end()
                                         ->prototype('scalar')->end()
                                     ->end()
                                 ->end()

DependencyInjection/FOSRestExtension.php

@@ -16,9 +16,7 @@
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
 use Symfony\Component\DependencyInjection\DefinitionDecorator;
-use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
-use Symfony\Component\HttpKernel\Kernel;
 
 use FOS\RestBundle\Util\Codes;
 
@@ -165,7 +163,10 @@ public function load(array $configs, ContainerBuilder $container)
             $container->setDefinition($this->getAlias().'.view_handler', $handler);
 
             $container->setParameter($this->getAlias().'.view_handler.jsonp.callback_param', $config['view']['jsonp_handler']['callback_param']);
-            $container->setParameter($this->getAlias().'.view_handler.jsonp.callback_filter', $config['view']['jsonp_handler']['callback_filter']);
+
+            if ('/(^[a-z0-9_]+$)|(^YUI\.Env\.JSONP\._[0-9]+$)/i' !== $config['view']['jsonp_handler']['callback_filter']) {
+                throw new \LogicException('As of 1.2.0, the "callback_filter" parameter is deprecated, and is not used anymore. For more information, read: https://github.com/FriendsOfSymfony/FOSRestBundle/pull/642.');
+            }
 
             if (empty($config['view']['mime_types']['jsonp'])) {
                 $config['view']['mime_types']['jsonp'] = $config['view']['jsonp_handler']['mime_type'];

Resources/config/view.xml

@@ -8,7 +8,6 @@
         <parameter key="fos_rest.serializer.exclusion_strategy.version" />
         <parameter key="fos_rest.serializer.exclusion_strategy.groups"/>
         <parameter key="fos_rest.view_handler.jsonp.callback_param"/>
-        <parameter key="fos_rest.view_handler.jsonp.callback_filter"/>
         <parameter key="fos_rest.view.exception_wrapper_handler" />
         <parameter key="fos_rest.view_handler.default.class">FOS\RestBundle\View\ViewHandler</parameter>
         <parameter key="fos_rest.view_handler.jsonp.class">FOS\RestBundle\View\JsonpHandler</parameter>
@@ -39,7 +38,6 @@
 
         <service id="fos_rest.view_handler.jsonp" class="%fos_rest.view_handler.jsonp.class%" public="false">
             <argument>%fos_rest.view_handler.jsonp.callback_param%</argument>
-            <argument>%fos_rest.view_handler.jsonp.callback_filter%</argument>
         </service>
 
         <service id="fos_rest.view.exception_wrapper_handler" class="%fos_rest.view.exception_wrapper_handler%" />

Resources/doc/2-the-view-layer.md

@@ -292,7 +292,6 @@ fos_rest:
     view:
         jsonp_handler:
            callback_param:       mycallback
-           callback_filter:      /^[a-z0-9_]+$/i
 ```
 
 Finally the filter can also be disabled by setting it to false.

Resources/doc/configuration-reference.md

@@ -54,7 +54,6 @@ fos_rest:
         serialize_null:       false
         jsonp_handler:
             callback_param:       callback
-            callback_filter:      /(^[a-z0-9_]+$)|(^YUI\.Env\.JSONP\._[0-9]+$)/i
             mime_type:            application/javascript+jsonp
     exception:
         codes:
@@ -77,13 +76,13 @@ fos_rest:
             -
                 # URL path info
                 path:                 ~
-    
+
                 # URL host name
                 host:                 ~
                 prefer_extension:     true
                 fallback_format:      html
                 priorities:
-    
+
                     # Prototype
                     name:                 []
         media_type:

Tests/DependencyInjection/FOSRestExtensionTest.php

@@ -474,4 +474,12 @@ public function testCheckExceptionWrapperHandler()
         $exceptionWrapperHandler = $this->container->getDefinition('fos_rest.view.exception_wrapper_handler');
         $this->assertEquals('%fos_rest.view.exception_wrapper_handler%', $exceptionWrapperHandler->getClass());
     }
+
+    /**
+     * @expectedException \LogicException
+     */
+    public function testExceptionThrownIfCallbackFilterIsUsed()
+    {
+        $this->extension->load(array('fos_rest' => array('view' => array('jsonp_handler' => array('callback_filter' => 'foo')))), $this->container);
+    }
 }

Tests/View/JsonpHandlerTest.php

@@ -28,12 +28,12 @@ class JsonpHandlerTest extends \PHPUnit_Framework_TestCase
     /**
      * @dataProvider handleDataProvider
      */
-    public function testHandle($query, $callbackFilter = '/(^[a-z0-9_]+$)|(^YUI\.Env\.JSONP\._[0-9]+$)/i')
+    public function testHandle($query)
     {
         $data = array('foo' => 'bar');
 
         $viewHandler = new ViewHandler(array('jsonp' => false));
-        $jsonpHandler = new JsonpHandler(key($query), $callbackFilter);
+        $jsonpHandler = new JsonpHandler(key($query));
         $viewHandler->registerHandler('jsonp', array($jsonpHandler, 'createResponse'));
 
         $container = $this->getMock('Symfony\Component\DependencyInjection\Container', array('get', 'getParameter'));
@@ -76,15 +76,15 @@ public static function handleDataProvider()
     }
 
     /**
-     * @expectedException \Symfony\Component\HttpKernel\Exception\HttpException
+     * @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
      * @dataProvider getCallbackFailureDataProvider
      */
-    public function testGetCallbackFailure(Request $request, $callbackFilter = '/(^[a-z0-9_]+$)|(^YUI\.Env\.JSONP\._[0-9]+$)/i')
+    public function testGetCallbackFailure(Request $request)
     {
         $data = array('foo' => 'bar');
 
         $viewHandler = new ViewHandler(array('jsonp' => false));
-        $jsonpHandler = new JsonpHandler('callback', $callbackFilter);
+        $jsonpHandler = new JsonpHandler('callback');
         $viewHandler->registerHandler('jsonp', array($jsonpHandler, 'createResponse'));
 
         $container = $this->getMock('Symfony\Component\DependencyInjection\Container', array('get', 'getParameter'));
@@ -121,7 +121,6 @@ public function getCallbackFailureDataProvider()
             'incorrect callback param name'  => array(new Request(array('foo' => 'bar'))),
             'incorrect callback param value' => array(new Request(array('callback' => 'ding.dong'))),
             'incorrect callback param name and value' => array(new Request(array('foo' => 'bar'))),
-            'incorrect callback param value with a custom filter' => array(new Request(array('foo' => 'bar')), '/[0-9]+/'),
         );
     }
 }

View/JsonpHandler.php

@@ -23,21 +23,19 @@
 class JsonpHandler
 {
     protected $callbackParam;
-    protected $callbackFilter;
 
-    public function __construct($callbackParam, $callbackFilter)
+    public function __construct($callbackParam)
     {
         $this->callbackParam = $callbackParam;
-        $this->callbackFilter = $callbackFilter;
     }
 
     protected function getCallback(Request $request)
     {
-        $callback = $request->query->get($this->callbackParam);
+        $callback  = $request->query->get($this->callbackParam);
+        $validator = new \JsonpCallbackValidator();
 
-        if ($this->callbackFilter && !preg_match($this->callbackFilter, $callback)) {
-            $msg = "Callback '$callback' does not match '{$this->callbackFilter}'";
-            throw new BadRequestHttpException($msg);
+        if (!$validator->validate($this->callbackParam)) {
+            throw new BadRequestHttpException('Invalid JSONP callback value');
         }
 
         return $callback;

composer.json

@@ -24,7 +24,8 @@
         "php":                            ">=5.3.2",
         "symfony/framework-bundle":       "~2.2",
         "doctrine/inflector":             "1.0.*",
-        "willdurand/negotiation":         "~1.2.0"
+        "willdurand/negotiation":         "~1.2.0",
+        "willdurand/jsonp-callback-validator": "~1.0.0"
     },
 
     "require-dev": {