From 4ecf31541b13167e95a92da1bb632875577cd9ca Mon Sep 17 00:00:00 2001 From: Kyle Fazzari Date: Tue, 20 Feb 2018 22:44:37 -0800 Subject: [PATCH] theming: handle not being in the serverroot Currently, the theming app assumes it's in the serverroot. However, with Nextcloud's flexibility regarding configurable app paths, this is not a safe assumption to make. If it happens to be an incorrect assumption, the theming app fails to work. Instead of relying on the serverroot, just use the path from the AppManager and utilize relative paths for assets from there. Fix #8462 Signed-off-by: Kyle Fazzari --- .../lib/Controller/ThemingController.php | 13 +++++++--- .../Controller/ThemingControllerTest.php | 26 +++++++++++++++++-- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 6592eb7f7ab91..3ac8b7fe587be 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -53,6 +53,7 @@ use OCA\Theming\Util; use OCP\ITempManager; use OCP\IURLGenerator; +use OCP\App\IAppManager; /** * Class ThemingController @@ -80,6 +81,8 @@ class ThemingController extends Controller { private $scssCacher; /** @var IURLGenerator */ private $urlGenerator; + /** @var IAppManager */ + private $appManager; /** * ThemingController constructor. @@ -95,6 +98,7 @@ class ThemingController extends Controller { * @param IAppData $appData * @param SCSSCacher $scssCacher * @param IURLGenerator $urlGenerator + * @param IAppManager $appManager */ public function __construct( $appName, @@ -107,7 +111,8 @@ public function __construct( ITempManager $tempManager, IAppData $appData, SCSSCacher $scssCacher, - IURLGenerator $urlGenerator + IURLGenerator $urlGenerator, + IAppManager $appManager ) { parent::__construct($appName, $request); @@ -120,6 +125,7 @@ public function __construct( $this->appData = $appData; $this->scssCacher = $scssCacher; $this->urlGenerator = $urlGenerator; + $this->appManager = $appManager; } /** @@ -413,12 +419,13 @@ public function getLoginBackground() { * @return FileDisplayResponse|NotFoundResponse */ public function getStylesheet() { - $appPath = substr(\OC::$server->getAppManager()->getAppPath('theming'), strlen(\OC::$SERVERROOT) + 1); + $appPath = $this->appManager->getAppPath('theming'); + /* SCSSCacher is required here * We cannot rely on automatic caching done by \OC_Util::addStyle, * since we need to add the cacheBuster value to the url */ - $cssCached = $this->scssCacher->process(\OC::$SERVERROOT, $appPath . '/css/theming.scss', 'theming'); + $cssCached = $this->scssCacher->process($appPath, 'css/theming.scss', 'theming'); if(!$cssCached) { return new NotFoundResponse(); } diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 54b842bc4e88c..debc1b71e4740 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -106,7 +106,8 @@ public function setUp() { $this->tempManager, $this->appData, $this->scssCacher, - $this->urlGenerator + $this->urlGenerator, + $this->appManager ); return parent::setUp(); @@ -798,7 +799,7 @@ public function testGetLoginBackground() { public function testGetStylesheet() { - + $this->appManager->expects($this->once())->method('getAppPath')->with('theming')->willReturn(\OC::$SERVERROOT . '/theming'); $file = $this->createMock(ISimpleFile::class); $file->expects($this->any())->method('getName')->willReturn('theming.css'); $file->expects($this->any())->method('getContent')->willReturn('compiled'); @@ -818,6 +819,7 @@ public function testGetStylesheet() { } public function testGetStylesheetFails() { + $this->appManager->expects($this->once())->method('getAppPath')->with('theming')->willReturn(\OC::$SERVERROOT . '/theming'); $file = $this->createMock(ISimpleFile::class); $file->expects($this->any())->method('getName')->willReturn('theming.css'); $file->expects($this->any())->method('getContent')->willReturn('compiled'); @@ -829,6 +831,26 @@ public function testGetStylesheetFails() { $this->assertEquals($response, $actual); } + public function testGetStylesheetOutsideServerroot() { + $this->appManager->expects($this->once())->method('getAppPath')->with('theming')->willReturn('/outside/serverroot/theming'); + $file = $this->createMock(ISimpleFile::class); + $file->expects($this->any())->method('getName')->willReturn('theming.css'); + $file->expects($this->any())->method('getContent')->willReturn('compiled'); + $this->scssCacher->expects($this->once())->method('process')->with('/outside/serverroot/theming', 'css/theming.scss', 'theming')->willReturn(true); + $this->scssCacher->expects($this->once())->method('getCachedCSS')->willReturn($file); + + $response = new Http\FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'text/css']); + $response->cacheFor(86400); + $expires = new \DateTime(); + $expires->setTimestamp($this->timeFactory->getTime()); + $expires->add(new \DateInterval('PT24H')); + $response->addHeader('Expires', $expires->format(\DateTime::RFC1123)); + $response->addHeader('Pragma', 'cache'); + + $actual = $this->themingController->getStylesheet(); + $this->assertEquals($response, $actual); + } + public function testGetJavascript() { $this->themingDefaults ->expects($this->at(0))