MM-11420: plugins: compute bundle hash on load (mattermost#9172)

* plugins: compute bundle hash on load Use this hash to bust client caches whenever the plugin bundle changes. * eliminate redundant pluginHandler * switch to 64-bit FNV-1a * Fix test
nicospacez · Jul 31, 2018 · 0788cdc · 0788cdc
1 parent 8c56f52
commit 0788cdc
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 54 deletions.
diff --git a/app/plugin.go b/app/plugin.go
@@ -40,7 +40,12 @@ func (a *App) SyncPluginsActiveState() {
 
 			// If it's not enabled we need to deactivate it
 			if !pluginEnabled {
-				a.Plugins.Deactivate(pluginId)
+				deactivated := a.Plugins.Deactivate(pluginId)
+				if deactivated && plugin.Manifest.HasClient() {
+					message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_DISABLED, "", "", "", nil)
+					message.Add("manifest", plugin.Manifest.ClientManifest())
+					a.Publish(message)
+				}
 			}
 		}
 
@@ -60,8 +65,16 @@ func (a *App) SyncPluginsActiveState() {
 
 			// Activate plugin if enabled
 			if pluginEnabled {
-				if err := a.Plugins.Activate(pluginId); err != nil {
+				updatedManifest, activated, err := a.Plugins.Activate(pluginId)
+				if err != nil {
 					plugin.WrapLogger(a.Log).Error("Unable to activate plugin", mlog.Err(err))
+					continue
+				}
+
+				if activated && updatedManifest.HasClient() {
+					message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_ENABLED, "", "", "", nil)
+					message.Add("manifest", updatedManifest.ClientManifest())
+					a.Publish(message)
 				}
 			}
 		}
@@ -194,12 +207,6 @@ func (a *App) EnablePlugin(id string) *model.AppError {
 		cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: true}
 	})
 
-	if manifest.HasClient() {
-		message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_ENABLED, "", "", "", nil)
-		message.Add("manifest", manifest.ClientManifest())
-		a.Publish(message)
-	}
-
 	// This call will cause SyncPluginsActiveState to be called and the plugin to be activated
 	if err := a.SaveConfig(a.Config(), true); err != nil {
 		if err.Id == "ent.cluster.save_config.error" {
@@ -240,12 +247,6 @@ func (a *App) DisablePlugin(id string) *model.AppError {
 		cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: false}
 	})
 
-	if manifest.HasClient() {
-		message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_PLUGIN_DISABLED, "", "", "", nil)
-		message.Add("manifest", manifest.ClientManifest())
-		a.Publish(message)
-	}
-
 	if err := a.SaveConfig(a.Config(), true); err != nil {
 		return model.NewAppError("DisablePlugin", "app.plugin.config.app_error", nil, err.Error(), http.StatusInternalServerError)
 	}

diff --git a/model/manifest.go b/model/manifest.go
@@ -5,6 +5,7 @@ package model
 
 import (
 	"encoding/json"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -151,6 +152,9 @@ type ManifestWebapp struct {
 	// The path to your webapp bundle. This should be relative to the root of your bundle and the
 	// location of the manifest file.
 	BundlePath string `json:"bundle_path" yaml:"bundle_path"`
+
+	// BundleHash is the 64-bit FNV-1a hash of the webapp bundle, computed when the plugin is loaded
+	BundleHash []byte `json:"-"`
 }
 
 func (m *Manifest) ToJson() string {
@@ -188,7 +192,7 @@ func (m *Manifest) ClientManifest() *Manifest {
 	if cm.Webapp != nil {
 		cm.Webapp = new(ManifestWebapp)
 		*cm.Webapp = *m.Webapp
-		cm.Webapp.BundlePath = "/static/" + m.Id + "/" + m.Id + "_bundle.js"
+		cm.Webapp.BundlePath = "/static/" + m.Id + "/" + fmt.Sprintf("%s_%x_bundle.js", m.Id, m.Webapp.BundleHash)
 	}
 	return cm
 }

diff --git a/model/manifest_test.go b/model/manifest_test.go
@@ -255,6 +255,7 @@ func TestManifestClientManifest(t *testing.T) {
 		},
 		Webapp: &ManifestWebapp{
 			BundlePath: "thebundlepath",
+			BundleHash: []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15},
 		},
 		SettingsSchema: &PluginSettingsSchema{
 			Header: "theheadertext",
@@ -281,10 +282,11 @@ func TestManifestClientManifest(t *testing.T) {
 
 	sanitized := manifest.ClientManifest()
 
-	assert.NotEmpty(t, sanitized.Id)
-	assert.NotEmpty(t, sanitized.Version)
-	assert.NotEmpty(t, sanitized.Webapp)
-	assert.NotEmpty(t, sanitized.SettingsSchema)
+	assert.Equal(t, manifest.Id, sanitized.Id)
+	assert.Equal(t, manifest.Version, sanitized.Version)
+	assert.Equal(t, "/static/theid/theid_000102030405060708090a0b0c0d0e0f_bundle.js", sanitized.Webapp.BundlePath)
+	assert.Equal(t, manifest.Webapp.BundleHash, sanitized.Webapp.BundleHash)
+	assert.Equal(t, manifest.SettingsSchema, sanitized.SettingsSchema)
 	assert.Empty(t, sanitized.Name)
 	assert.Empty(t, sanitized.Description)
 	assert.Empty(t, sanitized.Server)

diff --git a/plugin/environment.go b/plugin/environment.go
@@ -5,6 +5,7 @@ package plugin
 
 import (
 	"fmt"
+	"hash/fnv"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -133,29 +134,27 @@ func (env *Environment) Statuses() (model.PluginStatuses, error) {
 	return pluginStatuses, nil
 }
 
-// Activate activates the plugin with the given id.
-func (env *Environment) Activate(id string) (reterr error) {
-
+func (env *Environment) Activate(id string) (manifest *model.Manifest, activated bool, reterr error) {
 	// Check if we are already active
 	if _, ok := env.activePlugins.Load(id); ok {
-		return nil
+		return nil, false, nil
 	}
 
 	plugins, err := env.Available()
 	if err != nil {
-		return err
+		return nil, false, err
 	}
 	var pluginInfo *model.BundleInfo
 	for _, p := range plugins {
 		if p.Manifest != nil && p.Manifest.Id == id {
 			if pluginInfo != nil {
-				return fmt.Errorf("multiple plugins found: %v", id)
+				return nil, false, fmt.Errorf("multiple plugins found: %v", id)
 			}
 			pluginInfo = p
 		}
 	}
 	if pluginInfo == nil {
-		return fmt.Errorf("plugin not found: %v", id)
+		return nil, false, fmt.Errorf("plugin not found: %v", id)
 	}
 
 	activePlugin := activePlugin{BundleInfo: pluginInfo}
@@ -171,43 +170,54 @@ func (env *Environment) Activate(id string) (reterr error) {
 	if pluginInfo.Manifest.Webapp != nil {
 		bundlePath := filepath.Clean(pluginInfo.Manifest.Webapp.BundlePath)
 		if bundlePath == "" || bundlePath[0] == '.' {
-			return fmt.Errorf("invalid webapp bundle path")
+			return nil, false, fmt.Errorf("invalid webapp bundle path")
 		}
 		bundlePath = filepath.Join(env.pluginDir, id, bundlePath)
 		destinationPath := filepath.Join(env.webappPluginDir, id)
 
 		if err := os.RemoveAll(destinationPath); err != nil {
-			return errors.Wrapf(err, "unable to remove old webapp bundle directory: %v", destinationPath)
+			return nil, false, errors.Wrapf(err, "unable to remove old webapp bundle directory: %v", destinationPath)
 		}
 
 		if err := utils.CopyDir(filepath.Dir(bundlePath), destinationPath); err != nil {
-			return errors.Wrapf(err, "unable to copy webapp bundle directory: %v", id)
+			return nil, false, errors.Wrapf(err, "unable to copy webapp bundle directory: %v", id)
 		}
 
+		sourceBundleFilepath := filepath.Join(destinationPath, filepath.Base(bundlePath))
+
+		sourceBundleFileContents, err := ioutil.ReadFile(sourceBundleFilepath)
+		if err != nil {
+			return nil, false, errors.Wrapf(err, "unable to read webapp bundle: %v", id)
+		}
+
+		hash := fnv.New64a()
+		hash.Write(sourceBundleFileContents)
+		pluginInfo.Manifest.Webapp.BundleHash = hash.Sum([]byte{})
+
 		if err := os.Rename(
-			filepath.Join(destinationPath, filepath.Base(bundlePath)),
-			filepath.Join(destinationPath, fmt.Sprintf("%s_bundle.js", id)),
+			sourceBundleFilepath,
+			filepath.Join(destinationPath, fmt.Sprintf("%s_%x_bundle.js", id, pluginInfo.Manifest.Webapp.BundleHash)),
 		); err != nil {
-			return errors.Wrapf(err, "unable to rename webapp bundle: %v", id)
+			return nil, false, errors.Wrapf(err, "unable to rename webapp bundle: %v", id)
 		}
 	}
 
 	if pluginInfo.Manifest.HasServer() {
 		supervisor, err := newSupervisor(pluginInfo, env.logger, env.newAPIImpl(pluginInfo.Manifest))
 		if err != nil {
-			return errors.Wrapf(err, "unable to start plugin: %v", id)
+			return nil, false, errors.Wrapf(err, "unable to start plugin: %v", id)
 		}
 		activePlugin.supervisor = supervisor
 	}
 
-	return nil
+	return pluginInfo.Manifest, true, nil
 }
 
 // Deactivates the plugin with the given id.
-func (env *Environment) Deactivate(id string) {
+func (env *Environment) Deactivate(id string) bool {
 	p, ok := env.activePlugins.Load(id)
 	if !ok {
-		return
+		return false
 	}
 
 	env.activePlugins.Delete(id)
@@ -219,6 +229,8 @@ func (env *Environment) Deactivate(id string) {
 		}
 		activePlugin.supervisor.Shutdown()
 	}
+
+	return true
 }
 
 // Shutdown deactivates all plugins and gracefully shuts down the environment.

diff --git a/web/static.go b/web/static.go
@@ -29,8 +29,8 @@ func (w *Web) InitStatic() {
 
 		mime.AddExtensionType(".wasm", "application/wasm")
 
-		staticHandler := staticHandler(http.StripPrefix(path.Join(subpath, "static"), http.FileServer(http.Dir(staticDir))))
-		pluginHandler := pluginHandler(w.App.Config, http.StripPrefix(path.Join(subpath, "static", "plugins"), http.FileServer(http.Dir(*w.App.Config().PluginSettings.ClientDirectory))))
+		staticHandler := staticFilesHandler(http.StripPrefix(path.Join(subpath, "static"), http.FileServer(http.Dir(staticDir))))
+		pluginHandler := staticFilesHandler(http.StripPrefix(path.Join(subpath, "static", "plugins"), http.FileServer(http.Dir(*w.App.Config().PluginSettings.ClientDirectory))))
 
 		if *w.App.Config().ServiceSettings.WebserverMode == "gzip" {
 			staticHandler = gziphandler.GzipHandler(staticHandler)
@@ -72,7 +72,7 @@ func root(c *Context, w http.ResponseWriter, r *http.Request) {
 	http.ServeFile(w, r, filepath.Join(staticDir, "root.html"))
 }
 
-func staticHandler(handler http.Handler) http.Handler {
+func staticFilesHandler(handler http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Cache-Control", "max-age=31556926, public")
 		if strings.HasSuffix(r.URL.Path, "/") {
@@ -82,18 +82,3 @@ func staticHandler(handler http.Handler) http.Handler {
 		handler.ServeHTTP(w, r)
 	})
 }
-
-func pluginHandler(config model.ConfigFunc, handler http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if *config().ServiceSettings.EnableDeveloper {
-			w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate")
-		} else {
-			w.Header().Set("Cache-Control", "max-age=31556926, public")
-		}
-		if strings.HasSuffix(r.URL.Path, "/") {
-			http.NotFound(w, r)
-			return
-		}
-		handler.ServeHTTP(w, r)
-	})
-}