Merge pull request #1221 from nextcloud/proper_204_304_response

No body or content-length for 204 and 304 responses

lib/private/AppFramework/App.php

@@ -30,6 +30,7 @@ namespace OC\AppFramework;
 use OC\AppFramework\Http\Dispatcher;
 use OC_App;
 use OC\AppFramework\DependencyInjection\DIContainer;
+use OCP\AppFramework\Http;
 use OCP\AppFramework\QueryException;
 use OCP\AppFramework\Http\ICallbackResponse;
 
@@ -142,11 +143,19 @@ class App {
 			);
 		}
 
-		if ($response instanceof ICallbackResponse) {
-			$response->callback($io);
-		} else if(!is_null($output)) {
-			$io->setHeader('Content-Length: ' . strlen($output));
-			$io->setOutput($output);
+		/*
+		 * Status 204 does not have a body and no Content Length
+		 * Status 304 does not have a body and does not need a Content Length
+		 * 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) {
+			if ($response instanceof ICallbackResponse) {
+				$response->callback($io);
+			} else if (!is_null($output)) {
+				$io->setHeader('Content-Length: ' . strlen($output));
+				$io->setOutput($output);
+			}
 		}
 
 	}

tests/lib/AppFramework/AppTest.php

@@ -25,6 +25,7 @@
 namespace Test\AppFramework;
 
 use OC\AppFramework\App;
+use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Response;
 
 
@@ -134,7 +135,7 @@ class AppTest extends \Test\TestCase {
 
 
 	public function testOutputIsPrinted(){
-		$return = [null, [], [], $this->output, new Response()];
+		$return = [Http::STATUS_OK, [], [], $this->output, new Response()];
 		$this->dispatcher->expects($this->once())
 			->method('dispatch')
 			->with($this->equalTo($this->controller),
@@ -146,6 +147,32 @@ class AppTest extends \Test\TestCase {
 		App::main($this->controllerName, $this->controllerMethod, $this->container, []);
 	}
 
+	public function dataNoOutput() {
+		return [
+			[Http::STATUS_NO_CONTENT],
+			[Http::STATUS_NOT_MODIFIED],
+		];
+	}
+
+	/**
+	 * @dataProvider dataNoOutput
+	 * @param int $statusCode
+	 */
+	public function testNoOutput($statusCode) {
+		$return = [$statusCode, [], [], $this->output, new Response()];
+		$this->dispatcher->expects($this->once())
+			->method('dispatch')
+			->with($this->equalTo($this->controller),
+				$this->equalTo($this->controllerMethod))
+			->will($this->returnValue($return));
+		$this->io->expects($this->once())
+			->method('setHeader')
+			->with($this->equalTo($statusCode));
+		$this->io->expects($this->never())
+			->method('setOutput');
+		App::main($this->controllerName, $this->controllerMethod, $this->container, []);
+	}
+
 
 	public function testCallbackIsCalled(){
 		$mock = $this->getMockBuilder('OCP\AppFramework\Http\ICallbackResponse')