From 2e567918c14bc831a61f2aae3b8d4f321617eb75 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 24 May 2019 13:00:17 +0200 Subject: [PATCH] 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 Signed-off-by: Morris Jobke --- lib/private/AppFramework/App.php | 10 +++++++++- tests/lib/AppFramework/AppTest.php | 19 +++++++++---------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/lib/private/AppFramework/App.php b/lib/private/AppFramework/App.php index 5a9fb0c64fc..6185a35d1d7 100644 --- a/lib/private/AppFramework/App.php +++ b/lib/private/AppFramework/App.php @@ -157,7 +157,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 b31f4428777..3917cea68dd 100644 --- 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),