From 345b0c560a78f3b4484ca4acb5de1542035558f6 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Thu, 5 Dec 2019 23:57:30 -0400 Subject: [PATCH] MM-20948: nocache for static plugin assets (#13322) Serve static plugin assets with a `Cache-Control: no-cache, public` header. This avoids caching a 404 response for such an asset, preventing it from being loaded until expiry even if the file later becomes available. This is currently preventing updates of plugins on community and would generally affect any customer with a cache in front of the Mattermost servers. Fixes: https://mattermost.atlassian.net/browse/MM-20948 --- web/static.go | 17 +++++++++- web/web_test.go | 87 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/web/static.go b/web/static.go index 5ecb60e16e..bf44044825 100644 --- a/web/static.go +++ b/web/static.go @@ -34,7 +34,7 @@ func (w *Web) InitStatic() { mime.AddExtensionType(".wasm", "application/wasm") 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.ConfigService.Config().PluginSettings.ClientDirectory)))) + pluginHandler := staticFilesWithValidationHandler(http.StripPrefix(path.Join(subpath, "static", "plugins"), http.FileServer(http.Dir(*w.ConfigService.Config().PluginSettings.ClientDirectory)))) if *w.ConfigService.Config().ServiceSettings.WebserverMode == "gzip" { staticHandler = gziphandler.GzipHandler(staticHandler) @@ -78,6 +78,21 @@ func root(c *Context, w http.ResponseWriter, r *http.Request) { 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, "/") { + http.NotFound(w, r) + return + } + handler.ServeHTTP(w, r) + }) +} + +func staticFilesWithValidationHandler(handler http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Require validation from any cache, achieved via Last-Modified and the + // http.FileServer. + w.Header().Set("Cache-Control", "no-cache, public") + if strings.HasSuffix(r.URL.Path, "/") { http.NotFound(w, r) return diff --git a/web/web_test.go b/web/web_test.go index 9dc5244070..7d7dc71031 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "testing" + "time" "github.com/mattermost/mattermost-server/v5/app" "github.com/mattermost/mattermost-server/v5/config" @@ -128,6 +129,92 @@ func (th *TestHelper) TearDown() { } } +func TestStaticFilesRequest(t *testing.T) { + th := Setup().InitPlugins() + defer th.TearDown() + + pluginID := "com.mattermost.sample" + + // Setup the directory directly in the plugin working path. + pluginDir := filepath.Join(*th.App.Config().PluginSettings.Directory, pluginID) + err := os.MkdirAll(pluginDir, 0777) + require.NoError(t, err) + pluginDir, err = filepath.Abs(pluginDir) + require.NoError(t, err) + + // Compile the backend + backend := filepath.Join(pluginDir, "backend.exe") + pluginCode := ` + package main + + import ( + "github.com/mattermost/mattermost-server/v5/plugin" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } +` + utils.CompileGo(t, pluginCode, backend) + + // Write out the frontend + mainJS := `var x = alert();` + mainJSPath := filepath.Join(pluginDir, "main.js") + require.NoError(t, err) + err = ioutil.WriteFile(mainJSPath, []byte(mainJS), 0777) + require.NoError(t, err) + + // Write the plugin.json manifest + pluginManifest := `{"id": "com.mattermost.sample", "server": {"executable": "backend.exe"}, "webapp": {"bundle_path":"main.js"}, "settings_schema": {"settings": []}}` + ioutil.WriteFile(filepath.Join(pluginDir, "plugin.json"), []byte(pluginManifest), 0600) + + // Activate the plugin + manifest, activated, reterr := th.App.GetPluginsEnvironment().Activate(pluginID) + require.Nil(t, reterr) + require.NotNil(t, manifest) + require.True(t, activated) + + // Verify access to the bundle with requisite headers + req, _ := http.NewRequest("GET", "/static/plugins/com.mattermost.sample/com.mattermost.sample_724ed0e2ebb2b841_bundle.js", nil) + res := httptest.NewRecorder() + th.Web.MainRouter.ServeHTTP(res, req) + assert.Equal(t, http.StatusOK, res.Code) + assert.Equal(t, mainJS, res.Body.String()) + assert.Equal(t, []string{"no-cache, public"}, res.Result().Header[http.CanonicalHeaderKey("Cache-Control")]) + + // Verify cached access to the bundle with an If-Modified-Since timestamp in the future + future := time.Now().Add(24 * time.Hour) + req, _ = http.NewRequest("GET", "/static/plugins/com.mattermost.sample/com.mattermost.sample_724ed0e2ebb2b841_bundle.js", nil) + req.Header.Add("If-Modified-Since", future.Format(time.RFC850)) + res = httptest.NewRecorder() + th.Web.MainRouter.ServeHTTP(res, req) + assert.Equal(t, http.StatusNotModified, res.Code) + assert.Empty(t, res.Body.String()) + assert.Equal(t, []string{"no-cache, public"}, res.Result().Header[http.CanonicalHeaderKey("Cache-Control")]) + + // Verify access to the bundle with an If-Modified-Since timestamp in the past + past := time.Now().Add(-24 * time.Hour) + req, _ = http.NewRequest("GET", "/static/plugins/com.mattermost.sample/com.mattermost.sample_724ed0e2ebb2b841_bundle.js", nil) + req.Header.Add("If-Modified-Since", past.Format(time.RFC850)) + res = httptest.NewRecorder() + th.Web.MainRouter.ServeHTTP(res, req) + assert.Equal(t, http.StatusOK, res.Code) + assert.Equal(t, mainJS, res.Body.String()) + assert.Equal(t, []string{"no-cache, public"}, res.Result().Header[http.CanonicalHeaderKey("Cache-Control")]) + + // Verify handling of 404. + req, _ = http.NewRequest("GET", "/static/plugins/com.mattermost.sample/404.js", nil) + res = httptest.NewRecorder() + th.Web.MainRouter.ServeHTTP(res, req) + assert.Equal(t, http.StatusNotFound, res.Code) + assert.Equal(t, "404 page not found\n", res.Body.String()) + assert.Equal(t, []string{"no-cache, public"}, res.Result().Header[http.CanonicalHeaderKey("Cache-Control")]) +} + func TestPublicFilesRequest(t *testing.T) { th := Setup().InitPlugins() defer th.TearDown()