From b0c030cbb58dca4b2d5ba12a98d073587ec6ad3a 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 5a9fb0c64f..6185a35d1d 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 b31f442877..3917cea68d 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),