diff --git a/lib/private/appframework/app.php b/lib/private/appframework/app.php index 537f10255a..6d54b931d5 100644 --- a/lib/private/appframework/app.php +++ b/lib/private/appframework/app.php @@ -24,9 +24,10 @@ namespace OC\AppFramework; -use \OC_App; -use \OC\AppFramework\DependencyInjection\DIContainer; -use \OCP\AppFramework\QueryException; +use OC_App; +use OC\AppFramework\DependencyInjection\DIContainer; +use OCP\AppFramework\QueryException; +use OCP\AppFramework\Http\ICallbackResponse; /** * Entry point for every request in your app. You can consider this as your @@ -93,15 +94,22 @@ class App { // initialize the dispatcher and run all the middleware before the controller $dispatcher = $container['Dispatcher']; - list($httpHeaders, $responseHeaders, $responseCookies, $output) = - $dispatcher->dispatch($controller, $methodName); + list( + $httpHeaders, + $responseHeaders, + $responseCookies, + $output, + $response + ) = $dispatcher->dispatch($controller, $methodName); + + $io = $container['OCP\\AppFramework\\Http\\IOutput']; if(!is_null($httpHeaders)) { - header($httpHeaders); + $io->setHeader($httpHeaders); } foreach($responseHeaders as $name => $value) { - header($name . ': ' . $value); + $io->setHeader($name . ': ' . $value); } foreach($responseCookies as $name => $value) { @@ -109,12 +117,22 @@ class App { if($value['expireDate'] instanceof \DateTime) { $expireDate = $value['expireDate']->getTimestamp(); } - setcookie($name, $value['value'], $expireDate, $container->getServer()->getWebRoot(), null, $container->getServer()->getConfig()->getSystemValue('forcessl', false), true); + $io->setCookie( + $name, + $value['value'], + $expireDate, + $container->getServer()->getWebRoot(), + null, + $container->getServer()->getConfig()->getSystemValue('forcessl', false), + true + ); } - if(!is_null($output)) { - header('Content-Length: ' . strlen($output)); - print($output); + if ($response instanceof ICallbackResponse) { + $response->callback($io); + } else if(!is_null($output)) { + $io->setHeader('Content-Length: ' . strlen($output)); + $io->setOutput($output); } } diff --git a/lib/private/appframework/dependencyinjection/dicontainer.php b/lib/private/appframework/dependencyinjection/dicontainer.php index 4229b251e2..e88177c639 100644 --- a/lib/private/appframework/dependencyinjection/dicontainer.php +++ b/lib/private/appframework/dependencyinjection/dicontainer.php @@ -28,6 +28,7 @@ use OC; use OC\AppFramework\Http; use OC\AppFramework\Http\Request; use OC\AppFramework\Http\Dispatcher; +use OC\AppFramework\Http\Output; use OC\AppFramework\Core\API; use OC\AppFramework\Middleware\MiddlewareDispatcher; use OC\AppFramework\Middleware\Security\SecurityMiddleware; @@ -69,6 +70,10 @@ class DIContainer extends SimpleContainer implements IAppContainer { return $this->getServer()->getAppManager(); }); + $this->registerService('OCP\\AppFramework\\Http\\IOutput', function($c){ + return new Output(); + }); + $this->registerService('OCP\\IAvatarManager', function($c) { return $this->getServer()->getAvatarManager(); }); diff --git a/lib/private/appframework/http/dispatcher.php b/lib/private/appframework/http/dispatcher.php index 24540ef3c9..910e9c32ed 100644 --- a/lib/private/appframework/http/dispatcher.php +++ b/lib/private/appframework/http/dispatcher.php @@ -100,17 +100,15 @@ class Dispatcher { $response = $this->middlewareDispatcher->afterController( $controller, $methodName, $response); - // get the output which should be printed and run the after output - // middleware to modify the response - $output = $response->render(); - $out[3] = $this->middlewareDispatcher->beforeOutput( - $controller, $methodName, $output); - // depending on the cache object the headers need to be changed $out[0] = $this->protocol->getStatusHeader($response->getStatus(), $response->getLastModified(), $response->getETag()); $out[1] = array_merge($response->getHeaders()); $out[2] = $response->getCookies(); + $out[3] = $this->middlewareDispatcher->beforeOutput( + $controller, $methodName, $response->render() + ); + $out[4] = $response; return $out; } diff --git a/lib/private/appframework/http/output.php b/lib/private/appframework/http/output.php new file mode 100644 index 0000000000..808f1ec6df --- /dev/null +++ b/lib/private/appframework/http/output.php @@ -0,0 +1,70 @@ + + * + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\AppFramework\Http; + +use OCP\AppFramework\Http\IOutput; + +/** + * Very thin wrapper class to make output testable + */ +class Output implements IOutput { + + /** + * @param string $out + */ + public function setOutput($out) { + print($out); + } + + /** + * @param string $path + * + * @return bool false if an error occured + */ + public function setReadfile($path) { + return @readfile($path); + } + + /** + * @param string $header + */ + public function setHeader($header) { + header($header); + } + + /** + * @param int $code sets the http status code + */ + public function setHttpResponseCode($code) { + http_response_code($code); + } + + /** + * @return int returns the current http response code + */ + public function getHttpResponseCode() { + return http_response_code(); + } + + /** + * @param string $name + * @param string $value + * @param int $expire + * @param string $path + * @param string $domain + * @param bool $secure + * @param bool $httponly + */ + public function setCookie($name, $value, $expire, $path, $domain, $secure, $httponly) { + setcookie($name, $value, $expire, $path, $domain, $secure, $httponly); + } + +} diff --git a/lib/public/appframework/http/icallbackresponse.php b/lib/public/appframework/http/icallbackresponse.php new file mode 100644 index 0000000000..4a392ed081 --- /dev/null +++ b/lib/public/appframework/http/icallbackresponse.php @@ -0,0 +1,28 @@ + + * + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCP\AppFramework\Http; + + +/** + * Interface ICallbackResponse + * + * @package OCP\AppFramework\Http + */ +interface ICallbackResponse { + + /** + * Outputs the content that should be printed + * + * @param IOutput a small wrapper that handles output + */ + function callback(IOutput $output); + +} diff --git a/lib/public/appframework/http/ioutput.php b/lib/public/appframework/http/ioutput.php new file mode 100644 index 0000000000..191f84374d --- /dev/null +++ b/lib/public/appframework/http/ioutput.php @@ -0,0 +1,57 @@ + + * + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCP\AppFramework\Http; + + +/** + * Very thin wrapper class to make output testable + */ +interface IOutput { + + /** + * @param string $out + */ + public function setOutput($out); + + /** + * @param string $path + * + * @return bool false if an error occured + */ + public function setReadfile($path); + + /** + * @param string $header + */ + public function setHeader($header); + + /** + * @return int returns the current http response code + */ + public function getHttpResponseCode(); + + /** + * @param int $code sets the http status code + */ + public function setHttpResponseCode($code); + + /** + * @param string $name + * @param string $value + * @param int $expire + * @param string $path + * @param string $domain + * @param bool $secure + * @param bool $httponly + */ + public function setCookie($name, $value, $expire, $path, $domain, $secure, $httponly); + +} diff --git a/lib/public/appframework/http/streamresponse.php b/lib/public/appframework/http/streamresponse.php new file mode 100644 index 0000000000..870eb95cc1 --- /dev/null +++ b/lib/public/appframework/http/streamresponse.php @@ -0,0 +1,48 @@ + + * + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCP\AppFramework\Http; + +use OCP\AppFramework\Http; + +/** + * Class StreamResponse + * + * @package OCP\AppFramework\Http + */ +class StreamResponse extends Response implements ICallbackResponse { + /** @var string */ + private $filePath; + + /** + * @param string $filePath the path to the file which should be streamed + */ + public function __construct ($filePath) { + $this->filePath = $filePath; + } + + + /** + * Streams the file using readfile + * + * @param IOutput a small wrapper that handles output + */ + public function callback (IOutput $output) { + // handle caching + if ($output->getHttpResponseCode() !== Http::STATUS_NOT_MODIFIED) { + if (!file_exists($this->filePath)) { + $output->setHttpResponseCode(Http::STATUS_NOT_FOUND); + } elseif ($output->setReadfile($this->filePath) === false) { + $output->setHttpResponseCode(Http::STATUS_BAD_REQUEST); + } + } + } + +} diff --git a/tests/lib/appframework/AppTest.php b/tests/lib/appframework/AppTest.php index e60f3439f2..05190ca09b 100644 --- a/tests/lib/appframework/AppTest.php +++ b/tests/lib/appframework/AppTest.php @@ -24,6 +24,9 @@ namespace OC\AppFramework; +use OCP\AppFramework\Http\Response; + + function rrmdir($directory) { $files = array_diff(scandir($directory), array('.','..')); foreach ($files as $file) { @@ -36,9 +39,11 @@ function rrmdir($directory) { return rmdir($directory); } + class AppTest extends \Test\TestCase { private $container; + private $io; private $api; private $controller; private $dispatcher; @@ -62,6 +67,7 @@ class AppTest extends \Test\TestCase { ->disableOriginalConstructor() ->getMock(); + $this->io = $this->getMockBuilder('OCP\\AppFramework\\Http\\IOutput')->getMock(); $this->headers = array('key' => 'value'); $this->output = 'hi'; @@ -70,6 +76,7 @@ class AppTest extends \Test\TestCase { $this->container[$this->controllerName] = $this->controller; $this->container['Dispatcher'] = $this->dispatcher; + $this->container['OCP\\AppFramework\\Http\\IOutput'] = $this->io; $this->container['urlParams'] = array(); $this->appPath = __DIR__ . '/../../../apps/namespacetestapp/appinfo'; @@ -86,14 +93,15 @@ class AppTest extends \Test\TestCase { public function testControllerNameAndMethodAreBeingPassed(){ - $return = array(null, array(), array(), null); + $return = array(null, array(), array(), null, new Response()); $this->dispatcher->expects($this->once()) ->method('dispatch') ->with($this->equalTo($this->controller), $this->equalTo($this->controllerMethod)) ->will($this->returnValue($return)); - $this->expectOutputString(''); + $this->io->expects($this->never()) + ->method('setOutput'); App::main($this->controllerName, $this->controllerMethod, $this->container); @@ -122,26 +130,34 @@ class AppTest extends \Test\TestCase { rrmdir($this->appPath); } - /* - FIXME: this complains about shit headers which are already sent because - of the content length. Would be cool if someone could fix this public function testOutputIsPrinted(){ - $return = array(null, array(), $this->output); + $return = [null, [], [], $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->expectOutputString($this->output); - - App::main($this->controllerName, $this->controllerMethod, array(), - $this->container); + $this->io->expects($this->once()) + ->method('setOutput') + ->with($this->equalTo($this->output)); + App::main($this->controllerName, $this->controllerMethod, $this->container, []); } - */ - // FIXME: if someone manages to test the headers output, I'd be grateful + public function testCallbackIsCalled(){ + $mock = $this->getMockBuilder('OCP\AppFramework\Http\ICallbackResponse') + ->getMock(); + + $return = [null, [], [], $this->output, $mock]; + $this->dispatcher->expects($this->once()) + ->method('dispatch') + ->with($this->equalTo($this->controller), + $this->equalTo($this->controllerMethod)) + ->will($this->returnValue($return)); + $mock->expects($this->once()) + ->method('callback'); + App::main($this->controllerName, $this->controllerMethod, $this->container, []); + } } diff --git a/tests/lib/appframework/http/StreamResponseTest.php b/tests/lib/appframework/http/StreamResponseTest.php new file mode 100644 index 0000000000..4c47ecfbd6 --- /dev/null +++ b/tests/lib/appframework/http/StreamResponseTest.php @@ -0,0 +1,99 @@ + + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE + * License as published by the Free Software Foundation; either + * version 3 of the License, or any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU Affero General Public + * License along with this library. If not, see . + * + */ + + +namespace OC\AppFramework\Http; + + +use OCP\AppFramework\Http\StreamResponse; +use OCP\AppFramework\Http; + + +class StreamResponseTest extends \Test\TestCase { + + /** @var IOutput */ + private $output; + + protected function setUp() { + parent::setUp(); + $this->output = $this->getMock('OCP\\AppFramework\\Http\\IOutput'); + } + + public function testOutputNotModified(){ + $path = __FILE__; + $this->output->expects($this->once()) + ->method('getHttpResponseCode') + ->will($this->returnValue(Http::STATUS_NOT_MODIFIED)); + $this->output->expects($this->never()) + ->method('setReadfile'); + $response = new StreamResponse($path); + + $response->callback($this->output); + } + + public function testOutputOk(){ + $path = __FILE__; + $this->output->expects($this->once()) + ->method('getHttpResponseCode') + ->will($this->returnValue(Http::STATUS_OK)); + $this->output->expects($this->once()) + ->method('setReadfile') + ->with($this->equalTo($path)) + ->will($this->returnValue(true)); + $response = new StreamResponse($path); + + $response->callback($this->output); + } + + public function testOutputNotFound(){ + $path = __FILE__ . 'test'; + $this->output->expects($this->once()) + ->method('getHttpResponseCode') + ->will($this->returnValue(Http::STATUS_OK)); + $this->output->expects($this->never()) + ->method('setReadfile'); + $this->output->expects($this->once()) + ->method('setHttpResponseCode') + ->with($this->equalTo(Http::STATUS_NOT_FOUND)); + $response = new StreamResponse($path); + + $response->callback($this->output); + } + + public function testOutputReadFileError(){ + $path = __FILE__; + $this->output->expects($this->once()) + ->method('getHttpResponseCode') + ->will($this->returnValue(Http::STATUS_OK)); + $this->output->expects($this->once()) + ->method('setReadfile') + ->will($this->returnValue(false)); + $this->output->expects($this->once()) + ->method('setHttpResponseCode') + ->with($this->equalTo(Http::STATUS_BAD_REQUEST)); + $response = new StreamResponse($path); + + $response->callback($this->output); + } + +}