summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/plugin.go73
-rw-r--r--i18n/en.json6
-rw-r--r--mlog/log.go1
-rw-r--r--model/manifest.go6
-rw-r--r--plugin/pluginenv/environment.go6
-rw-r--r--plugin/pluginenv/environment_test.go2
-rw-r--r--plugin/rpcplugin/hooks.go10
-rw-r--r--plugin/rpcplugin/hooks_test.go2
-rw-r--r--plugin/rpcplugin/main.go4
-rw-r--r--plugin/rpcplugin/main_test.go12
-rw-r--r--plugin/rpcplugin/rpcplugintest/supervisor.go83
-rw-r--r--plugin/rpcplugin/sandbox/main_test.go18
-rw-r--r--plugin/rpcplugin/supervisor.go26
-rw-r--r--plugin/valid.go32
-rw-r--r--plugin/valid_test.go32
15 files changed, 256 insertions, 57 deletions
diff --git a/app/plugin.go b/app/plugin.go
index 3da9cea40..0aaa8d1d4 100644
--- a/app/plugin.go
+++ b/app/plugin.go
@@ -15,7 +15,6 @@ import (
"os"
"path/filepath"
"strings"
- "unicode/utf8"
"github.com/gorilla/mux"
"github.com/mattermost/mattermost-server/mlog"
@@ -33,10 +32,6 @@ import (
"github.com/mattermost/mattermost-server/plugin/rpcplugin/sandbox"
)
-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,
@@ -47,7 +42,7 @@ func (a *App) initBuiltInPlugins() {
"ldapextras": &ldapextras.Plugin{},
}
for id, p := range plugins {
- mlog.Debug("Initializing built-in plugin: " + id)
+ mlog.Debug("Initializing built-in plugin", mlog.String("plugin_id", id))
api := &BuiltInPluginAPI{
id: id,
router: a.Srv.Router.PathPrefix("/plugins/" + id).Subrouter(),
@@ -65,21 +60,23 @@ func (a *App) initBuiltInPlugins() {
}
}
-// ActivatePlugins will activate any plugins enabled in the config
-// and deactivate all other plugins.
-func (a *App) ActivatePlugins() {
+func (a *App) setPluginsActive(activate bool) {
if a.PluginEnv == nil {
- mlog.Error("plugin env not initialized")
+ mlog.Error(fmt.Sprintf("Cannot setPluginsActive(%t): plugin env not initialized", activate))
return
}
plugins, err := a.PluginEnv.Plugins()
if err != nil {
- mlog.Error("failed to activate plugins: " + err.Error())
+ mlog.Error(fmt.Sprintf("Cannot setPluginsActive(%t)", activate), mlog.Err(err))
return
}
for _, plugin := range plugins {
+ if plugin.Manifest == nil {
+ continue
+ }
+
id := plugin.Manifest.Id
pluginState := &model.PluginState{Enable: false}
@@ -89,15 +86,14 @@ func (a *App) ActivatePlugins() {
active := a.PluginEnv.IsPluginActive(id)
- if pluginState.Enable && !active {
+ if activate && pluginState.Enable && !active {
if err := a.activatePlugin(plugin.Manifest); err != nil {
- mlog.Error(fmt.Sprintf("%v plugin enabled in config.json but failing to activate err=%v", plugin.Manifest.Id, err.DetailedError))
- continue
+ mlog.Error("Plugin failed to activate", mlog.String("plugin_id", plugin.Manifest.Id), mlog.String("err", err.DetailedError))
}
- } else if !pluginState.Enable && active {
+ } else if (!activate || !pluginState.Enable) && active {
if err := a.deactivatePlugin(plugin.Manifest); err != nil {
- mlog.Error(err.Error())
+ mlog.Error("Plugin failed to deactivate", mlog.String("plugin_id", plugin.Manifest.Id), mlog.String("err", err.DetailedError))
}
}
}
@@ -114,13 +110,13 @@ func (a *App) activatePlugin(manifest *model.Manifest) *model.AppError {
a.Publish(message)
}
- mlog.Info(fmt.Sprintf("Activated %v plugin", manifest.Id))
+ mlog.Info("Activated plugin", mlog.String("plugin_id", manifest.Id))
return nil
}
func (a *App) deactivatePlugin(manifest *model.Manifest) *model.AppError {
if err := a.PluginEnv.DeactivatePlugin(manifest.Id); err != nil {
- return model.NewAppError("removePlugin", "app.plugin.deactivate.app_error", nil, err.Error(), http.StatusBadRequest)
+ return model.NewAppError("deactivatePlugin", "app.plugin.deactivate.app_error", nil, err.Error(), http.StatusBadRequest)
}
a.UnregisterPluginCommands(manifest.Id)
@@ -131,7 +127,7 @@ func (a *App) deactivatePlugin(manifest *model.Manifest) *model.AppError {
a.Publish(message)
}
- mlog.Info(fmt.Sprintf("Deactivated %v plugin", manifest.Id))
+ mlog.Info("Deactivated plugin", mlog.String("plugin_id", manifest.Id))
return nil
}
@@ -174,8 +170,8 @@ 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)
+ if !plugin.IsValidId(manifest.Id) {
+ return nil, model.NewAppError("installPlugin", "app.plugin.invalid_id.app_error", map[string]interface{}{"Min": plugin.MinIdLength, "Max": plugin.MaxIdLength, "Regex": plugin.ValidId.String()}, "", http.StatusBadRequest)
}
bundles, err := a.PluginEnv.Plugins()
@@ -184,7 +180,7 @@ func (a *App) installPlugin(pluginFile io.Reader, allowPrepackaged bool) (*model
}
for _, bundle := range bundles {
- if bundle.Manifest.Id == manifest.Id {
+ if bundle.Manifest != nil && bundle.Manifest.Id == manifest.Id {
return nil, model.NewAppError("installPlugin", "app.plugin.install_id.app_error", nil, "", http.StatusBadRequest)
}
}
@@ -211,6 +207,10 @@ func (a *App) GetPlugins() (*model.PluginsResponse, *model.AppError) {
resp := &model.PluginsResponse{Active: []*model.PluginInfo{}, Inactive: []*model.PluginInfo{}}
for _, plugin := range plugins {
+ if plugin.Manifest == nil {
+ continue
+ }
+
info := &model.PluginInfo{
Manifest: *plugin.Manifest,
}
@@ -259,9 +259,11 @@ func (a *App) removePlugin(id string, allowPrepackaged bool) *model.AppError {
}
var manifest *model.Manifest
+ var pluginPath string
for _, p := range plugins {
- if p.Manifest.Id == id {
+ if p.Manifest != nil && p.Manifest.Id == id {
manifest = p.Manifest
+ pluginPath = filepath.Dir(p.ManifestPath)
break
}
}
@@ -277,7 +279,7 @@ func (a *App) removePlugin(id string, allowPrepackaged bool) *model.AppError {
}
}
- err = os.RemoveAll(filepath.Join(a.PluginEnv.SearchPath(), id))
+ err = os.RemoveAll(pluginPath)
if err != nil {
return model.NewAppError("removePlugin", "app.plugin.remove.app_error", nil, err.Error(), http.StatusInternalServerError)
}
@@ -372,12 +374,12 @@ func (a *App) InitPlugins(pluginPath, webappPath string, supervisorOverride plug
mlog.Info("Starting up plugins")
if err := os.Mkdir(pluginPath, 0744); err != nil && !os.IsExist(err) {
- mlog.Error("failed to start up plugins: " + err.Error())
+ mlog.Error("Failed to start up plugins", mlog.Err(err))
return
}
if err := os.Mkdir(webappPath, 0744); err != nil && !os.IsExist(err) {
- mlog.Error("failed to start up plugins: " + err.Error())
+ mlog.Error("Failed to start up plugins", mlog.Err(err))
return
}
@@ -399,15 +401,14 @@ func (a *App) InitPlugins(pluginPath, webappPath string, supervisorOverride plug
if supervisorOverride != nil {
options = append(options, pluginenv.SupervisorProvider(supervisorOverride))
} else if err := sandbox.CheckSupport(); err != nil {
- mlog.Warn(err.Error())
- mlog.Warn("plugin sandboxing is not supported. plugins will run with the same access level as the server. See documentation to learn more: https://developers.mattermost.com/extend/plugins/security/")
+ mlog.Warn("plugin sandboxing is not supported. plugins will run with the same access level as the server. See documentation to learn more: https://developers.mattermost.com/extend/plugins/security/", mlog.Err(err))
options = append(options, pluginenv.SupervisorProvider(rpcplugin.SupervisorProvider))
} else {
options = append(options, pluginenv.SupervisorProvider(sandbox.SupervisorProvider))
}
if env, err := pluginenv.New(options...); err != nil {
- mlog.Error("failed to start up plugins: " + err.Error())
+ mlog.Error("Failed to start up plugins", mlog.Err(err))
return
} else {
a.PluginEnv = env
@@ -415,36 +416,34 @@ func (a *App) InitPlugins(pluginPath, webappPath string, supervisorOverride plug
for id, asset := range prepackagedPlugins {
if tarball, err := asset("plugin.tar.gz"); err != nil {
- mlog.Error("failed to install prepackaged plugin: " + err.Error())
+ mlog.Error("Failed to install prepackaged plugin", mlog.Err(err))
} else if tarball != nil {
a.removePlugin(id, true)
if _, err := a.installPlugin(bytes.NewReader(tarball), true); err != nil {
- mlog.Error("failed to install prepackaged plugin: " + err.Error())
+ mlog.Error("Failed to install prepackaged plugin", mlog.Err(err))
}
if _, ok := a.Config().PluginSettings.PluginStates[id]; !ok && id != "zoom" {
if err := a.EnablePlugin(id); err != nil {
- mlog.Error("failed to enable prepackaged plugin: " + err.Error())
+ mlog.Error("Failed to enable prepackaged plugin", mlog.Err(err))
}
}
}
}
a.RemoveConfigListener(a.PluginConfigListenerId)
- a.PluginConfigListenerId = a.AddConfigListener(func(prevCfg, cfg *model.Config) {
+ a.PluginConfigListenerId = a.AddConfigListener(func(_, cfg *model.Config) {
if a.PluginEnv == nil {
return
}
- if *prevCfg.PluginSettings.Enable && *cfg.PluginSettings.Enable {
- a.ActivatePlugins()
- }
+ a.setPluginsActive(*cfg.PluginSettings.Enable)
for _, err := range a.PluginEnv.Hooks().OnConfigurationChange() {
mlog.Error(err.Error())
}
})
- a.ActivatePlugins()
+ a.setPluginsActive(true)
}
func (a *App) ServePluginRequest(w http.ResponseWriter, r *http.Request) {
diff --git a/i18n/en.json b/i18n/en.json
index 0afaa326c..d4a08b07a 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -3812,7 +3812,7 @@
},
{
"id": "app.plugin.activate.app_error",
- "translation": "Unable to activate extracted plugin. Plugin may already exist and be activated."
+ "translation": "Unable to activate extracted plugin."
},
{
"id": "app.plugin.cluster.save_config.app_error",
@@ -3847,8 +3847,8 @@
"translation": "Unable to get active plugins"
},
{
- "id": "app.plugin.id_length.app_error",
- "translation": "Plugin Id must be less than {{.Max}} characters."
+ "id": "app.plugin.invalid_id.app_error",
+ "translation": "Plugin Id must be at least {{.Min}} characters, at most {{.Max}} characters and match {{.Regex}}."
},
{
"id": "app.plugin.install.app_error",
diff --git a/mlog/log.go b/mlog/log.go
index 22db0c7b1..ad537a11d 100644
--- a/mlog/log.go
+++ b/mlog/log.go
@@ -29,6 +29,7 @@ type Field = zapcore.Field
var Int64 = zap.Int64
var Int = zap.Int
var String = zap.String
+var Err = zap.Error
type LoggerConfiguration struct {
EnableConsole bool
diff --git a/model/manifest.go b/model/manifest.go
index 32d4341cd..d6a064d4e 100644
--- a/model/manifest.go
+++ b/model/manifest.go
@@ -93,9 +93,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. Ids are limited
- // to 190 characters. Reverse-DNS notation using a name you control is a good option.
- // For example, "com.mycompany.myplugin".
+ // The id is a globally unique identifier that represents your plugin. Ids must be at least
+ // 3 characters, at most 190 characters and must match ^[a-zA-Z0-9-_\.]+$.
+ // Reverse-DNS notation using a name you control is a good option, e.g. "com.mycompany.myplugin".
Id string `json:"id" yaml:"id"`
// The name to be displayed for the plugin.
diff --git a/plugin/pluginenv/environment.go b/plugin/pluginenv/environment.go
index adc9ddbde..adc02e885 100644
--- a/plugin/pluginenv/environment.go
+++ b/plugin/pluginenv/environment.go
@@ -112,8 +112,12 @@ func (env *Environment) ActivatePlugin(id string) error {
env.mutex.Lock()
defer env.mutex.Unlock()
+ if !plugin.IsValidId(id) {
+ return fmt.Errorf("invalid plugin id: %s", id)
+ }
+
if _, ok := env.activePlugins[id]; ok {
- return fmt.Errorf("plugin already active: %v", id)
+ return nil
}
plugins, err := ScanSearchPath(env.searchPath)
if err != nil {
diff --git a/plugin/pluginenv/environment_test.go b/plugin/pluginenv/environment_test.go
index 2a52b3830..91d639f69 100644
--- a/plugin/pluginenv/environment_test.go
+++ b/plugin/pluginenv/environment_test.go
@@ -149,7 +149,7 @@ func TestEnvironment(t *testing.T) {
assert.Equal(t, env.ActivePluginIds(), []string{"foo"})
activePlugins = env.ActivePlugins()
assert.Len(t, activePlugins, 1)
- assert.Error(t, env.ActivatePlugin("foo"))
+ assert.NoError(t, env.ActivatePlugin("foo"))
assert.True(t, env.IsPluginActive("foo"))
hooks.On("OnDeactivate").Return(nil)
diff --git a/plugin/rpcplugin/hooks.go b/plugin/rpcplugin/hooks.go
index 7b44d0de7..90734fd1c 100644
--- a/plugin/rpcplugin/hooks.go
+++ b/plugin/rpcplugin/hooks.go
@@ -11,6 +11,7 @@ import (
"net/rpc"
"reflect"
+ "github.com/mattermost/mattermost-server/mlog"
"github.com/mattermost/mattermost-server/model"
"github.com/mattermost/mattermost-server/plugin"
)
@@ -165,6 +166,7 @@ type RemoteHooks struct {
muxer *Muxer
apiCloser io.Closer
implemented [maxRemoteHookCount]bool
+ pluginId string
}
var _ plugin.Hooks = (*RemoteHooks)(nil)
@@ -237,6 +239,7 @@ func (h *RemoteHooks) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Request: forwardedRequest,
RequestBodyStream: requestBodyStream,
}, nil); err != nil {
+ mlog.Error("Plugin failed to ServeHTTP", mlog.String("plugin_id", h.pluginId), mlog.Err(err))
http.Error(w, "500 internal server error", http.StatusInternalServerError)
}
}
@@ -260,10 +263,11 @@ func (h *RemoteHooks) Close() error {
return h.client.Close()
}
-func ConnectHooks(conn io.ReadWriteCloser, muxer *Muxer) (*RemoteHooks, error) {
+func ConnectHooks(conn io.ReadWriteCloser, muxer *Muxer, pluginId string) (*RemoteHooks, error) {
remote := &RemoteHooks{
- client: rpc.NewClient(conn),
- muxer: muxer,
+ client: rpc.NewClient(conn),
+ muxer: muxer,
+ pluginId: pluginId,
}
implemented, err := remote.Implemented()
if err != nil {
diff --git a/plugin/rpcplugin/hooks_test.go b/plugin/rpcplugin/hooks_test.go
index 116038dae..c404442b7 100644
--- a/plugin/rpcplugin/hooks_test.go
+++ b/plugin/rpcplugin/hooks_test.go
@@ -31,7 +31,7 @@ func testHooksRPC(hooks interface{}, f func(*RemoteHooks)) error {
id, server := c1.Serve()
go ServeHooks(hooks, server, c1)
- remote, err := ConnectHooks(c2.Connect(id), c2)
+ remote, err := ConnectHooks(c2.Connect(id), c2, "plugin_id")
if err != nil {
return err
}
diff --git a/plugin/rpcplugin/main.go b/plugin/rpcplugin/main.go
index 96a61c068..efb880605 100644
--- a/plugin/rpcplugin/main.go
+++ b/plugin/rpcplugin/main.go
@@ -30,7 +30,7 @@ func Main(hooks interface{}) {
}
// Returns the hooks being served by a call to Main.
-func ConnectMain(muxer *Muxer) (*RemoteHooks, error) {
+func ConnectMain(muxer *Muxer, pluginId string) (*RemoteHooks, error) {
buf := make([]byte, 1)
if _, err := muxer.Read(buf); err != nil {
return nil, err
@@ -43,5 +43,5 @@ func ConnectMain(muxer *Muxer) (*RemoteHooks, error) {
return nil, err
}
- return ConnectHooks(muxer.Connect(id), muxer)
+ return ConnectHooks(muxer.Connect(id), muxer, pluginId)
}
diff --git a/plugin/rpcplugin/main_test.go b/plugin/rpcplugin/main_test.go
index 6cdd46df0..06423106c 100644
--- a/plugin/rpcplugin/main_test.go
+++ b/plugin/rpcplugin/main_test.go
@@ -10,11 +10,21 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
+ "github.com/mattermost/mattermost-server/mlog"
"github.com/mattermost/mattermost-server/plugin/plugintest"
"github.com/mattermost/mattermost-server/plugin/rpcplugin/rpcplugintest"
)
func TestMain(t *testing.T) {
+ // Setup a global logger to catch tests logging outside of app context
+ // The global logger will be stomped by apps initalizing but that's fine for testing. Ideally this won't happen.
+ mlog.InitGlobalLogger(mlog.NewLogger(&mlog.LoggerConfiguration{
+ EnableConsole: true,
+ ConsoleJson: true,
+ ConsoleLevel: "error",
+ EnableFile: false,
+ }))
+
dir, err := ioutil.TempDir("", "")
require.NoError(t, err)
defer os.RemoveAll(dir)
@@ -46,7 +56,7 @@ func TestMain(t *testing.T) {
var api plugintest.API
- hooks, err := ConnectMain(muxer)
+ hooks, err := ConnectMain(muxer, "plugin_id")
require.NoError(t, err)
assert.NoError(t, hooks.OnActivate(&api))
assert.NoError(t, hooks.OnDeactivate())
diff --git a/plugin/rpcplugin/rpcplugintest/supervisor.go b/plugin/rpcplugin/rpcplugintest/supervisor.go
index 05dc8ed8f..2ae065621 100644
--- a/plugin/rpcplugin/rpcplugintest/supervisor.go
+++ b/plugin/rpcplugin/rpcplugintest/supervisor.go
@@ -7,6 +7,8 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
+ "net/http"
+ "net/http/httptest"
"os"
"path/filepath"
"testing"
@@ -30,6 +32,7 @@ func TestSupervisorProvider(t *testing.T, sp SupervisorProviderFunc) {
"Supervisor_NonExistentExecutablePath": testSupervisor_NonExistentExecutablePath,
"Supervisor_StartTimeout": testSupervisor_StartTimeout,
"Supervisor_PluginCrash": testSupervisor_PluginCrash,
+ "Supervisor_PluginRepeatedlyCrash": testSupervisor_PluginRepeatedlyCrash,
} {
t.Run(name, func(t *testing.T) { f(t, sp) })
}
@@ -188,3 +191,83 @@ func testSupervisor_PluginCrash(t *testing.T, sp SupervisorProviderFunc) {
assert.True(t, recovered)
require.NoError(t, supervisor.Stop())
}
+
+// Crashed plugins should be relaunched at most three times.
+func testSupervisor_PluginRepeatedlyCrash(t *testing.T, sp SupervisorProviderFunc) {
+ dir, err := ioutil.TempDir("", "")
+ require.NoError(t, err)
+ defer os.RemoveAll(dir)
+
+ backend := filepath.Join(dir, "backend.exe")
+ CompileGo(t, `
+ package main
+
+ import (
+ "net/http"
+ "os"
+
+ "github.com/mattermost/mattermost-server/plugin/rpcplugin"
+ )
+
+ type MyPlugin struct {
+ crashing bool
+ }
+
+ func (p *MyPlugin) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+ if r.Method == http.MethodPost {
+ p.crashing = true
+ go func() {
+ os.Exit(1)
+ }()
+ }
+
+ if p.crashing {
+ w.WriteHeader(http.StatusInternalServerError)
+ } else {
+ w.WriteHeader(http.StatusOK)
+ }
+ }
+
+ func main() {
+ rpcplugin.Main(&MyPlugin{})
+ }
+ `, backend)
+
+ ioutil.WriteFile(filepath.Join(dir, "plugin.json"), []byte(`{"id": "foo", "backend": {"executable": "backend.exe"}}`), 0600)
+
+ var api plugintest.API
+ bundle := model.BundleInfoForPath(dir)
+ supervisor, err := sp(bundle)
+ require.NoError(t, err)
+ require.NoError(t, supervisor.Start(&api))
+
+ for attempt := 1; attempt <= 4; attempt++ {
+ // Verify that the plugin is operational
+ response := httptest.NewRecorder()
+ supervisor.Hooks().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/plugins/id", nil))
+ require.Equal(t, http.StatusOK, response.Result().StatusCode)
+
+ // Crash the plugin
+ supervisor.Hooks().ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/plugins/id", nil))
+
+ // Wait for it to potentially recover
+ recovered := false
+ for i := 0; i < 125; i++ {
+ response := httptest.NewRecorder()
+ supervisor.Hooks().ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/plugins/id", nil))
+ if response.Result().StatusCode == http.StatusOK {
+ recovered = true
+ break
+ }
+
+ time.Sleep(time.Millisecond * 100)
+ }
+
+ if attempt < 4 {
+ require.True(t, recovered, "failed to recover after attempt %d", attempt)
+ } else {
+ require.False(t, recovered, "unexpectedly recovered after attempt %d", attempt)
+ }
+ }
+ require.NoError(t, supervisor.Stop())
+}
diff --git a/plugin/rpcplugin/sandbox/main_test.go b/plugin/rpcplugin/sandbox/main_test.go
new file mode 100644
index 000000000..4be4a42af
--- /dev/null
+++ b/plugin/rpcplugin/sandbox/main_test.go
@@ -0,0 +1,18 @@
+package sandbox
+
+import (
+ "testing"
+
+ "github.com/mattermost/mattermost-server/mlog"
+)
+
+func TestMain(t *testing.T) {
+ // Setup a global logger to catch tests logging outside of app context
+ // The global logger will be stomped by apps initalizing but that's fine for testing. Ideally this won't happen.
+ mlog.InitGlobalLogger(mlog.NewLogger(&mlog.LoggerConfiguration{
+ EnableConsole: true,
+ ConsoleJson: true,
+ ConsoleLevel: "error",
+ EnableFile: false,
+ }))
+}
diff --git a/plugin/rpcplugin/supervisor.go b/plugin/rpcplugin/supervisor.go
index 6a48cb5e8..6e26d5682 100644
--- a/plugin/rpcplugin/supervisor.go
+++ b/plugin/rpcplugin/supervisor.go
@@ -12,19 +12,26 @@ import (
"sync/atomic"
"time"
+ "github.com/mattermost/mattermost-server/mlog"
"github.com/mattermost/mattermost-server/model"
"github.com/mattermost/mattermost-server/plugin"
)
+const (
+ MaxProcessRestarts = 3
+)
+
// Supervisor implements a plugin.Supervisor that launches the plugin in a separate process and
// communicates via RPC.
//
-// If the plugin unexpectedly exists, the supervisor will relaunch it after a short delay.
+// If the plugin unexpectedly exits, the supervisor will relaunch it after a short delay, but will
+// only restart a plugin at most three times.
type Supervisor struct {
hooks atomic.Value
done chan bool
cancel context.CancelFunc
newProcess func(context.Context) (Process, io.ReadWriteCloser, error)
+ pluginId string
}
var _ plugin.Supervisor = (*Supervisor)(nil)
@@ -66,19 +73,28 @@ func (s *Supervisor) run(ctx context.Context, start chan<- error, api plugin.API
s.done <- true
}()
done := ctx.Done()
- for {
+ for i := 0; i <= MaxProcessRestarts; i++ {
s.runPlugin(ctx, start, api)
select {
case <-done:
return
default:
start = nil
- time.Sleep(time.Second)
+ if i < MaxProcessRestarts {
+ mlog.Debug("Plugin terminated unexpectedly", mlog.String("plugin_id", s.pluginId))
+ time.Sleep(time.Duration((1 + i*i)) * time.Second)
+ } else {
+ mlog.Debug("Plugin terminated unexpectedly too many times", mlog.String("plugin_id", s.pluginId), mlog.Int("max_process_restarts", MaxProcessRestarts))
+ }
}
}
}
func (s *Supervisor) runPlugin(ctx context.Context, start chan<- error, api plugin.API) error {
+ if start == nil {
+ mlog.Debug("Restarting plugin", mlog.String("plugin_id", s.pluginId))
+ }
+
p, ipc, err := s.newProcess(ctx)
if err != nil {
if start != nil {
@@ -100,7 +116,7 @@ func (s *Supervisor) runPlugin(ctx context.Context, start chan<- error, api plug
muxerClosed <- muxer.Close()
}()
- hooks, err := ConnectMain(muxer)
+ hooks, err := ConnectMain(muxer, s.pluginId)
if err == nil {
err = hooks.OnActivate(api)
}
@@ -147,5 +163,5 @@ func SupervisorWithNewProcessFunc(bundle *model.BundleInfo, newProcess func(cont
if strings.HasPrefix(executable, "..") {
return nil, fmt.Errorf("invalid backend executable")
}
- return &Supervisor{newProcess: newProcess}, nil
+ return &Supervisor{pluginId: bundle.Manifest.Id, newProcess: newProcess}, nil
}
diff --git a/plugin/valid.go b/plugin/valid.go
new file mode 100644
index 000000000..62c594a1a
--- /dev/null
+++ b/plugin/valid.go
@@ -0,0 +1,32 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package plugin
+
+import (
+ "regexp"
+ "unicode/utf8"
+)
+
+const (
+ MinIdLength = 3
+ MaxIdLength = 190
+)
+
+var ValidId *regexp.Regexp
+
+func init() {
+ ValidId = regexp.MustCompile(`^[a-zA-Z0-9-_\.]+$`)
+}
+
+func IsValidId(id string) bool {
+ if utf8.RuneCountInString(id) < MinIdLength {
+ return false
+ }
+
+ if utf8.RuneCountInString(id) > MaxIdLength {
+ return false
+ }
+
+ return ValidId.MatchString(id)
+}
diff --git a/plugin/valid_test.go b/plugin/valid_test.go
new file mode 100644
index 000000000..d47eeb58b
--- /dev/null
+++ b/plugin/valid_test.go
@@ -0,0 +1,32 @@
+package plugin_test
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+
+ "github.com/mattermost/mattermost-server/plugin"
+)
+
+func TestIsValid(t *testing.T) {
+ t.Parallel()
+
+ testCases := map[string]bool{
+ "": false,
+ "a": false,
+ "ab": false,
+ "abc": true,
+ "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij": true,
+ "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1": false,
+ "../path": false,
+ "/etc/passwd": false,
+ "com.mattermost.plugin_with_features-0.9": true,
+ "PLUGINS-THAT-YELL-ARE-OK-2": true,
+ }
+
+ for id, valid := range testCases {
+ t.Run(id, func(t *testing.T) {
+ assert.Equal(t, valid, plugin.IsValidId(id))
+ })
+ }
+}
generated by cgit v1.2.3-1-g7c22 (git 2.25.1) at 2024-05-18 05:57:54 +0000