Check the actual status code for 204 and 304

The header is the full http header like: HTTP/1.1 304 Not Modified
So comparing this to an int always yields false
This also makes the 304 RFC compliant as the resulting content length
should otherwise be the length of the message and not 0.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Morris Jobke <hey@morrisjobke.de>

diff --git a/lib/private/AppFramework/App.php b/lib/private/AppFramework/App.php
index abb779ca979af58510c9d024b7c77c7cb3c2ff3d..a3f216ce91123809fd32f01e8f8991a68b81f1ac 100644 (file)
--- a/lib/private/AppFramework/App.php
+++ b/lib/private/AppFramework/App.php
@@ -149,7 +149,15 @@ class App {
                 * https://tools.ietf.org/html/rfc7230#section-3.3
                 * https://tools.ietf.org/html/rfc7230#section-3.3.2
                 */
-               if ($httpHeaders !== Http::STATUS_NO_CONTENT && $httpHeaders !== Http::STATUS_NOT_MODIFIED) {
+               $emptyResponse = false;
+               if (preg_match('/^HTTP\/\d\.\d (\d{3}) .*$/', $httpHeaders, $matches)) {
+                       $status = (int)$matches[1];
+                       if ($status === Http::STATUS_NO_CONTENT || $status === Http::STATUS_NOT_MODIFIED) {
+                               $emptyResponse = true;
+                       }
+               }
+
+               if (!$emptyResponse) {
                        if ($response instanceof ICallbackResponse) {
                                $response->callback($io);
                        } else if (!is_null($output)) {

diff --git a/tests/lib/AppFramework/AppTest.php b/tests/lib/AppFramework/AppTest.php
index b31f442877785fa9b747c9ba8d1463f7ae04addf..3917cea68dd481e8c408a6c13c8b516445e4f64f 100644 (file)
--- a/tests/lib/AppFramework/AppTest.php
+++ b/tests/lib/AppFramework/AppTest.php
@@ -90,7 +90,7 @@ class AppTest extends \Test\TestCase {
 
 
        public function testControllerNameAndMethodAreBeingPassed(){
-               $return = array(null, array(), array(), null, new Response());
+               $return = ['HTTP/2.0 200 OK', [], [], null, new Response()];
                $this->dispatcher->expects($this->once())
                        ->method('dispatch')
                        ->with($this->equalTo($this->controller),
@@ -130,7 +130,7 @@ class AppTest extends \Test\TestCase {
 
 
        public function testOutputIsPrinted(){
-               $return = [Http::STATUS_OK, [], [], $this->output, new Response()];
+               $return = ['HTTP/2.0 200 OK', [], [], $this->output, new Response()];
                $this->dispatcher->expects($this->once())
                        ->method('dispatch')
                        ->with($this->equalTo($this->controller),
@@ -144,16 +144,15 @@ class AppTest extends \Test\TestCase {
 
        public function dataNoOutput() {
                return [
-                       [Http::STATUS_NO_CONTENT],
-                       [Http::STATUS_NOT_MODIFIED],
+                       ['HTTP/2.0 204 No content'],
+                       ['HTTP/2.0 304 Not modified'],
                ];
        }
 
        /**
         * @dataProvider dataNoOutput
-        * @param int $statusCode
         */
-       public function testNoOutput($statusCode) {
+       public function testNoOutput(string $statusCode) {
                $return = [$statusCode, [], [], $this->output, new Response()];
                $this->dispatcher->expects($this->once())
                        ->method('dispatch')
@@ -173,7 +172,7 @@ class AppTest extends \Test\TestCase {
                $mock = $this->getMockBuilder('OCP\AppFramework\Http\ICallbackResponse')
                        ->getMock();
 
-               $return = [null, [], [], $this->output, $mock];
+               $return = ['HTTP/2.0 200 OK', [], [], $this->output, $mock];
                $this->dispatcher->expects($this->once())
                        ->method('dispatch')
                        ->with($this->equalTo($this->controller),
@@ -188,7 +187,7 @@ class AppTest extends \Test\TestCase {
                $this->container['AppName'] = 'core';
                $this->container['OC\Core\Controller\Foo'] = $this->controller;
 
-               $return = array(null, array(), array(), null, new Response());
+               $return = ['HTTP/2.0 200 OK', [], [], null, new Response()];
                $this->dispatcher->expects($this->once())
                        ->method('dispatch')
                        ->with($this->equalTo($this->controller),
@@ -205,7 +204,7 @@ class AppTest extends \Test\TestCase {
                $this->container['AppName'] = 'settings';
                $this->container['OC\Settings\Controller\Foo'] = $this->controller;
 
-               $return = array(null, array(), array(), null, new Response());
+               $return = ['HTTP/2.0 200 OK', [], [], null, new Response()];
                $this->dispatcher->expects($this->once())
                        ->method('dispatch')
                        ->with($this->equalTo($this->controller),
@@ -222,7 +221,7 @@ class AppTest extends \Test\TestCase {
                $this->container['AppName'] = 'bar';
                $this->container['OCA\Bar\Controller\Foo'] = $this->controller;
 
-               $return = array(null, array(), array(), null, new Response());
+               $return = ['HTTP/2.0 200 OK', [], [], null, new Response()];
                $this->dispatcher->expects($this->once())
                        ->method('dispatch')
                        ->with($this->equalTo($this->controller),