From 678822459baefb992740a5b46fddefbd872fca17 Mon Sep 17 00:00:00 2001 From: Toni Uebernickel Date: Mon, 19 Mar 2012 18:27:08 +0100 Subject: [PATCH 1/5] add JSONP support to JsonResponse --- .../Component/HttpFoundation/JsonResponse.php | 21 ++++++++++++++----- .../HttpFoundation/JsonResponseTest.php | 8 +++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/JsonResponse.php b/src/Symfony/Component/HttpFoundation/JsonResponse.php index 8e02926e29..ef668fdf7b 100644 --- a/src/Symfony/Component/HttpFoundation/JsonResponse.php +++ b/src/Symfony/Component/HttpFoundation/JsonResponse.php @@ -24,26 +24,37 @@ class JsonResponse extends Response * @param mixed $data The response data * @param integer $status The response status code * @param array $headers An array of response headers + * @param string $jsonp A JSONP callback name */ - public function __construct($data = array(), $status = 200, $headers = array()) + public function __construct($data = array(), $status = 200, $headers = array(), $jsonp = '') { // root should be JSON object, not array if (is_array($data) && 0 === count($data)) { $data = new \ArrayObject(); } + $content = json_encode($data); + $contentType = 'application/json'; + if (!empty($jsonp)) { + $content = sprintf('%s(%s);', $jsonp, $content); + // Not using application/javascript for compatibility reasons with older browsers. + $contentType = 'text/javascript'; + } + parent::__construct( - json_encode($data), + $content, $status, - array_merge(array('Content-Type' => 'application/json'), $headers) + array_merge(array('Content-Type' => $contentType), $headers) ); } /** * {@inheritDoc} + * + * @param string $jsonp A JSONP callback name. */ - static public function create($data = array(), $status = 200, $headers = array()) + static public function create($data = array(), $status = 200, $headers = array(), $jsonp = '') { - return new static($data, $status, $headers); + return new static($data, $status, $headers, $jsonp = ''); } } diff --git a/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php b/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php index 0f373da0ac..51c33e91ea 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php @@ -86,4 +86,12 @@ class JsonResponseTest extends \PHPUnit_Framework_TestCase $this->assertEquals('{"foo":"bar"}', $response->getContent()); $this->assertEquals(204, $response->getStatusCode()); } + + public function testJsonp() + { + $response = new JsonResponse(array('foo' => 'bar'), 200, array(), 'callback'); + + $this->assertEquals('callback({"foo":"bar"});', $response->getContent()); + $this->assertEquals('text/javascript', $response->headers->get('Content-Type')); + } } From 38b79a7023dc38f1bb49ef664f459df1d8c1e276 Mon Sep 17 00:00:00 2001 From: Toni Uebernickel Date: Mon, 19 Mar 2012 19:40:54 +0100 Subject: [PATCH 2/5] add data and callback setter to JsonResponse --- .../Component/HttpFoundation/JsonResponse.php | 75 ++++++++++++++----- .../HttpFoundation/JsonResponseTest.php | 10 +++ 2 files changed, 68 insertions(+), 17 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/JsonResponse.php b/src/Symfony/Component/HttpFoundation/JsonResponse.php index ef668fdf7b..2f756e8c59 100644 --- a/src/Symfony/Component/HttpFoundation/JsonResponse.php +++ b/src/Symfony/Component/HttpFoundation/JsonResponse.php @@ -18,6 +18,9 @@ namespace Symfony\Component\HttpFoundation; */ class JsonResponse extends Response { + protected $data; + protected $callback; + /** * Constructor. * @@ -28,24 +31,10 @@ class JsonResponse extends Response */ public function __construct($data = array(), $status = 200, $headers = array(), $jsonp = '') { - // root should be JSON object, not array - if (is_array($data) && 0 === count($data)) { - $data = new \ArrayObject(); - } + parent::__construct('', $status, $headers); - $content = json_encode($data); - $contentType = 'application/json'; - if (!empty($jsonp)) { - $content = sprintf('%s(%s);', $jsonp, $content); - // Not using application/javascript for compatibility reasons with older browsers. - $contentType = 'text/javascript'; - } - - parent::__construct( - $content, - $status, - array_merge(array('Content-Type' => $contentType), $headers) - ); + $this->setData($data); + $this->setCallback($jsonp); } /** @@ -57,4 +46,56 @@ class JsonResponse extends Response { return new static($data, $status, $headers, $jsonp = ''); } + + /** + * Sets the JSONP callback. + * + * @param string $callback + * + * @return JsonResponse + */ + public function setCallback($callback) + { + $this->callback = $callback; + + return $this->update(); + } + + /** + * Sets the data to be sent as json. + * + * @param mixed $data + * + * @return JsonResponse + */ + public function setData($data = array()) + { + // root should be JSON object, not array + if (is_array($data) && 0 === count($data)) { + $data = new \ArrayObject(); + } + + $this->data = json_encode($data); + + return $this->update(); + } + + /** + * Updates the content and headers according to the json data and callback. + * + * @return JsonResponse + */ + protected function update() + { + $content = $this->data; + $this->headers->set('Content-Type', 'application/json', false); + + if (!empty($this->callback)) { + $content = sprintf('%s(%s);', $this->callback, $content); + // Not using application/javascript for compatibility reasons with older browsers. + $this->headers->set('Content-Type', 'text/javascript', true); + } + + return $this->setContent($content); + } } diff --git a/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php b/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php index 51c33e91ea..ca7687d1af 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php @@ -15,6 +15,8 @@ use Symfony\Component\HttpFoundation\JsonResponse; /** * @covers Symfony\Component\HttpFoundation\JsonResponse::__construct + * @covers Symfony\Component\HttpFoundation\JsonResponse::setData + * @covers Symfony\Component\HttpFoundation\JsonResponse::setCallback */ class JsonResponseTest extends \PHPUnit_Framework_TestCase { @@ -94,4 +96,12 @@ class JsonResponseTest extends \PHPUnit_Framework_TestCase $this->assertEquals('callback({"foo":"bar"});', $response->getContent()); $this->assertEquals('text/javascript', $response->headers->get('Content-Type')); } + + public function testSetCallback() + { + $response = JsonResponse::create(array('foo' => 'bar'))->setCallback('callback'); + + $this->assertEquals('callback({"foo":"bar"});', $response->getContent()); + $this->assertEquals('text/javascript', $response->headers->get('Content-Type')); + } } From 266f76d963ad3fe3521dc292b95fed5a4434fda3 Mon Sep 17 00:00:00 2001 From: Toni Uebernickel Date: Tue, 20 Mar 2012 10:10:35 +0100 Subject: [PATCH 3/5] rename jsonp to callback, defaults to null --- .../Component/HttpFoundation/JsonResponse.php | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/JsonResponse.php b/src/Symfony/Component/HttpFoundation/JsonResponse.php index 2f756e8c59..9cfda963c7 100644 --- a/src/Symfony/Component/HttpFoundation/JsonResponse.php +++ b/src/Symfony/Component/HttpFoundation/JsonResponse.php @@ -24,27 +24,27 @@ class JsonResponse extends Response /** * Constructor. * - * @param mixed $data The response data - * @param integer $status The response status code - * @param array $headers An array of response headers - * @param string $jsonp A JSONP callback name + * @param mixed $data The response data + * @param integer $status The response status code + * @param array $headers An array of response headers + * @param string $callback A JSONP callback name */ - public function __construct($data = array(), $status = 200, $headers = array(), $jsonp = '') + public function __construct($data = array(), $status = 200, $headers = array(), $callback = null) { parent::__construct('', $status, $headers); $this->setData($data); - $this->setCallback($jsonp); + $this->setCallback($callback); } /** * {@inheritDoc} * - * @param string $jsonp A JSONP callback name. + * @param string $callback A JSONP callback name. */ - static public function create($data = array(), $status = 200, $headers = array(), $jsonp = '') + static public function create($data = array(), $status = 200, $headers = array(), $callback = null) { - return new static($data, $status, $headers, $jsonp = ''); + return new static($data, $status, $headers, $callback); } /** @@ -54,7 +54,7 @@ class JsonResponse extends Response * * @return JsonResponse */ - public function setCallback($callback) + public function setCallback($callback = null) { $this->callback = $callback; @@ -90,7 +90,7 @@ class JsonResponse extends Response $content = $this->data; $this->headers->set('Content-Type', 'application/json', false); - if (!empty($this->callback)) { + if ($this->callback) { $content = sprintf('%s(%s);', $this->callback, $content); // Not using application/javascript for compatibility reasons with older browsers. $this->headers->set('Content-Type', 'text/javascript', true); From 601b87ca01a823484470aea0d57b0b5051656c0c Mon Sep 17 00:00:00 2001 From: Toni Uebernickel Date: Tue, 20 Mar 2012 11:05:22 +0100 Subject: [PATCH 4/5] add basic validation of callback name --- src/Symfony/Component/HttpFoundation/JsonResponse.php | 8 ++++++++ .../Tests/Component/HttpFoundation/JsonResponseTest.php | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/src/Symfony/Component/HttpFoundation/JsonResponse.php b/src/Symfony/Component/HttpFoundation/JsonResponse.php index 9cfda963c7..c04b55a658 100644 --- a/src/Symfony/Component/HttpFoundation/JsonResponse.php +++ b/src/Symfony/Component/HttpFoundation/JsonResponse.php @@ -56,6 +56,14 @@ class JsonResponse extends Response */ public function setCallback($callback = null) { + if ($callback) { + // taken from http://www.geekality.net/2011/08/03/valid-javascript-identifier/ + $pattern = '/^[$_\p{L}][$_\p{L}\p{Mn}\p{Mc}\p{Nd}\p{Pc}\x{200C}\x{200D}]*+$/u'; + if (!preg_match($pattern, $callback)) { + throw new \InvalidArgumentException('The callback name is not valid.'); + } + } + $this->callback = $callback; return $this->update(); diff --git a/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php b/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php index ca7687d1af..6b08967932 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php @@ -104,4 +104,12 @@ class JsonResponseTest extends \PHPUnit_Framework_TestCase $this->assertEquals('callback({"foo":"bar"});', $response->getContent()); $this->assertEquals('text/javascript', $response->headers->get('Content-Type')); } + + public function testSetCallbackInvalidIdentifier() + { + $response = new JsonResponse('foo'); + + $this->setExpectedException('InvalidArgumentException'); + $response->setCallback('+invalid'); + } } From 4a43453db8c9f1509a6e86e7bdc78437fc367ea1 Mon Sep 17 00:00:00 2001 From: Toni Uebernickel Date: Wed, 21 Mar 2012 22:40:19 +0100 Subject: [PATCH 5/5] remove callback from constructor and create method --- src/Symfony/Component/HttpFoundation/JsonResponse.php | 10 +++------- .../Component/HttpFoundation/JsonResponseTest.php | 8 -------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/JsonResponse.php b/src/Symfony/Component/HttpFoundation/JsonResponse.php index c04b55a658..85b607132e 100644 --- a/src/Symfony/Component/HttpFoundation/JsonResponse.php +++ b/src/Symfony/Component/HttpFoundation/JsonResponse.php @@ -27,24 +27,20 @@ class JsonResponse extends Response * @param mixed $data The response data * @param integer $status The response status code * @param array $headers An array of response headers - * @param string $callback A JSONP callback name */ - public function __construct($data = array(), $status = 200, $headers = array(), $callback = null) + public function __construct($data = array(), $status = 200, $headers = array()) { parent::__construct('', $status, $headers); $this->setData($data); - $this->setCallback($callback); } /** * {@inheritDoc} - * - * @param string $callback A JSONP callback name. */ - static public function create($data = array(), $status = 200, $headers = array(), $callback = null) + static public function create($data = array(), $status = 200, $headers = array()) { - return new static($data, $status, $headers, $callback); + return new static($data, $status, $headers); } /** diff --git a/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php b/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php index 6b08967932..d95f46f897 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/JsonResponseTest.php @@ -89,14 +89,6 @@ class JsonResponseTest extends \PHPUnit_Framework_TestCase $this->assertEquals(204, $response->getStatusCode()); } - public function testJsonp() - { - $response = new JsonResponse(array('foo' => 'bar'), 200, array(), 'callback'); - - $this->assertEquals('callback({"foo":"bar"});', $response->getContent()); - $this->assertEquals('text/javascript', $response->headers->get('Content-Type')); - } - public function testSetCallback() { $response = JsonResponse::create(array('foo' => 'bar'))->setCallback('callback');