Changed the strlen to mb_strl with 8bit encoding to ensure that the content length is calculated in bytes in all PHP installations. Added test cases to cover non-ascii content (multi-byte characters, null byte, LTR text).

mattijv · mattijv · commit 22b9e56dbb42 · 2019-03-07T13:16:59.000+02:00
diff --git a/lib/internal/Magento/Framework/App/Http.php b/lib/internal/Magento/Framework/App/Http.php
@@ -157,7 +157,11 @@ public function launch()
      */
     private function handleHeadRequest()
     {
-        $contentLength = strlen($this->_response->getContent());
+        // It is possible that some PHP installations have overloaded strlen to use mb_strlen instead.
+        // This means strlen might return the actual number of characters in a non-ascii string instead
+        // of the number of bytes. Use mb_strlen explicitly with a single byte character encoding to ensure
+        // that the content length is calculated in bytes.
+        $contentLength = mb_strlen($this->_response->getContent(), '8bit');
         $this->_response->clearBody();
         $this->_response->setHeader('Content-Length', $contentLength);
     }
diff --git a/lib/internal/Magento/Framework/App/Test/Unit/HttpTest.php b/lib/internal/Magento/Framework/App/Test/Unit/HttpTest.php
@@ -182,10 +182,15 @@ function () {
         $this->http->launch();
     }
 
-    public function testLaunchHeadRequest()
+
+    /**
+     * Test that HEAD requests lead to an empty body and a Content-Length header matching the original body size.
+     * @dataProvider dataProviderForTestLaunchHeadRequest
+     * @param string $body
+     * @param int $expectedLength
+     */
+    public function testLaunchHeadRequest($body, $expectedLength)
     {
-        $body = "<html><head></head><body>Test</body></html>";
-        $contentLength = strlen($body);
         $this->setUpLaunch();
         $this->requestMock->expects($this->once())->method('isHead')->will($this->returnValue(true));
         $this->responseMock->expects($this->once())
@@ -199,7 +204,7 @@ public function testLaunchHeadRequest()
             ->will($this->returnValue($this->responseMock));
         $this->responseMock->expects($this->once())
             ->method('setHeader')
-            ->with('Content-Length', $contentLength)
+            ->with('Content-Length', $expectedLength)
             ->will($this->returnValue($this->responseMock));
         $this->eventManagerMock->expects($this->once())
             ->method('dispatch')
@@ -210,6 +215,32 @@ public function testLaunchHeadRequest()
         $this->assertSame($this->responseMock, $this->http->launch());
     }
 
+    /**
+     * Different test content for responseMock with their expected lengths in bytes.
+     * @return array
+     */
+    public function dataProviderForTestLaunchHeadRequest()
+    {
+        return [
+            [
+                '<html><head></head><body>Test</body></html>',                // Ascii text
+                43                                                            // Expected Content-Length
+            ],
+            [
+                '<html><head></head><body>部落格</body></html>',               // Multi-byte characters
+                48                                                            // Expected Content-Length
+            ],
+            [
+                '<html><head></head><body>'.chr(0).'</body></html>',     // Null byte
+                40                                                            // Expected Content-Length
+            ],
+            [
+                '<html><head></head>خرید<body></body></html>',                // LTR text
+                47                                                            // Expected Content-Length
+            ]
+        ];
+    }
+
     public function testHandleDeveloperModeNotInstalled()
     {
         $dir = $this->getMockForAbstractClass(\Magento\Framework\Filesystem\Directory\ReadInterface::class);

Original file line number	Diff line number	Diff line change
`@@ -157,7 +157,11 @@ public function launch()`
`157`	`157`	`*/`
`158`	`158`	`private function handleHeadRequest()`
`159`	`159`	`{`
`160`		`- $contentLength = strlen($this->_response->getContent());`
	`160`	`+ // It is possible that some PHP installations have overloaded strlen to use mb_strlen instead.`
	`161`	`+ // This means strlen might return the actual number of characters in a non-ascii string instead`
	`162`	`+ // of the number of bytes. Use mb_strlen explicitly with a single byte character encoding to ensure`
	`163`	`+ // that the content length is calculated in bytes.`
	`164`	`+ $contentLength = mb_strlen($this->_response->getContent(), '8bit');`
`161`	`165`	`$this->_response->clearBody();`
`162`	`166`	`$this->_response->setHeader('Content-Length', $contentLength);`
`163`	`167`	`}`