Always suggest the overwrite.cli.url

Signed-off-by: Joas Schilling <coding@schilljs.com>

lib/private/Server.php

@@ -811,6 +811,7 @@ class Server extends ServerContainer implements IServerContainer {
 				$c->getEncryptionManager(),
 				$c->getUserManager(),
 				$c->getLockingProvider(),
+				$c->getRequest(),
 				new \OC\Settings\Mapper($c->getDatabaseConnection()),
 				$c->getURLGenerator()
 			);

lib/private/Settings/Admin/Server.php

@@ -32,12 +32,15 @@ use OCP\AppFramework\Http\TemplateResponse;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IL10N;
+use OCP\IRequest;
 use OCP\Lock\ILockingProvider;
 use OCP\Settings\ISettings;
 
 class Server implements ISettings {
 	/** @var IDBConnection|Connection */
 	private $db;
+	/** @var IRequest */
+	private $request;
 	/** @var IConfig */
 	private $config;
 	/** @var ILockingProvider */
@@ -47,15 +50,18 @@ class Server implements ISettings {
 
 	/**
 	 * @param IDBConnection $db
+	 * @param IRequest $request
 	 * @param IConfig $config
 	 * @param ILockingProvider $lockingProvider
 	 * @param IL10N $l
 	 */
 	public function __construct(IDBConnection $db,
+								IRequest $request,
 								IConfig $config,
 								ILockingProvider $lockingProvider,
 								IL10N $l) {
 		$this->db = $db;
+		$this->request = $request;
 		$this->config = $config;
 		$this->lockingProvider = $lockingProvider;
 		$this->l = $l;
@@ -99,12 +105,15 @@ class Server implements ISettings {
 			$fileLockingType = 'cache';
 		}
 
-		// If the current web root is non-empty but the web root from the config is,
-		// and system cron is used, the URL generator fails to build valid URLs.
-		$shouldSuggestOverwriteCliUrl = $this->config->getAppValue('core', 'backgroundjobs_mode', 'ajax') === 'cron'
-			&& \OC::$WEBROOT && \OC::$WEBROOT !== '/'
-			&& !$this->config->getSystemValue('overwrite.cli.url', '');
-		$suggestedOverwriteCliUrl = ($shouldSuggestOverwriteCliUrl) ? \OC::$WEBROOT : '';
+		$suggestedOverwriteCliUrl = '';
+		if ($this->config->getSystemValue('overwrite.cli.url', '') === '') {
+			$suggestedOverwriteCliUrl = $this->request->getServerProtocol() . '://' . $this->request->getInsecureServerHost() . \OC::$WEBROOT;
+			if (!$this->config->getSystemValue('config_is_read_only', false)) {
+				// Set the overwrite URL when it was not set yet.
+				$this->config->setSystemValue('overwrite.cli.url', $suggestedOverwriteCliUrl);
+				$suggestedOverwriteCliUrl = '';
+			}
+		}
 
 		$parameters = [
 			// Diagnosis

lib/private/Settings/Manager.php

@@ -29,6 +29,7 @@ use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IL10N;
 use OCP\ILogger;
+use OCP\IRequest;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\Lock\ILockingProvider;
@@ -56,6 +57,8 @@ class Manager implements IManager {
 	private $userManager;
 	/** @var ILockingProvider */
 	private $lockingProvider;
+	/** @var IRequest */
+	private $request;
 	/** @var IURLGenerator */
 	private $url;
 
@@ -67,6 +70,7 @@ class Manager implements IManager {
 	 * @param EncryptionManager $encryptionManager
 	 * @param IUserManager $userManager
 	 * @param ILockingProvider $lockingProvider
+	 * @param IRequest $request
 	 * @param Mapper $mapper
 	 * @param IURLGenerator $url
 	 */
@@ -78,6 +82,7 @@ class Manager implements IManager {
 		EncryptionManager $encryptionManager,
 		IUserManager $userManager,
 		ILockingProvider $lockingProvider,
+		IRequest $request,
 		Mapper $mapper,
 		IURLGenerator $url
 	) {
@@ -89,6 +94,7 @@ class Manager implements IManager {
 		$this->encryptionManager = $encryptionManager;
 		$this->userManager = $userManager;
 		$this->lockingProvider = $lockingProvider;
+		$this->request = $request;
 		$this->url = $url;
 	}
 
@@ -299,7 +305,7 @@ class Manager implements IManager {
 		try {
 			if ($section === 'server') {
 				/** @var ISettings $form */
-				$form = new Admin\Server($this->dbc, $this->config, $this->lockingProvider, $this->l);
+				$form = new Admin\Server($this->dbc, $this->request, $this->config, $this->lockingProvider, $this->l);
 				$forms[$form->getPriority()] = [$form];
 				$form = new Admin\ServerDevNotice();
 				$forms[$form->getPriority()] = [$form];

tests/lib/Settings/Admin/ServerTest.php

@@ -29,6 +29,7 @@ use OCP\AppFramework\Http\TemplateResponse;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IL10N;
+use OCP\IRequest;
 use OCP\Lock\ILockingProvider;
 use Test\TestCase;
 
@@ -37,6 +38,8 @@ class ServerTest extends TestCase {
 	private $admin;
 	/** @var IDBConnection */
 	private $dbConnection;
+	/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
+	private $request;
 	/** @var IConfig */
 	private $config;
 	/** @var ILockingProvider */
@@ -47,12 +50,14 @@ class ServerTest extends TestCase {
 	public function setUp() {
 		parent::setUp();
 		$this->config = $this->getMockBuilder('\OCP\IConfig')->getMock();
+		$this->request = $this->createMock(IRequest::class);
 		$this->dbConnection = $this->getMockBuilder('\OCP\IDBConnection')->getMock();
 		$this->lockingProvider = $this->getMockBuilder('\OCP\Lock\ILockingProvider')->getMock();
 		$this->l10n = $this->getMockBuilder('\OCP\IL10N')->getMock();
 
 		$this->admin = new Server(
 			$this->dbConnection,
+			$this->request,
 			$this->config,
 			$this->lockingProvider,
 			$this->l10n
@@ -66,9 +71,9 @@ class ServerTest extends TestCase {
 			->willReturn(new SqlitePlatform());
 		$this->config
 			->expects($this->at(0))
-			->method('getAppValue')
-			->with('core', 'backgroundjobs_mode', 'ajax')
-			->willReturn('ajax');
+			->method('getSystemValue')
+			->with('overwrite.cli.url', '')
+			->willReturn(true);
 		$this->config
 			->expects($this->at(2))
 			->method('getAppValue')

tests/lib/Settings/ManagerTest.php

@@ -32,6 +32,7 @@ use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IL10N;
 use OCP\ILogger;
+use OCP\IRequest;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\Lock\ILockingProvider;
@@ -54,6 +55,8 @@ class ManagerTest extends TestCase {
 	private $userManager;
 	/** @var ILockingProvider|\PHPUnit_Framework_MockObject_MockObject */
 	private $lockingProvider;
+	/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
+	private $request;
 	/** @var Mapper|\PHPUnit_Framework_MockObject_MockObject */
 	private $mapper;
 	/** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
@@ -69,6 +72,7 @@ class ManagerTest extends TestCase {
 		$this->encryptionManager = $this->createMock(IManager::class);
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->lockingProvider = $this->createMock(ILockingProvider::class);
+		$this->request = $this->createMock(IRequest::class);
 		$this->mapper = $this->createMock(Mapper::class);
 		$this->url = $this->createMock(IURLGenerator::class);
 
@@ -80,6 +84,7 @@ class ManagerTest extends TestCase {
 			$this->encryptionManager,
 			$this->userManager,
 			$this->lockingProvider,
+			$this->request,
 			$this->mapper,
 			$this->url
 		);