Hash key for plugin store and limit id length (#7933)

app/plugin.go

@@ -6,12 +6,15 @@ package app
 import (
 	"bytes"
 	"context"
+	"crypto/sha256"
+	"encoding/base64"
 	"io"
 	"io/ioutil"
 	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
+	"unicode/utf8"
 
 	l4g "github.com/alecthomas/log4go"
 
@@ -28,6 +31,10 @@ import (
 	"github.com/mattermost/mattermost-server/plugin/pluginenv"
 )
 
+const (
+	PLUGIN_MAX_ID_LENGTH = 190
+)
+
 var prepackagedPlugins map[string]func(string) ([]byte, error) = map[string]func(string) ([]byte, error){
 	"jira": jira.Asset,
 	"zoom": zoom.Asset,
@@ -149,6 +156,10 @@ func (a *App) installPlugin(pluginFile io.Reader, allowPrepackaged bool) (*model
 		return nil, model.NewAppError("installPlugin", "app.plugin.prepackaged.app_error", nil, "", http.StatusBadRequest)
 	}
 
+	if utf8.RuneCountInString(manifest.Id) > PLUGIN_MAX_ID_LENGTH {
+		return nil, model.NewAppError("installPlugin", "app.plugin.id_length.app_error", map[string]interface{}{"Max": PLUGIN_MAX_ID_LENGTH}, err.Error(), http.StatusBadRequest)
+	}
+
 	bundles, err := a.PluginEnv.Plugins()
 	if err != nil {
 		return nil, model.NewAppError("installPlugin", "app.plugin.install.app_error", nil, err.Error(), http.StatusInternalServerError)
@@ -475,10 +486,16 @@ func (a *App) ShutDownPlugins() {
 	a.PluginEnv = nil
 }
 
+func getKeyHash(key string) string {
+	hash := sha256.New()
+	hash.Write([]byte(key))
+	return base64.StdEncoding.EncodeToString(hash.Sum(nil))
+}
+
 func (a *App) SetPluginKey(pluginId string, key string, value []byte) *model.AppError {
 	kv := &model.PluginKeyValue{
 		PluginId: pluginId,
-		Key:      key,
+		Key:      getKeyHash(key),
 		Value:    value,
 	}
 
@@ -492,7 +509,7 @@ func (a *App) SetPluginKey(pluginId string, key string, value []byte) *model.App
 }
 
 func (a *App) GetPluginKey(pluginId string, key string) ([]byte, *model.AppError) {
-	result := <-a.Srv.Store.Plugin().Get(pluginId, key)
+	result := <-a.Srv.Store.Plugin().Get(pluginId, getKeyHash(key))
 
 	if result.Err != nil {
 		if result.Err.StatusCode == http.StatusNotFound {
@@ -508,7 +525,7 @@ func (a *App) GetPluginKey(pluginId string, key string) ([]byte, *model.AppError
 }
 
 func (a *App) DeletePluginKey(pluginId string, key string) *model.AppError {
-	result := <-a.Srv.Store.Plugin().Delete(pluginId, key)
+	result := <-a.Srv.Store.Plugin().Delete(pluginId, getKeyHash(key))
 
 	if result.Err != nil {
 		l4g.Error(result.Err.Error())

i18n/en.json

@@ -3558,18 +3558,14 @@
     "id": "app.notification.subject.notification.full",
     "translation": "[{{ .SiteName }}] Notification in {{ .TeamName}} on {{.Month}} {{.Day}}, {{.Year}}"
   },
-  {
-    "id": "app.plugin.prepackaged.app_error",
-    "translation": "Prepackaged plugins cannot be modified."
-  },
-  {
-    "id": "app.plugin.key_value.set.app_error",
-    "translation": "Unable to set key value. See detailed error for more information."
-  },
   {
     "id": "app.plugin.activate.app_error",
     "translation": "Unable to activate extracted plugin. Plugin may already exist and be activated."
   },
+  {
+    "id": "app.plugin.id_length.app_error",
+    "translation": "Plugin Id must be less than {{.Max}} characters."
+  },
   {
     "id": "app.plugin.config.app_error",
     "translation": "Error saving plugin state in config"

model/manifest.go

@@ -102,8 +102,9 @@ type PluginSettingsSchema struct {
 //               help_text: When true, an extra thing will be enabled!
 //               default: false
 type Manifest struct {
-	// The id is a globally unique identifier that represents your plugin. Reverse-DNS notation
+	// The id is a globally unique identifier that represents your plugin. Ids are limited
-	// using a name you control is a good option. For example, "com.mycompany.myplugin".
+	// to 190 characters. Reverse-DNS notation using a name you control is a good option.
+	// For example, "com.mycompany.myplugin".
 	Id string `json:"id" yaml:"id"`
 
 	// The name to be displayed for the plugin.

model/plugin_key_value.go

@@ -9,8 +9,8 @@ import (
 )
 
 const (
-	KEY_VALUE_PLUGIN_ID_MAX_RUNES = 100
+	KEY_VALUE_PLUGIN_ID_MAX_RUNES = 190
-	KEY_VALUE_KEY_MAX_RUNES       = 100
+	KEY_VALUE_KEY_MAX_RUNES       = 50
 )
 
 type PluginKeyValue struct {

store/sqlstore/plugin_store.go

@@ -21,8 +21,8 @@ func NewSqlPluginStore(sqlStore SqlStore) store.PluginStore {
 
 	for _, db := range sqlStore.GetAllConns() {
 		table := db.AddTableWithName(model.PluginKeyValue{}, "PluginKeyValueStore").SetKeys(false, "PluginId", "Key")
-		table.ColMap("PluginId").SetMaxSize(100)
+		table.ColMap("PluginId").SetMaxSize(190)
-		table.ColMap("Key").SetMaxSize(100)
+		table.ColMap("Key").SetMaxSize(50)
 		table.ColMap("Value").SetMaxSize(8192)
 	}
 

store/sqlstore/upgrade.go

@@ -323,6 +323,10 @@ func UpgradeDatabaseToVersion44(sqlStore SqlStore) {
 }
 
 func UpgradeDatabaseToVersion45(sqlStore SqlStore) {
+	//TODO: Remove next two lines before 4.5 release. They're just here to fix CI servers
+	sqlStore.AlterColumnTypeIfExists("PluginKeyValueStore", "PKey", "varchar(50)", "varchar(50)")
+	sqlStore.AlterColumnTypeIfExists("PluginKeyValueStore", "PluginId", "varchar(190)", "varchar(190)")
+
 	//TODO: Uncomment when 4.5 is released
 	/*if shouldPerformUpgrade(sqlStore, VERSION_4_4_0, VERSION_4_5_0) {