Moved most of the logic from ResponseListener to the Response::prepar…

…e() method That allows projects that only use HttpFoundation and not HttpKernel to be able to enforce the HTTP specification "rules". $request = Request::createFromGlobals(); $response = new Response(); // do whatever you want with the Respons // enforce HTTP spec $response->prepare($request); $response->send(); Within Symfony2, the prepare method is automatically called by the ResponseListener.
symfony · Oct 18, 2011 · 347053c · 347053c
1 parent fbbda26
commit 347053c
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 91 deletions.
diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md
@@ -61,6 +61,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
 
 ### HttpFoundation
 
+ * made Response::prepare() method the place to enforce HTTP specification
  * [BC BREAK] moved management of the locale from the Session class to the Request class
  * added a generic access to the PHP built-in filter mechanism: ParameterBag::filter()
  * made FileBinaryMimeTypeGuesser command configurable

diff --git a/src/Symfony/Component/HttpFoundation/Response.php b/src/Symfony/Component/HttpFoundation/Response.php
@@ -96,14 +96,18 @@ public function __construct($content = '', $status = 200, $headers = array())
     }
 
     /**
-     * Returns the response content as it will be sent (with the headers).
+     * Returns the Response as an HTTP string.
      *
-     * @return string The response content
+     * The string representation of the Resonse is the same as the
+     * one that will be sent to the client only if the prepare() method
+     * has been called before.
+     *
+     * @return string The Response as an HTTP string
+     *
+     * @see prepare()
      */
     public function __toString()
     {
-        $this->prepare();
-
         return
             sprintf('HTTP/%s %s %s', $this->version, $this->statusCode, $this->statusText)."\r\n".
             $this->headers."\r\n".
@@ -122,26 +126,48 @@ public function __clone()
      * Prepares the Response before it is sent to the client.
      *
      * This method tweaks the Response to ensure that it is
-     * compliant with RFC 2616.
+     * compliant with RFC 2616. Most of the changes are based on
+     * the Request that is "associated" with this Response.
+     *
+     * @param Request $request A Request instance
      */
-    public function prepare()
+    public function prepare(Request $request)
     {
+        $headers = $this->headers;
+
         if ($this->isInformational() || in_array($this->statusCode, array(204, 304))) {
             $this->setContent('');
         }
 
+        // Content-type based on the Request
+        if (!$headers->has('Content-Type')) {
+            $format = $request->getRequestFormat();
+            if (null !== $format && $mimeType = $request->getMimeType($format)) {
+                $headers->set('Content-Type', $mimeType);
+            }
+        }
+
         // Fix Content-Type
         $charset = $this->charset ?: 'UTF-8';
-        if (!$this->headers->has('Content-Type')) {
-            $this->headers->set('Content-Type', 'text/html; charset='.$charset);
-        } elseif ('text/' === substr($this->headers->get('Content-Type'), 0, 5) && false === strpos($this->headers->get('Content-Type'), 'charset')) {
+        if (!$headers->has('Content-Type')) {
+            $headers->set('Content-Type', 'text/html; charset='.$charset);
+        } elseif ('text/' === substr($headers->get('Content-Type'), 0, 5) && false === strpos($headers->get('Content-Type'), 'charset')) {
             // add the charset
-            $this->headers->set('Content-Type', $this->headers->get('Content-Type').'; charset='.$charset);
+            $headers->set('Content-Type', $headers->get('Content-Type').'; charset='.$charset);
         }
 
         // Fix Content-Length
-        if ($this->headers->has('Transfer-Encoding')) {
-            $this->headers->remove('Content-Length');
+        if ($headers->has('Transfer-Encoding')) {
+            $headers->remove('Content-Length');
+        }
+
+        if ('HEAD' === $request->getMethod()) {
+            // cf. RFC2616 14.13
+            $length = $headers->get('Content-Length');
+            $this->setContent('');
+            if ($length) {
+                $headers->set('Content-Length', $length);
+            }
         }
     }
 
@@ -155,8 +181,6 @@ public function sendHeaders()
             return;
         }
 
-        $this->prepare();
-
         // status
         header(sprintf('HTTP/%s %s %s', $this->version, $this->statusCode, $this->statusText));
 

diff --git a/src/Symfony/Component/HttpKernel/EventListener/ResponseListener.php b/src/Symfony/Component/HttpKernel/EventListener/ResponseListener.php
@@ -12,7 +12,6 @@
 namespace Symfony\Component\HttpKernel\EventListener;
 
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
-use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
@@ -34,38 +33,21 @@ public function __construct($charset)
     /**
      * Filters the Response.
      *
-     * @param FilterResponseEvent $event    A FilterResponseEvent instance
+     * @param FilterResponseEvent $event A FilterResponseEvent instance
      */
     public function onKernelResponse(FilterResponseEvent $event)
     {
-        $request = $event->getRequest();
-        $response = $event->getResponse();
-
-        if ('HEAD' === $request->getMethod()) {
-            // cf. RFC2616 14.13
-            $length = $response->headers->get('Content-Length');
-            $response->setContent('');
-            if ($length) {
-                $response->headers->set('Content-Length', $length);
-            }
-        }
-
         if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
             return;
         }
 
+        $response = $event->getResponse();
+
         if (null === $response->getCharset()) {
             $response->setCharset($this->charset);
         }
 
-        if ($response->headers->has('Content-Type')) {
-            return;
-        }
-
-        $format = $request->getRequestFormat();
-        if ((null !== $format) && $mimeType = $request->getMimeType($format)) {
-            $response->headers->set('Content-Type', $mimeType);
-        }
+        $response->prepare($event->getRequest());
     }
 
     static public function getSubscribedEvents()

diff --git a/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php b/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
@@ -200,7 +200,7 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
             }
         }
 
-        $response->prepare();
+        $response->prepare($request);
 
         return $response;
     }

diff --git a/tests/Symfony/Tests/Component/HttpFoundation/ResponseTest.php b/tests/Symfony/Tests/Component/HttpFoundation/ResponseTest.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Tests\Component\HttpFoundation;
 
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 
 class ResponseTest extends \PHPUnit_Framework_TestCase
@@ -182,21 +183,15 @@ public function testDefaultContentType()
         $headerMock = $this->getMock('Symfony\Component\HttpFoundation\ResponseHeaderBag', array('set'));
         $headerMock->expects($this->at(0))
             ->method('set')
-            ->with('Content-Type', 'text/html; charset=UTF-8');
+            ->with('Content-Type', 'text/html');
         $headerMock->expects($this->at(1))
             ->method('set')
-            ->with('Content-Type', 'text/html; charset=Foo');
+            ->with('Content-Type', 'text/html; charset=UTF-8');
 
         $response = new Response('foo');
         $response->headers = $headerMock;
 
-        // verify first set()
-        $response->__toString();
-
-        $response->headers->remove('Content-Type');
-        $response->setCharset('Foo');
-        // verify second set()
-        $response->__toString();
+        $response->prepare(new Request());
     }
 
     public function testContentTypeCharset()
@@ -205,11 +200,51 @@ public function testContentTypeCharset()
         $response->headers->set('Content-Type', 'text/css');
 
         // force fixContentType() to be called
-        $response->__toString();
+        $response->prepare(new Request());
 
         $this->assertEquals('text/css; charset=UTF-8', $response->headers->get('Content-Type'));
     }
 
+    public function testPrepareDoesNothingIfContentTypeIsSet()
+    {
+        $response = new Response('foo');
+        $response->headers->set('Content-Type', 'text/plain');
+
+        $response->prepare(new Request());
+
+        $this->assertEquals('text/plain; charset=UTF-8', $response->headers->get('content-type'));
+    }
+
+    public function testPrepareDoesNothingIfRequestFormatIsNotDefined()
+    {
+        $response = new Response('foo');
+
+        $response->prepare(new Request());
+
+        $this->assertEquals('text/html; charset=UTF-8', $response->headers->get('content-type'));
+    }
+
+    public function testPrepareSetContentType()
+    {
+        $response = new Response('foo');
+        $request = Request::create('/');
+        $request->setRequestFormat('json');
+
+        $response->prepare($request);
+
+        $this->assertEquals('application/json', $response->headers->get('content-type'));
+    }
+
+    public function testPrepareRemovesContentForHeadRequests()
+    {
+        $response = new Response('foo');
+        $request = Request::create('/', 'HEAD');
+
+        $response->prepare($request);
+
+        $this->assertEquals('', $response->getContent());
+    }
+
     public function testSetCache()
     {
         $response = new Response();

diff --git a/tests/Symfony/Tests/Component/HttpKernel/EventListener/ResponseListenerTest.php b/tests/Symfony/Tests/Component/HttpKernel/EventListener/ResponseListenerTest.php
@@ -51,50 +51,6 @@ public function testFilterDoesNothingForSubRequests()
         $this->assertEquals('', $event->getResponse()->headers->get('content-type'));
     }
 
-    public function testFilterDoesNothingIfContentTypeIsSet()
-    {
-        $response = new Response('foo');
-        $response->headers->set('Content-Type', 'text/plain');
-
-        $event = new FilterResponseEvent($this->kernel, new Request(), HttpKernelInterface::MASTER_REQUEST, $response);
-        $this->dispatcher->dispatch(KernelEvents::RESPONSE, $event);
-
-        $this->assertEquals('text/plain', $event->getResponse()->headers->get('content-type'));
-    }
-
-    public function testFilterDoesNothingIfRequestFormatIsNotDefined()
-    {
-        $response = new Response('foo');
-
-        $event = new FilterResponseEvent($this->kernel, Request::create('/'), HttpKernelInterface::MASTER_REQUEST, $response);
-        $this->dispatcher->dispatch(KernelEvents::RESPONSE, $event);
-
-        $this->assertEquals('text/html', $event->getResponse()->headers->get('content-type'));
-    }
-
-    public function testFilterSetContentType()
-    {
-        $response = new Response('foo');
-        $request = Request::create('/');
-        $request->setRequestFormat('json');
-
-        $event = new FilterResponseEvent($this->kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response);
-        $this->dispatcher->dispatch(KernelEvents::RESPONSE, $event);
-
-        $this->assertEquals('application/json', $event->getResponse()->headers->get('content-type'));
-    }
-
-    public function testFilterRemovesContentForHeadRequests()
-    {
-        $response = new Response('foo');
-        $request = Request::create('/', 'HEAD');
-
-        $event = new FilterResponseEvent($this->kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response);
-        $this->dispatcher->dispatch(KernelEvents::RESPONSE, $event);
-
-        $this->assertEquals('', $response->getContent());
-    }
-
     public function testFilterSetsNonDefaultCharsetIfNotOverridden()
     {
         $listener = new ResponseListener('ISO-8859-15');