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 <kyrofa@ubuntu.com>
This commit is contained in:
parent
6591a3bc36
commit
a1f1824116
2 changed files with 49 additions and 3 deletions
|
@ -51,6 +51,7 @@ use OCP\IRequest;
|
|||
use OCA\Theming\Util;
|
||||
use OCP\ITempManager;
|
||||
use OCP\IURLGenerator;
|
||||
use OCP\App\IAppManager;
|
||||
|
||||
/**
|
||||
* Class ThemingController
|
||||
|
@ -78,6 +79,8 @@ class ThemingController extends Controller {
|
|||
private $scssCacher;
|
||||
/** @var IURLGenerator */
|
||||
private $urlGenerator;
|
||||
/** @var IAppManager */
|
||||
private $appManager;
|
||||
|
||||
/**
|
||||
* ThemingController constructor.
|
||||
|
@ -93,6 +96,7 @@ class ThemingController extends Controller {
|
|||
* @param IAppData $appData
|
||||
* @param SCSSCacher $scssCacher
|
||||
* @param IURLGenerator $urlGenerator
|
||||
* @param IAppManager $appManager
|
||||
*/
|
||||
public function __construct(
|
||||
$appName,
|
||||
|
@ -105,7 +109,8 @@ class ThemingController extends Controller {
|
|||
ITempManager $tempManager,
|
||||
IAppData $appData,
|
||||
SCSSCacher $scssCacher,
|
||||
IURLGenerator $urlGenerator
|
||||
IURLGenerator $urlGenerator,
|
||||
IAppManager $appManager = NULL
|
||||
) {
|
||||
parent::__construct($appName, $request);
|
||||
|
||||
|
@ -118,6 +123,12 @@ class ThemingController extends Controller {
|
|||
$this->appData = $appData;
|
||||
$this->scssCacher = $scssCacher;
|
||||
$this->urlGenerator = $urlGenerator;
|
||||
|
||||
if (!is_null($appManager)) {
|
||||
$this->appManager = $appManager;
|
||||
} else {
|
||||
$this->appManager = \OC::$server->getAppManager();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -409,12 +420,13 @@ class ThemingController extends Controller {
|
|||
* @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();
|
||||
}
|
||||
|
|
|
@ -829,6 +829,40 @@ class ThemingControllerTest extends TestCase {
|
|||
$this->assertEquals($response, $actual);
|
||||
}
|
||||
|
||||
public function testGetStylesheetOutsideServerroot() {
|
||||
$this->themingController = new ThemingController(
|
||||
'theming',
|
||||
$this->request,
|
||||
$this->config,
|
||||
$this->themingDefaults,
|
||||
$this->util,
|
||||
$this->timeFactory,
|
||||
$this->l10n,
|
||||
$this->tempManager,
|
||||
$this->appData,
|
||||
$this->scssCacher,
|
||||
$this->urlGenerator,
|
||||
$this->appManager
|
||||
);
|
||||
$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))
|
||||
|
|
Loading…
Reference in a new issue