Merge MC-30971 into 2.3-bugfixes-02072020

dvoskoboinikov · web-flow · commit 722d98914422 · 2020-02-07T16:48:38.000-06:00
diff --git a/app/code/Magento/Csp/Model/Policy/Renderer/SimplePolicyHeaderRenderer.php b/app/code/Magento/Csp/Model/Policy/Renderer/SimplePolicyHeaderRenderer.php
@@ -45,7 +45,7 @@ public function render(PolicyInterface $policy, HttpResponse $response): void
             $header = 'Content-Security-Policy';
         }
         $value = $policy->getId() .' ' .$policy->getValue() .';';
-        if ($config->getReportUri()) {
+        if ($config->getReportUri() && !$response->getHeader('Report-To')) {
             $reportToData = [
                 'group' => 'report-endpoint',
                 'max_age' => 10886400,
@@ -57,7 +57,10 @@ public function render(PolicyInterface $policy, HttpResponse $response): void
             $value .= ' report-to '. $reportToData['group'] .';';
             $response->setHeader('Report-To', json_encode($reportToData), true);
         }
-        $response->setHeader($header, $value, false);
+        if ($existing = $response->getHeader($header)) {
+            $value = $value .' ' .$existing->getFieldValue();
+        }
+        $response->setHeader($header, $value, true);
     }
 
     /**
diff --git a/dev/tests/integration/testsuite/Magento/Csp/CspAwareActionTest.php b/dev/tests/integration/testsuite/Magento/Csp/CspAwareActionTest.php
@@ -31,15 +31,13 @@ public function testAwareAction(): void
     {
         $this->getRequest()->setMethod('GET');
         $this->dispatch('csputil/csp/aware');
-        $headers = '';
-        foreach ($this->getResponse()->getHeaders() as $header) {
-            $headers .= $header->getFieldName() .': ' .$header->getFieldValue() .PHP_EOL;
-        }
+        $header = $this->getResponse()->getHeader('Content-Security-Policy');
+        $this->assertNotEmpty($header);
 
         $this->assertContains(
-            'Content-Security-Policy: script-src https://controller.magento.com'
+            'script-src https://controller.magento.com'
                 .' \'self\' \'sha256-H4RRnauTM2X2Xg/z9zkno1crqhsaY3uKKu97uwmnXXE=\'',
-            $headers
+            $header->getFieldValue()
         );
     }
 }
diff --git a/dev/tests/integration/testsuite/Magento/Csp/CspUtilTest.php b/dev/tests/integration/testsuite/Magento/Csp/CspUtilTest.php
@@ -20,6 +20,7 @@ class CspUtilTest extends AbstractController
      * Test that CSP helper for templates works.
      *
      * @return void
+     * @magentoConfigFixture default_store csp/mode/storefront/report_only 0
      */
     public function testPhtmlHelper(): void
     {
@@ -29,11 +30,9 @@ public function testPhtmlHelper(): void
 
         $this->assertContains('<script src="http://my.magento.com/static/script.js" />', $content);
         $this->assertContains("<script>\n    let myVar = 1;\n</script>", $content);
-        $headers = '';
-        foreach ($this->getResponse()->getHeaders() as $header) {
-            $headers .= $header->getFieldName() .': ' .$header->getFieldValue() .PHP_EOL;
-        }
-        $this->assertContains('http://my.magento.com', $headers);
-        $this->assertContains('\'sha256-H4RRnauTM2X2Xg/z9zkno1crqhsaY3uKKu97uwmnXXE=\'', $headers);
+        $header = $this->getResponse()->getHeader('Content-Security-Policy');
+        $this->assertNotEmpty($header);
+        $this->assertContains('http://my.magento.com', $header->getFieldValue());
+        $this->assertContains('\'sha256-H4RRnauTM2X2Xg/z9zkno1crqhsaY3uKKu97uwmnXXE=\'', $header->getFieldValue());
     }
 }
diff --git a/dev/tests/integration/testsuite/Magento/Csp/Model/Policy/Renderer/SimplePolicyHeaderRendererTest.php b/dev/tests/integration/testsuite/Magento/Csp/Model/Policy/Renderer/SimplePolicyHeaderRendererTest.php
@@ -53,13 +53,7 @@ public function testRenderRestrictMode(): void
 
         $this->assertNotEmpty($header = $this->response->getHeader('Content-Security-Policy'));
         $this->assertEmpty($this->response->getHeader('Content-Security-Policy-Report-Only'));
-        $contentSecurityPolicyContent = [];
-        if ($header instanceof \ArrayIterator) {
-            foreach ($header as $item) {
-                $contentSecurityPolicyContent[] = $item->getFieldValue();
-            }
-        }
-        $this->assertEquals(['default-src https://magento.com \'self\';'], $contentSecurityPolicyContent);
+        $this->assertEquals('default-src https://magento.com \'self\';', $header->getFieldValue());
     }
 
     /**
@@ -79,15 +73,9 @@ public function testRenderRestrictWithReportingMode(): void
 
         $this->assertNotEmpty($header = $this->response->getHeader('Content-Security-Policy'));
         $this->assertEmpty($this->response->getHeader('Content-Security-Policy-Report-Only'));
-        $contentSecurityPolicyContent = [];
-        if ($header instanceof \ArrayIterator) {
-            foreach ($header as $item) {
-                $contentSecurityPolicyContent[] = $item->getFieldValue();
-            }
-        }
         $this->assertEquals(
-            ['default-src https://magento.com \'self\'; report-uri /csp-reports/; report-to report-endpoint;'],
-            $contentSecurityPolicyContent
+            'default-src https://magento.com \'self\'; report-uri /csp-reports/; report-to report-endpoint;',
+            $header->getFieldValue()
         );
         $this->assertNotEmpty($reportToHeader = $this->response->getHeader('Report-To'));
         $this->assertNotEmpty($reportData = json_decode("[{$reportToHeader->getFieldValue()}]", true));
diff --git a/lib/internal/Magento/Framework/HTTP/PhpEnvironment/Response.php b/lib/internal/Magento/Framework/HTTP/PhpEnvironment/Response.php
@@ -28,6 +28,9 @@ public function getHeader($name)
         $headers = $this->getHeaders();
         if ($headers->has($name)) {
             $header = $headers->get($name);
+            if (is_iterable($header)) {
+                $header = $header[0];
+            }
         }
         return $header;
     }
@@ -94,8 +97,13 @@ public function clearHeader($name)
     {
         $headers = $this->getHeaders();
         if ($headers->has($name)) {
-            $header = $headers->get($name);
-            $headers->removeHeader($header);
+            $headerValues = $headers->get($name);
+            if (!is_iterable($headerValues)) {
+                $headerValues = [$headerValues];
+            }
+            foreach ($headerValues as $headerValue) {
+                $headers->removeHeader($headerValue);
+            }
         }
 
         return $this;
@@ -203,7 +211,6 @@ public function sendHeaders()
             header($header->toString(), false);
         }
 
-        $this->headersSent = true;
         return $this;
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ public function render(PolicyInterface $policy, HttpResponse $response): void`
`45`	`45`	`$header = 'Content-Security-Policy';`
`46`	`46`	`}`
`47`	`47`	`$value = $policy->getId() .' ' .$policy->getValue() .';';`
`48`		`- if ($config->getReportUri()) {`
	`48`	`+ if ($config->getReportUri() && !$response->getHeader('Report-To')) {`
`49`	`49`	`$reportToData = [`
`50`	`50`	`'group' => 'report-endpoint',`
`51`	`51`	`'max_age' => 10886400,`
`@@ -57,7 +57,10 @@ public function render(PolicyInterface $policy, HttpResponse $response): void`
`57`	`57`	`$value .= ' report-to '. $reportToData['group'] .';';`
`58`	`58`	`$response->setHeader('Report-To', json_encode($reportToData), true);`
`59`	`59`	`}`
`60`		`- $response->setHeader($header, $value, false);`
	`60`	`+ if ($existing = $response->getHeader($header)) {`
	`61`	`+ $value = $value .' ' .$existing->getFieldValue();`
	`62`	`+ }`
	`63`	`+ $response->setHeader($header, $value, true);`
`61`	`64`	`}`
`62`	`65`
`63`	`66`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -31,15 +31,13 @@ public function testAwareAction(): void`
`31`	`31`	`{`
`32`	`32`	`$this->getRequest()->setMethod('GET');`
`33`	`33`	`$this->dispatch('csputil/csp/aware');`
`34`		`- $headers = '';`
`35`		`- foreach ($this->getResponse()->getHeaders() as $header) {`
`36`		`- $headers .= $header->getFieldName() .': ' .$header->getFieldValue() .PHP_EOL;`
`37`		`- }`
	`34`	`+ $header = $this->getResponse()->getHeader('Content-Security-Policy');`
	`35`	`+ $this->assertNotEmpty($header);`
`38`	`36`
`39`	`37`	`$this->assertContains(`
`40`		`- 'Content-Security-Policy: script-src https://controller.magento.com'`
	`38`	`+ 'script-src https://controller.magento.com'`
`41`	`39`	`.' \'self\' \'sha256-H4RRnauTM2X2Xg/z9zkno1crqhsaY3uKKu97uwmnXXE=\'',`
`42`		`- $headers`
	`40`	`+ $header->getFieldValue()`
`43`	`41`	`);`
`44`	`42`	`}`
`45`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@ class CspUtilTest extends AbstractController`
`20`	`20`	`* Test that CSP helper for templates works.`
`21`	`21`	`*`
`22`	`22`	`* @return void`
	`23`	`+ * @magentoConfigFixture default_store csp/mode/storefront/report_only 0`
`23`	`24`	`*/`
`24`	`25`	`public function testPhtmlHelper(): void`
`25`	`26`	`{`
`@@ -29,11 +30,9 @@ public function testPhtmlHelper(): void`
`29`	`30`
`30`	`31`	`$this->assertContains('<script src="http://my.magento.com/static/script.js" />', $content);`
`31`	`32`	`$this->assertContains("<script>\n let myVar = 1;\n</script>", $content);`
`32`		`- $headers = '';`
`33`		`- foreach ($this->getResponse()->getHeaders() as $header) {`
`34`		`- $headers .= $header->getFieldName() .': ' .$header->getFieldValue() .PHP_EOL;`
`35`		`- }`
`36`		`- $this->assertContains('http://my.magento.com', $headers);`
`37`		`- $this->assertContains('\'sha256-H4RRnauTM2X2Xg/z9zkno1crqhsaY3uKKu97uwmnXXE=\'', $headers);`
	`33`	`+ $header = $this->getResponse()->getHeader('Content-Security-Policy');`
	`34`	`+ $this->assertNotEmpty($header);`
	`35`	`+ $this->assertContains('http://my.magento.com', $header->getFieldValue());`
	`36`	`+ $this->assertContains('\'sha256-H4RRnauTM2X2Xg/z9zkno1crqhsaY3uKKu97uwmnXXE=\'', $header->getFieldValue());`
`38`	`37`	`}`
`39`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,9 @@ public function getHeader($name)`
`28`	`28`	`$headers = $this->getHeaders();`
`29`	`29`	`if ($headers->has($name)) {`
`30`	`30`	`$header = $headers->get($name);`
	`31`	`+ if (is_iterable($header)) {`
	`32`	`+ $header = $header[0];`
	`33`	`+ }`
`31`	`34`	`}`
`32`	`35`	`return $header;`
`33`	`36`	`}`
`@@ -94,8 +97,13 @@ public function clearHeader($name)`
`94`	`97`	`{`
`95`	`98`	`$headers = $this->getHeaders();`
`96`	`99`	`if ($headers->has($name)) {`
`97`		`- $header = $headers->get($name);`
`98`		`- $headers->removeHeader($header);`
	`100`	`+ $headerValues = $headers->get($name);`
	`101`	`+ if (!is_iterable($headerValues)) {`
	`102`	`+ $headerValues = [$headerValues];`
	`103`	`+ }`
	`104`	`+ foreach ($headerValues as $headerValue) {`
	`105`	`+ $headers->removeHeader($headerValue);`
	`106`	`+ }`
`99`	`107`	`}`
`100`	`108`
`101`	`109`	`return $this;`
`@@ -203,7 +211,6 @@ public function sendHeaders()`
`203`	`211`	`header($header->toString(), false);`
`204`	`212`	`}`
`205`	`213`
`206`		`- $this->headersSent = true;`
`207`	`214`	`return $this;`
`208`	`215`	`}`
`209`	`216`	`}`