Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Stable10] Fix overriding for gif images in themes for CLI scripts #32131

Merged
merged 2 commits into from
Jul 24, 2018
Merged

[Stable10] Fix overriding for gif images in themes for CLI scripts #32131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbecaf5
Inject EnvironmentHelper
VicDeo Jul 4, 2018
68e0232
Load app-themes for CLI
VicDeo Jul 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,9 +581,8 @@ public static function init() {
\stream_wrapper_register('quota', 'OC\Files\Stream\Quota');

\OC::$server->getEventLogger()->start('init_session', 'Initialize session');
OC_App::loadApps(['session']);
OC_App::loadApps(['session', 'theme']);
if (!self::$CLI) {
\OC_App::loadApps(['theme']);
self::initSession();
}

Expand Down
3 changes: 2 additions & 1 deletion lib/private/Server.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,8 @@ public function __construct($webRoot, \OC\Config $config) {
return new URLGenerator(
$config,
$cacheFactory,
$router
$router,
new \OC\Helper\EnvironmentHelper()
);
});
$this->registerService('AppHelper', function ($c) {
Expand Down
50 changes: 35 additions & 15 deletions lib/private/URLGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
*/

namespace OC;

use OC\Helper\EnvironmentHelper;
use OCP\Theme\ITheme;
use OC_Defaults;
use OCP\ICacheFactory;
Expand All @@ -50,18 +52,25 @@ class URLGenerator implements IURLGenerator {
private $router;
/** @var ITheme */
private $theme;

/** @var EnvironmentHelper */
private $environmentHelper;

/**
* @param IConfig $config
* @param ICacheFactory $cacheFactory
* @param IRouter $router
* @param EnvironmentHelper $environmentHelper
*/
public function __construct(IConfig $config,
ICacheFactory $cacheFactory,
IRouter $router) {
IRouter $router,
EnvironmentHelper $environmentHelper
) {
$this->config = $config;
$this->cacheFactory = $cacheFactory;
$this->router = $router;
$this->environmentHelper = $environmentHelper;
$this->theme = \OC_Util::getTheme();
}

Expand Down Expand Up @@ -102,31 +111,32 @@ public function linkToRouteAbsolute($routeName, $arguments = []) {
*/
public function linkTo($app, $file, $args = []) {
$frontControllerActive = (\getenv('front_controller_active') === 'true');
$webRoot = $this->environmentHelper->getWebRoot();

if ($app != '') {
$app_path = \OC_App::getAppPath($app);
// Check if the app is in the app folder
if ($app_path && \file_exists($app_path . '/' . $file)) {
if (\substr($file, -3) == 'php') {
$urlLinkTo = \OC::$WEBROOT . '/index.php/apps/' . $app;
$urlLinkTo = $webRoot . '/index.php/apps/' . $app;
if ($frontControllerActive) {
$urlLinkTo = \OC::$WEBROOT . '/apps/' . $app;
$urlLinkTo = $webRoot . '/apps/' . $app;
}
$urlLinkTo .= ($file != 'index.php') ? '/' . $file : '';
} else {
$urlLinkTo = \OC_App::getAppWebPath($app) . '/' . $file;
}
} else {
$urlLinkTo = \OC::$WEBROOT . '/' . $app . '/' . $file;
$urlLinkTo = $webRoot . '/' . $app . '/' . $file;
}
} else {
if (\file_exists(\OC::$SERVERROOT . '/core/' . $file)) {
$urlLinkTo = \OC::$WEBROOT . '/core/' . $file;
if (\file_exists($this->environmentHelper->getServerRoot() . '/core/' . $file)) {
$urlLinkTo = $webRoot . '/core/' . $file;
} else {
if ($frontControllerActive && $file === 'index.php') {
$urlLinkTo = \OC::$WEBROOT . '/';
$urlLinkTo = $webRoot . '/';
} else {
$urlLinkTo = \OC::$WEBROOT . '/' . $file;
$urlLinkTo = $webRoot . '/' . $file;
}
}
}
Expand Down Expand Up @@ -160,7 +170,11 @@ public function imagePath($app, $image) {
$cache->set($cacheKey, $path);
return $path;
} else {
throw new RuntimeException('image not found: image:' . $image . ' webroot:' . \OC::$WEBROOT . ' serverroot:' . \OC::$SERVERROOT);
throw new RuntimeException(
'image not found: image:' . $image
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next time you see such string concat please replace with string interpolation. THX

. ' webroot:' . $this->environmentHelper->getWebRoot()
. ' serverroot:' . $this->environmentHelper->getServerRoot()
);
}
}

Expand All @@ -170,8 +184,13 @@ public function imagePath($app, $image) {
* @return string
*/
private function getImagePath($app, $imageName) {
$appWebPath = \OC_App::getAppWebPath($app);
$appPath = \substr($appWebPath, \strlen(\OC::$WEBROOT));
$webRoot = $this->environmentHelper->getWebRoot();
if ($app !== '') {
$appWebPath = \OC_App::getAppWebPath($app);
} else {
$appWebPath = $webRoot;
}
$appPath = \substr($appWebPath, \strlen($webRoot));

$directories = ["/core", ""];

Expand All @@ -190,8 +209,8 @@ private function getImagePath($app, $imageName) {
return $this->theme->getWebPath() . $file;
}

if ($imagePath = $this->getImagePathOrFallback(\OC::$SERVERROOT . $file)) {
return \OC::$WEBROOT . $file;
if ($imagePath = $this->getImagePathOrFallback($this->environmentHelper->getServerRoot() . $file)) {
return $webRoot . $file;
}
}
}
Expand All @@ -216,16 +235,17 @@ private function getImagePathOrFallback($file) {
* @return string the absolute version of the url
*/
public function getAbsoluteURL($url) {
$webRoot = $this->environmentHelper->getWebRoot();
$separator = $url[0] === '/' ? '' : '/';

if (\OC::$CLI && !\defined('PHPUNIT_RUN')) {
return \rtrim($this->config->getSystemValue('overwrite.cli.url'), '/') . '/' . \ltrim($url, '/');
}

// The ownCloud web root can already be prepended.
$webRoot = \substr($url, 0, \strlen(\OC::$WEBROOT)) === \OC::$WEBROOT
$webRoot = \substr($url, 0, \strlen($webRoot)) === $webRoot
? ''
: \OC::$WEBROOT;
: $webRoot;

$request = \OC::$server->getRequest();
return $request->getServerProtocol() . '://' . $request->getServerHost() . $webRoot . $separator . $url;
Expand Down
40 changes: 30 additions & 10 deletions tests/lib/UrlGeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

namespace Test;
use OC\Helper\EnvironmentHelper;
use OC\URLGenerator;
use OCP\ICacheFactory;
use OCP\IConfig;
Expand All @@ -23,12 +24,21 @@ class UrlGeneratorTest extends TestCase {
/** @var IRouter | \PHPUnit_Framework_MockObject_MockObject */
private $router;

/** @var EnvironmentHelper | \PHPUnit_Framework_MockObject_MockObject */
private $environmentHelper;

public function setUp() {
parent::setUp();
$config = $this->createMock(IConfig::class);
$cacheFactory = $this->createMock(ICacheFactory::class);
$this->router = $this->createMock(IRouter::class);
$this->urlGenerator = new URLGenerator($config, $cacheFactory, $this->router);
$this->environmentHelper = $this->createMock(EnvironmentHelper::class);
$this->urlGenerator = new URLGenerator(
$config,
$cacheFactory,
$this->router,
$this->environmentHelper
);
}

/**
Expand All @@ -37,9 +47,11 @@ public function setUp() {
* @dataProvider provideDocRootAppUrlParts
*/
public function testLinkToDocRoot($app, $file, $args, $expectedResult) {
\OC::$WEBROOT = '';
$result = $this->urlGenerator->linkTo($app, $file, $args);
$this->environmentHelper->expects($this->any())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expects($this->any()) is default - just drop it next time

->method('getWebRoot')
->willReturn('');

$result = $this->urlGenerator->linkTo($app, $file, $args);
$this->assertEquals($expectedResult, $result);
}

Expand All @@ -49,15 +61,19 @@ public function testLinkToDocRoot($app, $file, $args, $expectedResult) {
* @dataProvider provideSubDirAppUrlParts
*/
public function testLinkToSubDir($app, $file, $args, $expectedResult) {
\OC::$WEBROOT = '/owncloud';
$result = $this->urlGenerator->linkTo($app, $file, $args);
$this->environmentHelper->expects($this->any())
->method('getWebRoot')
->willReturn('/owncloud');

$result = $this->urlGenerator->linkTo($app, $file, $args);
$this->assertEquals($expectedResult, $result);
}

public function testLinkToRouteAbsolute() {
$route = 'files_ajax_list';
\OC::$WEBROOT = '/owncloud';
$this->environmentHelper->expects($this->any())
->method('getWebRoot')
->willReturn('/owncloud');
$this->router->expects($this->once())->method('generate')
->with($route)->willReturn('index.php/apps/files/ajax/list.php');

Expand Down Expand Up @@ -87,9 +103,11 @@ public function provideSubDirAppUrlParts() {
* @dataProvider provideDocRootURLs
*/
public function testGetAbsoluteURLDocRoot($url, $expectedResult) {
\OC::$WEBROOT = '';
$result = $this->urlGenerator->getAbsoluteURL($url);
$this->environmentHelper->expects($this->any())
->method('getWebRoot')
->willReturn('');

$result = $this->urlGenerator->getAbsoluteURL($url);
$this->assertEquals($expectedResult, $result);
}

Expand All @@ -99,9 +117,11 @@ public function testGetAbsoluteURLDocRoot($url, $expectedResult) {
* @dataProvider provideSubDirURLs
*/
public function testGetAbsoluteURLSubDir($url, $expectedResult) {
\OC::$WEBROOT = '/owncloud';
$result = $this->urlGenerator->getAbsoluteURL($url);
$this->environmentHelper->expects($this->any())
->method('getWebRoot')
->willReturn('/owncloud');

$result = $this->urlGenerator->getAbsoluteURL($url);
$this->assertEquals($expectedResult, $result);
}

Expand Down