diff --git a/services/imageproxy/local.go b/services/imageproxy/local.go index f79a1424b8..9584ee9c11 100644 --- a/services/imageproxy/local.go +++ b/services/imageproxy/local.go @@ -5,11 +5,14 @@ package imageproxy import ( "errors" + "fmt" "io" "io/ioutil" + "mime" "net/http" "net/http/httptest" "net/url" + "path/filepath" "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/services/httpservice" @@ -66,6 +69,24 @@ func makeLocalBackend(proxy *ImageProxy) *LocalBackend { } } +type contentTypeRecorder struct { + http.ResponseWriter + filename string +} + +func (rec *contentTypeRecorder) WriteHeader(code int) { + hdr := rec.ResponseWriter.Header() + contentType := hdr.Get("Content-Type") + mediaType, _, err := mime.ParseMediaType(contentType) + // The error is caused by a malformed input and there's not much use logging it. + // Therefore, even in the error case we set it to attachment mode to be safe. + if err != nil || mediaType == "image/svg+xml" { + hdr.Set("Content-Disposition", fmt.Sprintf("attachment;filename=%q", rec.filename)) + } + + rec.ResponseWriter.WriteHeader(code) +} + func (backend *LocalBackend) GetImage(w http.ResponseWriter, r *http.Request, imageURL string) { // The interface to the proxy only exposes a ServeHTTP method, so fake a request to it req, err := http.NewRequest(http.MethodGet, "/"+imageURL, nil) @@ -78,12 +99,21 @@ func (backend *LocalBackend) GetImage(w http.ResponseWriter, r *http.Request, im return } + u, err := url.Parse(imageURL) + if err != nil { + mlog.Error("Failed to parse URL for proxied image", mlog.String("url", imageURL), mlog.Err(err)) + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte{}) + return + } + w.Header().Set("X-Frame-Options", "deny") w.Header().Set("X-XSS-Protection", "1; mode=block") w.Header().Set("X-Content-Type-Options", "nosniff") w.Header().Set("Content-Security-Policy", "default-src 'none'; img-src data:; style-src 'unsafe-inline'") - backend.impl.ServeHTTP(w, req) + rec := contentTypeRecorder{w, filepath.Base(u.Path)} + backend.impl.ServeHTTP(&rec, req) } func (backend *LocalBackend) GetImageDirect(imageURL string) (io.ReadCloser, string, error) { diff --git a/services/imageproxy/local_test.go b/services/imageproxy/local_test.go index 34d95b2163..1837777a7e 100644 --- a/services/imageproxy/local_test.go +++ b/services/imageproxy/local_test.go @@ -164,6 +164,34 @@ func TestLocalBackend_GetImage(t *testing.T) { wait <- true }) + + t.Run("SVG attachment", func(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "max-age=2592000, private") + w.Header().Set("Content-Type", "image/svg+xml") + w.Header().Set("Content-Length", "10") + + w.WriteHeader(http.StatusOK) + w.Write([]byte("1111111111")) + }) + + mock := httptest.NewServer(handler) + defer mock.Close() + + proxy := makeTestLocalProxy() + + recorder := httptest.NewRecorder() + request, err := http.NewRequest(http.MethodGet, "", nil) + require.NoError(t, err) + proxy.GetImage(recorder, request, mock.URL+"/test.svg") + resp := recorder.Result() + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "attachment;filename=\"test.svg\"", resp.Header.Get("Content-Disposition")) + + _, err = ioutil.ReadAll(resp.Body) + require.NoError(t, err) + }) } func TestLocalBackend_GetImageDirect(t *testing.T) {