Fix up tests/behavior for CorsBuilder.

Change CorsBuilder to use immutable methods. This requires a matching
change in Response to copy new headers into the current response when
using the deprecated mode of cors().

src/Http/Response.php

@@ -2350,12 +2350,26 @@ class Response implements ResponseInterface
         if (empty($allowedDomains) && empty($allowedMethods) && empty($allowedHeaders)) {
             return $builder;
         }
+        deprecationWarning(
+            'The $allowedDomains, $allowedMethods, and $allowedHeaders parameters of Response::cors() ' .
+            'are deprecated. Instead you should use the builder methods on the return of cors().'
+        );
 
-        $builder->allowOrigin($allowedDomains)
+        $updated = $builder->allowOrigin($allowedDomains)
             ->allowMethods((array)$allowedMethods)
             ->allowHeaders((array)$allowedHeaders)
             ->build();
 
+        // If $updated is a new instance, mutate this object in-place
+        // to retain existing behavior.
+        if ($updated !== $this) {
+            foreach ($updated->getHeaders() as $name => $values) {
+                if (!$this->hasHeader($name)) {
+                    $this->_setHeader($name, $values[0]);
+                }
+            }
+        }
+
         return $builder;
     }
 

src/Network/CorsBuilder.php

@@ -14,7 +14,7 @@
  */
 namespace Cake\Network;
 
-use Cake\Http\Response as HttpResponse;
+use Psr\Http\Message\MessageInterface;
 
 /**
  * A builder object that assists in defining Cross Origin Request related
@@ -34,7 +34,7 @@ class CorsBuilder
     /**
      * The response object this builder is attached to.
      *
-     * @var \Cake\Http\Response
+     * @var \Psr\Http\Message\MessageInterface
      */
     protected $_response;
 
@@ -62,11 +62,11 @@ class CorsBuilder
     /**
      * Constructor.
      *
-     * @param \Cake\Http\Response $response The response object to add headers onto.
+     * @param \Psr\Http\Message\MessageInterface $response The response object to add headers onto.
      * @param string $origin The request's Origin header.
      * @param bool $isSsl Whether or not the request was over SSL.
      */
-    public function __construct(HttpResponse $response, $origin, $isSsl = false)
+    public function __construct(MessageInterface $response, $origin, $isSsl = false)
     {
         $this->_origin = $origin;
         $this->_isSsl = $isSsl;
@@ -79,18 +79,22 @@ class CorsBuilder
      * If the builder has no Origin, or if there are no allowed domains,
      * or if the allowed domains do not match the Origin header no headers will be applied.
      *
-     * @return \Cake\Http\Response
+     * @return \Psr\Http\Message\MessageInterface A new instance of the response with new headers.
      */
     public function build()
     {
+        $response = $this->_response;
         if (empty($this->_origin)) {
-            return $this->_response;
+            return $response;
         }
+
         if (isset($this->_headers['Access-Control-Allow-Origin'])) {
-            $this->_response->header($this->_headers);
+            foreach ($this->_headers as $key => $value) {
+                $response = $response->withHeader($key, $value);
+            }
         }
 
-        return $this->_response;
+        return $response;
     }
 
     /**

tests/TestCase/Http/ResponseTest.php

@@ -20,6 +20,7 @@ use Cake\Http\Cookie\Cookie;
 use Cake\Http\Cookie\CookieCollection;
 use Cake\Http\Response;
 use Cake\Http\ServerRequest;
+use Cake\Network\CorsBuilder;
 use Cake\Network\Exception\NotFoundException;
 use Cake\TestSuite\TestCase;
 use Zend\Diactoros\Stream;
@@ -1650,8 +1651,25 @@ class ResponseTest extends TestCase
     }
 
     /**
-     * Test CORS
+     * Test that cors() returns a builder.
      *
+     * @return void
+     */
+    public function testCors()
+    {
+        $request = new ServerRequest([
+            'environment' => ['HTTP_ORIGIN' => 'http://example.com']
+        ]);
+        $response = new Response();
+        $builder = $response->cors($request);
+        $this->assertInstanceOf(CorsBuilder::class, $builder);
+        $this->assertSame($response, $builder->build(), 'Empty builder returns same object');
+    }
+
+    /**
+     * Test CORS with additional parameters
+     *
+     * @group deprecated
      * @dataProvider corsData
      * @param Request $request
      * @param string $origin
@@ -1663,27 +1681,28 @@ class ResponseTest extends TestCase
      * @param string|bool $expectedHeaders
      * @return void
      */
-    public function testCors($request, $origin, $domains, $methods, $headers, $expectedOrigin, $expectedMethods = false, $expectedHeaders = false)
+    public function testCorsParameters($request, $origin, $domains, $methods, $headers, $expectedOrigin, $expectedMethods = false, $expectedHeaders = false)
     {
-        $request = $request->withEnv('HTTP_ORIGIN', $origin);
-        $response = new Response();
+        $this->deprecated(function () use ($request, $origin, $domains, $methods, $headers, $expectedOrigin, $expectedMethods, $expectedHeaders) {
+            $request = $request->withEnv('HTTP_ORIGIN', $origin);
+            $response = new Response();
 
-        $result = $response->cors($request, $domains, $methods, $headers);
-        $this->assertInstanceOf('Cake\Network\CorsBuilder', $result);
+            $result = $response->cors($request, $domains, $methods, $headers);
+            $this->assertInstanceOf('Cake\Network\CorsBuilder', $result);
 
-        if ($expectedOrigin) {
-            $this->assertTrue($response->hasHeader('Access-Control-Allow-Origin'));
-            $this->assertEquals($expectedOrigin, $response->getHeaderLine('Access-Control-Allow-Origin'));
-        }
-        if ($expectedMethods) {
-            $this->assertTrue($response->hasHeader('Access-Control-Allow-Methods'));
-            $this->assertEquals($expectedMethods, $response->getHeaderLine('Access-Control-Allow-Methods'));
-        }
-        if ($expectedHeaders) {
-            $this->assertTrue($response->hasHeader('Access-Control-Allow-Headers'));
-            $this->assertEquals($expectedHeaders, $response->getHeaderLine('Access-Control-Allow-Headers'));
-        }
-        unset($_SERVER['HTTP_ORIGIN']);
+            if ($expectedOrigin) {
+                $this->assertTrue($response->hasHeader('Access-Control-Allow-Origin'));
+                $this->assertEquals($expectedOrigin, $response->getHeaderLine('Access-Control-Allow-Origin'));
+            }
+            if ($expectedMethods) {
+                $this->assertTrue($response->hasHeader('Access-Control-Allow-Methods'));
+                $this->assertEquals($expectedMethods, $response->getHeaderLine('Access-Control-Allow-Methods'));
+            }
+            if ($expectedHeaders) {
+                $this->assertTrue($response->hasHeader('Access-Control-Allow-Headers'));
+                $this->assertEquals($expectedHeaders, $response->getHeaderLine('Access-Control-Allow-Headers'));
+            }
+        });
     }
 
     /**

tests/TestCase/Network/CorsBuilderTest.php

@@ -168,9 +168,8 @@ class CorsBuilderTest extends TestCase
      */
     protected function assertHeader($expected, Response $response, $header)
     {
-        $headers = $response->header();
-        $this->assertArrayHasKey($header, $headers, 'Header key not found.');
-        $this->assertEquals($expected, $headers[$header], 'Header value not found.');
+        $this->assertTrue($response->hasHeader($header), 'Header key not found.');
+        $this->assertEquals($expected, $response->getHeaderLine($header), 'Header value not found.');
     }
 
     /**
@@ -181,7 +180,6 @@ class CorsBuilderTest extends TestCase
      */
     protected function assertNoHeader(Response $response, $header)
     {
-        $headers = $response->header();
-        $this->assertArrayNotHasKey($header, $headers, 'Header key was found.');
+        $this->assertFalse($response->hasHeader($header), 'Header key was found.');
     }
 }