From 52c92d6659a4e01b3d79f67d2b72c7bd43927913 Mon Sep 17 00:00:00 2001 From: Eli Yukelzon Date: Wed, 1 Apr 2020 11:45:26 +0300 Subject: [PATCH] =?UTF-8?q?MM-23185=20-=20Markdown=20image=20hosted=20by?= =?UTF-8?q?=20plugins=20are=20not=20shown=20if=20l=E2=80=A6=20(#14185)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * avoid image proxy for local images * added test for local images --- api4/image.go | 17 +++++++++++++---- api4/image_test.go | 9 +++++++++ i18n/en.json | 4 ++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/api4/image.go b/api4/image.go index 46006c2bb2..c1087f5e2b 100644 --- a/api4/image.go +++ b/api4/image.go @@ -5,6 +5,9 @@ package api4 import ( "net/http" + "net/url" + + "github.com/mattermost/mattermost-server/v5/model" ) func (api *API) InitImage() { @@ -12,11 +15,17 @@ func (api *API) InitImage() { } func getImage(c *Context, w http.ResponseWriter, r *http.Request) { - url := r.URL.Query().Get("url") + actualURL := r.URL.Query().Get("url") + parsedURL, err := url.Parse(actualURL) + if err != nil { + c.Err = model.NewAppError("getImage", "api.image.get.app_error", nil, err.Error(), http.StatusBadRequest) + return + } - if *c.App.Config().ImageProxySettings.Enable { - c.App.ImageProxy().GetImage(w, r, url) + // in case image proxy is enabled and we are fetching a remote image (NOT static or served by plugins), pass request to proxy + if *c.App.Config().ImageProxySettings.Enable && parsedURL.IsAbs() { + c.App.ImageProxy().GetImage(w, r, actualURL) } else { - http.Redirect(w, r, url, http.StatusFound) + http.Redirect(w, r, actualURL, http.StatusFound) } } diff --git a/api4/image_test.go b/api4/image_test.go index f8e22d9cce..0a6f18cbad 100644 --- a/api4/image_test.go +++ b/api4/image_test.go @@ -91,5 +91,14 @@ func TestGetImage(t *testing.T) { respBody, err := ioutil.ReadAll(resp.Body) require.NoError(t, err) assert.Equal(t, "success", string(respBody)) + + // local images should not be proxied, but forwarded + r, err = http.NewRequest("GET", th.Client.ApiUrl+"/image?url=/plugins/test/image.png", nil) + require.NoError(t, err) + r.Header.Set(model.HEADER_AUTH, th.Client.AuthType+" "+th.Client.AuthToken) + + resp, err = th.Client.HttpClient.Do(r) + require.NoError(t, err) + assert.Equal(t, http.StatusFound, resp.StatusCode) }) } diff --git a/i18n/en.json b/i18n/en.json index 62e2556fc9..8c30fdc356 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1404,6 +1404,10 @@ "id": "api.file.write_file_locally.writing.app_error", "translation": "Encountered an error writing to local server storage." }, + { + "id": "api.image.get.app_error", + "translation": "Requested image url cannot be parsed." + }, { "id": "api.incoming_webhook.disabled.app_error", "translation": "Incoming webhooks have been disabled by the system admin."