MM-14686 Send all image proxy requests through /api/v4/image (#10775)

* MM-14686 Implement /api/v4/image when proxy is disabled

* MM-14686 Send all image proxy requests through /api/v4/image

* Update unit tests
This commit is contained in:
Harrison Healey
2019-05-06 09:22:37 -04:00
committed by GitHub
parent 4552c20d5b
commit dce6cb601f
10 changed files with 128 additions and 213 deletions

View File

@@ -23,11 +23,11 @@ func makeAtmosCamoBackend(proxy *ImageProxy) *AtmosCamoBackend {
}
func (backend *AtmosCamoBackend) GetImage(w http.ResponseWriter, r *http.Request, imageURL string) {
http.Redirect(w, r, backend.GetProxiedImageURL(imageURL), http.StatusFound)
http.Redirect(w, r, backend.getAtmosCamoImageURL(imageURL), http.StatusFound)
}
func (backend *AtmosCamoBackend) GetImageDirect(imageURL string) (io.ReadCloser, string, error) {
req, err := http.NewRequest("GET", backend.GetProxiedImageURL(imageURL), nil)
req, err := http.NewRequest("GET", backend.getAtmosCamoImageURL(imageURL), nil)
if err != nil {
return nil, "", Error{err}
}
@@ -43,7 +43,7 @@ func (backend *AtmosCamoBackend) GetImageDirect(imageURL string) (io.ReadCloser,
return resp.Body, resp.Header.Get("Content-Type"), nil
}
func (backend *AtmosCamoBackend) GetProxiedImageURL(imageURL string) string {
func (backend *AtmosCamoBackend) getAtmosCamoImageURL(imageURL string) string {
cfg := *backend.proxy.ConfigService.Config()
siteURL := *cfg.ServiceSettings.SiteURL
proxyURL := *cfg.ImageProxySettings.RemoteImageProxyURL
@@ -64,25 +64,3 @@ func getAtmosCamoImageURL(imageURL, siteURL, proxyURL, options string) string {
return proxyURL + "/" + digest + "/" + hex.EncodeToString([]byte(imageURL))
}
func (backend *AtmosCamoBackend) GetUnproxiedImageURL(proxiedURL string) string {
proxyURL := *backend.proxy.ConfigService.Config().ImageProxySettings.RemoteImageProxyURL + "/"
if !strings.HasPrefix(proxiedURL, proxyURL) {
return proxiedURL
}
path := proxiedURL[len(proxyURL):]
slash := strings.IndexByte(path, '/')
if slash == -1 {
return proxiedURL
}
decoded, err := hex.DecodeString(path[slash+1:])
if err != nil {
return proxiedURL
}
return string(decoded)
}

View File

@@ -76,22 +76,6 @@ func TestAtmosCamoBackend_GetImageDirect(t *testing.T) {
assert.Equal(t, []byte("1111111111"), respBody)
}
func TestAtmosCamoBackend_GetProxiedImageURL(t *testing.T) {
imageURL := "http://www.mattermost.org/wp-content/uploads/2016/03/logoHorizontal.png"
proxiedURL := "http://images.example.com/5b6f6661516bc837b89b54566eb619d14a5c3eca/687474703a2f2f7777772e6d61747465726d6f73742e6f72672f77702d636f6e74656e742f75706c6f6164732f323031362f30332f6c6f676f486f72697a6f6e74616c2e706e67"
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
})
mock := httptest.NewServer(handler)
defer mock.Close()
proxy := makeTestAtmosCamoProxy()
// Most of this logic is tested in TestGetAtmosCamoImageURL
assert.Equal(t, proxiedURL, proxy.GetProxiedImageURL(imageURL))
}
func TestGetAtmosCamoImageURL(t *testing.T) {
imageURL := "http://www.mattermost.org/wp-content/uploads/2016/03/logoHorizontal.png"
proxiedURL := "http://images.example.com/5b6f6661516bc837b89b54566eb619d14a5c3eca/687474703a2f2f7777772e6d61747465726d6f73742e6f72672f77702d636f6e74656e742f75706c6f6164732f323031362f30332f6c6f676f486f72697a6f6e74616c2e706e67"
@@ -155,41 +139,3 @@ func TestGetAtmosCamoImageURL(t *testing.T) {
}
}
func TestAtmosCamoBackend_GetUnproxiedImageURL(t *testing.T) {
imageURL := "http://www.mattermost.org/wp-content/uploads/2016/03/logoHorizontal.png"
proxiedURL := "http://images.example.com/5b6f6661516bc837b89b54566eb619d14a5c3eca/687474703a2f2f7777772e6d61747465726d6f73742e6f72672f77702d636f6e74656e742f75706c6f6164732f323031362f30332f6c6f676f486f72697a6f6e74616c2e706e67"
proxy := makeTestAtmosCamoProxy()
for _, test := range []struct {
Name string
Input string
Expected string
}{
{
Name: "should remove proxy",
Input: proxiedURL,
Expected: imageURL,
},
{
Name: "should not remove proxy from a relative image",
Input: "/static/logo.png",
Expected: "/static/logo.png",
},
{
Name: "should not remove proxy from an image on the Mattermost server",
Input: "https://mattermost.example.com/static/logo.png",
Expected: "https://mattermost.example.com/static/logo.png",
},
{
Name: "should not remove proxy from a non-proxied image",
Input: imageURL,
Expected: imageURL,
},
} {
t.Run(test.Name, func(t *testing.T) {
assert.Equal(t, test.Expected, proxy.GetUnproxiedImageURL(test.Input))
})
}
}

View File

@@ -7,6 +7,8 @@ import (
"errors"
"io"
"net/http"
"net/url"
"strings"
"sync"
"github.com/mattermost/mattermost-server/mlog"
@@ -39,13 +41,6 @@ type ImageProxyBackend interface {
// GetImageDirect returns a proxied image along with its content type.
GetImageDirect(imageURL string) (io.ReadCloser, string, error)
// GetProxiedImageURL returns the URL to access a given image through the image proxy, whether the image proxy is
// running externally or as part of the Mattermost server itself.
GetProxiedImageURL(imageURL string) string
// GetUnproxiedImageURL returns the original URL of an image from one that has been directed at the image proxy.
GetUnproxiedImageURL(proxiedURL string) string
}
func MakeImageProxy(configService configservice.ConfigService, httpService httpservice.HTTPService, logger *mlog.Logger) *ImageProxy {
@@ -123,24 +118,36 @@ func (proxy *ImageProxy) GetImageDirect(imageURL string) (io.ReadCloser, string,
// GetProxiedImageURL takes the URL of an image and returns a URL that can be used to view that image through the
// image proxy.
func (proxy *ImageProxy) GetProxiedImageURL(imageURL string) string {
proxy.lock.RLock()
defer proxy.lock.RUnlock()
return getProxiedImageURL(imageURL, *proxy.ConfigService.Config().ServiceSettings.SiteURL)
}
if proxy.backend == nil {
func getProxiedImageURL(imageURL, siteURL string) string {
if imageURL == "" || imageURL[0] == '/' || strings.HasPrefix(imageURL, siteURL) {
return imageURL
}
return proxy.backend.GetProxiedImageURL(imageURL)
return siteURL + "/api/v4/image?url=" + url.QueryEscape(imageURL)
}
// GetUnproxiedImageURL takes the URL of an image on the image proxy and returns the original URL of the image.
func (proxy *ImageProxy) GetUnproxiedImageURL(proxiedURL string) string {
proxy.lock.RLock()
defer proxy.lock.RUnlock()
return getUnproxiedImageURL(proxiedURL, *proxy.ConfigService.Config().ServiceSettings.SiteURL)
}
if proxy.backend == nil {
func getUnproxiedImageURL(proxiedURL, siteURL string) string {
if !strings.HasPrefix(proxiedURL, siteURL+"/api/v4/image?url=") {
return proxiedURL
}
return proxy.backend.GetUnproxiedImageURL(proxiedURL)
parsed, err := url.Parse(proxiedURL)
if err != nil {
return proxiedURL
}
u := parsed.Query()["url"]
if len(u) == 0 {
return proxiedURL
}
return u[0]
}

View File

@@ -0,0 +1,86 @@
// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
// See License.txt for license information.
package imageproxy
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestGetProxiedImageURL(t *testing.T) {
siteURL := "https://mattermost.example.com"
imageURL := "http://www.mattermost.org/wp-content/uploads/2016/03/logoHorizontal.png"
proxiedURL := "https://mattermost.example.com/api/v4/image?url=http%3A%2F%2Fwww.mattermost.org%2Fwp-content%2Fuploads%2F2016%2F03%2FlogoHorizontal.png"
for _, test := range []struct {
Name string
Input string
Expected string
}{
{
Name: "should proxy an image",
Input: imageURL,
Expected: proxiedURL,
},
{
Name: "should not proxy a relative image",
Input: "/static/logo.png",
Expected: "/static/logo.png",
},
{
Name: "should not proxy an image on the Mattermost server",
Input: "https://mattermost.example.com/static/logo.png",
Expected: "https://mattermost.example.com/static/logo.png",
},
{
Name: "should not proxy an image that has already been proxied",
Input: proxiedURL,
Expected: proxiedURL,
},
} {
t.Run(test.Name, func(t *testing.T) {
assert.Equal(t, test.Expected, getProxiedImageURL(test.Input, siteURL))
})
}
}
func TestGetUnproxiedImageURL(t *testing.T) {
siteURL := "https://mattermost.example.com"
imageURL := "http://www.mattermost.org/wp-content/uploads/2016/03/logoHorizontal.png"
proxiedURL := "https://mattermost.example.com/api/v4/image?url=http%3A%2F%2Fwww.mattermost.org%2Fwp-content%2Fuploads%2F2016%2F03%2FlogoHorizontal.png"
for _, test := range []struct {
Name string
Input string
Expected string
}{
{
Name: "should remove proxy",
Input: proxiedURL,
Expected: imageURL,
},
{
Name: "should not remove proxy from a relative image",
Input: "/static/logo.png",
Expected: "/static/logo.png",
},
{
Name: "should not remove proxy from an image on the Mattermost server",
Input: "https://mattermost.example.com/static/logo.png",
Expected: "https://mattermost.example.com/static/logo.png",
},
{
Name: "should not remove proxy from a non-proxied image",
Input: imageURL,
Expected: imageURL,
},
} {
t.Run(test.Name, func(t *testing.T) {
assert.Equal(t, test.Expected, getUnproxiedImageURL(test.Input, siteURL))
})
}
}

View File

@@ -10,7 +10,6 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"strings"
"time"
"github.com/mattermost/mattermost-server/mlog"
@@ -105,33 +104,3 @@ func (backend *LocalBackend) GetImageDirect(imageURL string) (io.ReadCloser, str
return ioutil.NopCloser(recorder.Body), recorder.Header().Get("Content-Type"), nil
}
func (backend *LocalBackend) GetProxiedImageURL(imageURL string) string {
siteURL := *backend.proxy.ConfigService.Config().ServiceSettings.SiteURL
if imageURL == "" || imageURL[0] == '/' || strings.HasPrefix(imageURL, siteURL) {
return imageURL
}
return siteURL + "/api/v4/image?url=" + url.QueryEscape(imageURL)
}
func (backend *LocalBackend) GetUnproxiedImageURL(proxiedURL string) string {
siteURL := *backend.proxy.ConfigService.Config().ServiceSettings.SiteURL
if !strings.HasPrefix(proxiedURL, siteURL+"/api/v4/image?url=") {
return proxiedURL
}
parsed, err := url.Parse(proxiedURL)
if err != nil {
return proxiedURL
}
u := parsed.Query()["url"]
if len(u) == 0 {
return proxiedURL
}
return u[0]
}

View File

@@ -293,79 +293,3 @@ func TestLocalBackend_GetImageDirect(t *testing.T) {
wait <- true
})
}
func TestLocalBackend_GetProxiedImageURL(t *testing.T) {
imageURL := "http://www.mattermost.org/wp-content/uploads/2016/03/logoHorizontal.png"
proxiedURL := "https://mattermost.example.com/api/v4/image?url=http%3A%2F%2Fwww.mattermost.org%2Fwp-content%2Fuploads%2F2016%2F03%2FlogoHorizontal.png"
proxy := makeTestLocalProxy()
for _, test := range []struct {
Name string
Input string
Expected string
}{
{
Name: "should proxy image",
Input: imageURL,
Expected: proxiedURL,
},
{
Name: "should not proxy a relative image",
Input: "/static/logo.png",
Expected: "/static/logo.png",
},
{
Name: "should not proxy an image on the Mattermost server",
Input: "https://mattermost.example.com/static/logo.png",
Expected: "https://mattermost.example.com/static/logo.png",
},
{
Name: "should not proxy an image that has already been proxied",
Input: proxiedURL,
Expected: proxiedURL,
},
} {
t.Run(test.Name, func(t *testing.T) {
assert.Equal(t, test.Expected, proxy.GetProxiedImageURL(test.Input))
})
}
}
func TestLocalBackend_GetUnproxiedImageURL(t *testing.T) {
imageURL := "http://www.mattermost.org/wp-content/uploads/2016/03/logoHorizontal.png"
proxiedURL := "https://mattermost.example.com/api/v4/image?url=http%3A%2F%2Fwww.mattermost.org%2Fwp-content%2Fuploads%2F2016%2F03%2FlogoHorizontal.png"
proxy := makeTestLocalProxy()
for _, test := range []struct {
Name string
Input string
Expected string
}{
{
Name: "should remove proxy",
Input: proxiedURL,
Expected: imageURL,
},
{
Name: "should not remove proxy from a relative image",
Input: "/static/logo.png",
Expected: "/static/logo.png",
},
{
Name: "should not remove proxy from an image on the Mattermost server",
Input: "https://mattermost.example.com/static/logo.png",
Expected: "https://mattermost.example.com/static/logo.png",
},
{
Name: "should not remove proxy from a non-proxied image",
Input: imageURL,
Expected: imageURL,
},
} {
t.Run(test.Name, func(t *testing.T) {
assert.Equal(t, test.Expected, proxy.GetUnproxiedImageURL(test.Input))
})
}
}