Merge pull request #18288 from nextcloud/bug/18277/catch-json-errors

Ensure that we don't merge broken json.
nextcloud · Dec 9, 2019 · 962a51e · 962a51e
2 parents b13da60 + 29575c4
commit 962a51e
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 19 deletions.
diff --git a/lib/private/Files/Type/Detection.php b/lib/private/Files/Type/Detection.php
@@ -35,6 +35,7 @@
 namespace OC\Files\Type;
 
 use OCP\Files\IMimeTypeDetector;
+use OCP\ILogger;
 use OCP\IURLGenerator;
 
 /**
@@ -45,6 +46,10 @@
  * @package OC\Files\Type
  */
 class Detection implements IMimeTypeDetector {
+
+	private const CUSTOM_MIMETYPEMAPPING = 'mimetypemapping.json';
+	private const CUSTOM_MIMETYPEALIASES = 'mimetypealiases.json';
+
 	protected $mimetypes = [];
 	protected $secureMimeTypes = [];
 
@@ -55,6 +60,9 @@ class Detection implements IMimeTypeDetector {
 	/** @var IURLGenerator */
 	private $urlGenerator;
 
+	/** @var ILogger */
+	private $logger;
+
 	/** @var string */
 	private $customConfigDir;
 
@@ -63,13 +71,16 @@ class Detection implements IMimeTypeDetector {
 
 	/**
 	 * @param IURLGenerator $urlGenerator
+	 * @param ILogger $logger
 	 * @param string $customConfigDir
 	 * @param string $defaultConfigDir
 	 */
 	public function __construct(IURLGenerator $urlGenerator,
+								ILogger $logger,
 								$customConfigDir,
 								$defaultConfigDir) {
 		$this->urlGenerator = $urlGenerator;
+		$this->logger = $logger;
 		$this->customConfigDir = $customConfigDir;
 		$this->defaultConfigDir = $defaultConfigDir;
 	}
@@ -110,6 +121,18 @@ public function registerTypeArray($types) {
 		}
 	}
 
+	private function loadCustomDefinitions(string $fileName, array $definitions): array {
+		if (file_exists($this->customConfigDir . '/' . $fileName)) {
+			$custom = json_decode(file_get_contents($this->customConfigDir . '/' . $fileName), true);
+			if (json_last_error() === JSON_ERROR_NONE) {
+				$definitions = array_merge($definitions, $custom);
+			} else {
+				$this->logger->warning('Failed to parse ' . $fileName . ': ' . json_last_error_msg());
+			}
+		}
+		return $definitions;
+	}
+
 	/**
 	 * Add the mimetype aliases if they are not yet present
 	 */
@@ -119,11 +142,7 @@ private function loadAliases() {
 		}
 
 		$this->mimeTypeAlias = json_decode(file_get_contents($this->defaultConfigDir . '/mimetypealiases.dist.json'), true);
-
-		if (file_exists($this->customConfigDir . '/mimetypealiases.json')) {
-			$custom = json_decode(file_get_contents($this->customConfigDir . '/mimetypealiases.json'), true);
-			$this->mimeTypeAlias = array_merge($this->mimeTypeAlias, $custom);
-		}
+		$this->mimeTypeAlias = $this->loadCustomDefinitions(self::CUSTOM_MIMETYPEALIASES, $this->mimeTypeAlias);
 	}
 
 	/**
@@ -149,12 +168,7 @@ private function loadMappings() {
 		}
 
 		$mimetypeMapping = json_decode(file_get_contents($this->defaultConfigDir . '/mimetypemapping.dist.json'), true);
-
-		//Check if need to load custom mappings
-		if (file_exists($this->customConfigDir . '/mimetypemapping.json')) {
-			$custom = json_decode(file_get_contents($this->customConfigDir . '/mimetypemapping.json'), true);
-			$mimetypeMapping = array_merge($mimetypeMapping, $custom);
-		}
+		$mimetypeMapping = $this->loadCustomDefinitions(self::CUSTOM_MIMETYPEMAPPING, $mimetypeMapping);
 
 		$this->registerTypeArray($mimetypeMapping);
 	}

diff --git a/lib/private/Server.php b/lib/private/Server.php
@@ -882,6 +882,7 @@ public function __construct($webRoot, \OC\Config $config) {
 		$this->registerService(\OCP\Files\IMimeTypeDetector::class, function (Server $c) {
 			return new \OC\Files\Type\Detection(
 				$c->getURLGenerator(),
+				$c->getLogger(),
 				\OC::$configDir,
 				\OC::$SERVERROOT . '/resources/config/'
 			);

diff --git a/tests/lib/Files/Type/DetectionTest.php b/tests/lib/Files/Type/DetectionTest.php
@@ -22,6 +22,7 @@
 namespace Test\Files\Type;
 
 use OC\Files\Type\Detection;
+use OCP\ILogger;
 use OCP\IURLGenerator;
 
 class DetectionTest extends \Test\TestCase {
@@ -32,6 +33,7 @@ protected function setUp(): void {
 		parent::setUp();
 		$this->detection = new Detection(
 			\OC::$server->getURLGenerator(),
+			\OC::$server->getLogger(),
 			\OC::$SERVERROOT . '/config/',
 			\OC::$SERVERROOT . '/resources/config/'
 		);
@@ -114,13 +116,16 @@ public function testMimeTypeIcon() {
 			->disableOriginalConstructor()
 			->getMock();
 
+		/** @var ILogger $logger */
+		$logger = $this->createMock(ILogger::class);
+
 		//Only call the url generator once
 		$urlGenerator->expects($this->once())
 			->method('imagePath')
 			->with($this->equalTo('core'), $this->equalTo('filetypes/folder.png'))
 			->willReturn('folder.svg');
 
-		$detection = new Detection($urlGenerator, $confDir->url(), $confDir->url());
+		$detection = new Detection($urlGenerator, $logger, $confDir->url(), $confDir->url());
 		$mimeType = $detection->mimeTypeIcon('dir');
 		$this->assertEquals('folder.svg', $mimeType);
 
@@ -139,7 +144,7 @@ public function testMimeTypeIcon() {
 			->with($this->equalTo('core'), $this->equalTo('filetypes/folder-shared.png'))
 			->willReturn('folder-shared.svg');
 
-		$detection = new Detection($urlGenerator, $confDir->url(), $confDir->url());
+		$detection = new Detection($urlGenerator, $logger, $confDir->url(), $confDir->url());
 		$mimeType = $detection->mimeTypeIcon('dir-shared');
 		$this->assertEquals('folder-shared.svg', $mimeType);
 
@@ -159,7 +164,7 @@ public function testMimeTypeIcon() {
 			->with($this->equalTo('core'), $this->equalTo('filetypes/folder-external.png'))
 			->willReturn('folder-external.svg');
 
-		$detection = new Detection($urlGenerator, $confDir->url(), $confDir->url());
+		$detection = new Detection($urlGenerator, $logger, $confDir->url(), $confDir->url());
 		$mimeType = $detection->mimeTypeIcon('dir-external');
 		$this->assertEquals('folder-external.svg', $mimeType);
 
@@ -179,7 +184,7 @@ public function testMimeTypeIcon() {
 			->with($this->equalTo('core'), $this->equalTo('filetypes/my-type.png'))
 			->willReturn('my-type.svg');
 
-		$detection = new Detection($urlGenerator, $confDir->url(), $confDir->url());
+		$detection = new Detection($urlGenerator, $logger, $confDir->url(), $confDir->url());
 		$mimeType = $detection->mimeTypeIcon('my-type');
 		$this->assertEquals('my-type.svg', $mimeType);
 
@@ -209,7 +214,7 @@ function($appName, $file) {
 				}
 			));
 
-		$detection = new Detection($urlGenerator, $confDir->url(), $confDir->url());
+		$detection = new Detection($urlGenerator, $logger, $confDir->url(), $confDir->url());
 		$mimeType = $detection->mimeTypeIcon('my-type');
 		$this->assertEquals('my.svg', $mimeType);
 
@@ -240,7 +245,7 @@ function($appName, $file) {
 				}
 			));
 
-		$detection = new Detection($urlGenerator, $confDir->url(), $confDir->url());
+		$detection = new Detection($urlGenerator, $logger, $confDir->url(), $confDir->url());
 		$mimeType = $detection->mimeTypeIcon('foo-bar');
 		$this->assertEquals('file.svg', $mimeType);
 
@@ -259,7 +264,7 @@ function($appName, $file) {
 			->with($this->equalTo('core'), $this->equalTo('filetypes/foo-bar.png'))
 			->willReturn('foo-bar.svg');
 
-		$detection = new Detection($urlGenerator, $confDir->url(), $confDir->url());
+		$detection = new Detection($urlGenerator, $logger, $confDir->url(), $confDir->url());
 		$mimeType = $detection->mimeTypeIcon('foo-bar');
 		$this->assertEquals('foo-bar.svg', $mimeType);
 		$mimeType = $detection->mimeTypeIcon('foo-bar');
@@ -285,7 +290,7 @@ function($appName, $file) {
 			->with($this->equalTo('core'), $this->equalTo('filetypes/foobar-baz.png'))
 			->willReturn('foobar-baz.svg');
 
-		$detection = new Detection($urlGenerator, $confDir->url(), $confDir->url());
+		$detection = new Detection($urlGenerator, $logger, $confDir->url(), $confDir->url());
 		$mimeType = $detection->mimeTypeIcon('foo');
 		$this->assertEquals('foobar-baz.svg', $mimeType);
 	}