From 9157d5981f6840bfa06652ad8fd16754c6fd679d Mon Sep 17 00:00:00 2001
From: Chocobozzz <me@florianbigard.com>
Date: Wed, 11 Sep 2019 10:19:03 +0200
Subject: [PATCH] Improve plugin package.json error message

---
 server/helpers/custom-validators/plugins.ts | 70 +++++++++++++++++----
 server/lib/plugins/plugin-manager.ts        | 24 +++++--
 2 files changed, 77 insertions(+), 17 deletions(-)

diff --git a/server/helpers/custom-validators/plugins.ts b/server/helpers/custom-validators/plugins.ts
index 63af91a44..2e3175742 100644
--- a/server/helpers/custom-validators/plugins.ts
+++ b/server/helpers/custom-validators/plugins.ts
@@ -84,17 +84,65 @@ function isThemeNameValid (name: string) {
 }
 
 function isPackageJSONValid (packageJSON: PluginPackageJson, pluginType: PluginType) {
-  return isNpmPluginNameValid(packageJSON.name) &&
-    isPluginDescriptionValid(packageJSON.description) &&
-    isPluginEngineValid(packageJSON.engine) &&
-    isPluginHomepage(packageJSON.homepage) &&
-    exists(packageJSON.author) &&
-    isPluginBugs(packageJSON.bugs) &&
-    (pluginType === PluginType.THEME || isSafePath(packageJSON.library)) &&
-    areStaticDirectoriesValid(packageJSON.staticDirs) &&
-    areCSSPathsValid(packageJSON.css) &&
-    areClientScriptsValid(packageJSON.clientScripts) &&
-    areTranslationPathsValid(packageJSON.translations)
+  let result = true
+  const badFields: string[] = []
+
+  if (!isNpmPluginNameValid(packageJSON.name)) {
+    result = false
+    badFields.push('name')
+  }
+
+  if (!isPluginDescriptionValid(packageJSON.description)) {
+    result = false
+    badFields.push('description')
+  }
+
+  if (!isPluginEngineValid(packageJSON.engine)) {
+    result = false
+    badFields.push('engine')
+  }
+
+  if (!isPluginHomepage(packageJSON.homepage)) {
+    result = false
+    badFields.push('homepage')
+  }
+
+  if (!exists(packageJSON.author)) {
+    result = false
+    badFields.push('author')
+  }
+
+  if (!isPluginBugs(packageJSON.bugs)) {
+    result = false
+    badFields.push('bugs')
+  }
+
+  if (pluginType === PluginType.PLUGIN && !isSafePath(packageJSON.library)) {
+    result = false
+    badFields.push('library')
+  }
+
+  if (!areStaticDirectoriesValid(packageJSON.staticDirs)) {
+    result = false
+    badFields.push('staticDirs')
+  }
+
+  if (!areCSSPathsValid(packageJSON.css)) {
+    result = false
+    badFields.push('css')
+  }
+
+  if (!areClientScriptsValid(packageJSON.clientScripts)) {
+    result = false
+    badFields.push('clientScripts')
+  }
+
+  if (!areTranslationPathsValid(packageJSON.translations)) {
+    result = false
+    badFields.push('translations')
+  }
+
+  return { result, badFields }
 }
 
 function isLibraryCodeValid (library: any) {
diff --git a/server/lib/plugins/plugin-manager.ts b/server/lib/plugins/plugin-manager.ts
index 444162a03..8127992b5 100644
--- a/server/lib/plugins/plugin-manager.ts
+++ b/server/lib/plugins/plugin-manager.ts
@@ -222,9 +222,8 @@ export class PluginManager implements ServerHook {
       const pluginName = PluginModel.normalizePluginName(npmName)
 
       const packageJSON = await this.getPackageJSON(pluginName, pluginType)
-      if (!isPackageJSONValid(packageJSON, pluginType)) {
-        throw new Error('PackageJSON is invalid.')
-      }
+
+      this.sanitizeAndCheckPackageJSONOrThrow(packageJSON, pluginType);
 
       [ plugin ] = await PluginModel.upsert({
         name: pluginName,
@@ -301,9 +300,7 @@ export class PluginManager implements ServerHook {
     const packageJSON = await this.getPackageJSON(plugin.name, plugin.type)
     const pluginPath = this.getPluginPath(plugin.name, plugin.type)
 
-    if (!isPackageJSONValid(packageJSON, plugin.type)) {
-      throw new Error('Package.JSON is invalid.')
-    }
+    this.sanitizeAndCheckPackageJSONOrThrow(packageJSON, plugin.type)
 
     let library: PluginLibrary
     if (plugin.type === PluginType.PLUGIN) {
@@ -598,6 +595,21 @@ export class PluginManager implements ServerHook {
     }
   }
 
+  private sanitizeAndCheckPackageJSONOrThrow (packageJSON: PluginPackageJson, pluginType: PluginType) {
+    if (!packageJSON.staticDirs) packageJSON.staticDirs = {}
+    if (!packageJSON.css) packageJSON.css = []
+    if (!packageJSON.clientScripts) packageJSON.clientScripts = []
+    if (!packageJSON.translations) packageJSON.translations = {}
+
+    const { result: packageJSONValid, badFields } = isPackageJSONValid(packageJSON, pluginType)
+    if (!packageJSONValid) {
+      const formattedFields = badFields.map(f => `"${f}"`)
+                              .join(', ')
+
+      throw new Error(`PackageJSON is invalid (invalid fields: ${formattedFields}).`)
+    }
+  }
+
   static get Instance () {
     return this.instance || (this.instance = new this())
   }
-- 
2.25.1