From 401a2692e8ca632ddbca186b0924f5d1bc5207f7 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 ad12e754e795b..97acedc562d31 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -48,6 +48,7 @@ use OCA\Theming\Util; use OCP\ITempManager; use OCP\IURLGenerator; +use OCP\App\IAppManager; /** * Class ThemingController @@ -75,6 +76,8 @@ class ThemingController extends Controller { private $scssCacher; /** @var IURLGenerator */ private $urlGenerator; + /** @var IAppManager */ + private $appManager; /** * ThemingController constructor. @@ -90,6 +93,7 @@ class ThemingController extends Controller { * @param IAppData $appData * @param SCSSCacher $scssCacher * @param IURLGenerator $urlGenerator + * @param IAppManager $appManager */ public function __construct( $appName, @@ -102,7 +106,8 @@ public function __construct( ITempManager $tempManager, IAppData $appData, SCSSCacher $scssCacher, - IURLGenerator $urlGenerator + IURLGenerator $urlGenerator, + IAppManager $appManager ) { parent::__construct($appName, $request); @@ -115,6 +120,7 @@ public function __construct( $this->appData = $appData; $this->scssCacher = $scssCacher; $this->urlGenerator = $urlGenerator; + $this->appManager = $appManager; } /** @@ -374,12 +380,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 5e6e43ca3cbeb..0cff6f398217b 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -100,7 +100,8 @@ public function setUp() { $this->tempManager, $this->appData, $this->scssCacher, - $this->urlGenerator + $this->urlGenerator, + $this->appManager ); return parent::setUp(); @@ -629,7 +630,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'); @@ -649,6 +650,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'); @@ -660,6 +662,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))