diff --git a/api4/image_test.go b/api4/image_test.go index 236d5785da047d6da0342ecbd4d69a91cb2f8137..aa3619cdd29bb8e5fe2dcc2b5ce29202bab62206 100644 --- a/api4/image_test.go +++ b/api4/image_test.go @@ -37,7 +37,8 @@ func TestGetImage(t *testing.T) { assert.Equal(t, http.StatusNotFound, resp.StatusCode) th.App.UpdateConfig(func(cfg *model.Config) { - cfg.ServiceSettings.ImageProxyType = model.NewString("willnorris/imageproxy") + cfg.ServiceSettings.ImageProxyType = model.NewString("atmos/camo") + cfg.ServiceSettings.ImageProxyOptions = model.NewString("foo") cfg.ServiceSettings.ImageProxyURL = model.NewString("https://proxy.foo.bar") }) @@ -48,5 +49,5 @@ func TestGetImage(t *testing.T) { resp, err = th.Client.HttpClient.Do(r) require.NoError(t, err) assert.Equal(t, http.StatusFound, resp.StatusCode) - assert.Equal(t, "https://proxy.foo.bar//"+originURL, resp.Header.Get("Location")) + assert.Equal(t, "https://proxy.foo.bar/004afe2ef382eb5f30c4490f793f8a8c5b33d8a2/687474703a2f2f666f6f2e6261722f62617a2e676966", resp.Header.Get("Location")) } diff --git a/app/post.go b/app/post.go index be9374e10a53d929d80a6cdd3c364492a97503b8..a541797fa688950e3fecbc8395fa2fe0a4a38ab0 100644 --- a/app/post.go +++ b/app/post.go @@ -6,12 +6,11 @@ package app import ( "crypto/hmac" "crypto/sha1" - "crypto/sha256" - "encoding/base64" "encoding/hex" "encoding/json" "fmt" "net/http" + "net/url" "regexp" "strings" @@ -734,23 +733,68 @@ func (a *App) GetFileInfosForPost(postId string, readFromMaster bool) ([]*model. return infos, nil } -func (a *App) GetOpenGraphMetadata(url string) *opengraph.OpenGraph { +func (a *App) GetOpenGraphMetadata(requestURL string) *opengraph.OpenGraph { og := opengraph.NewOpenGraph() - res, err := a.HTTPClient(false).Get(url) + res, err := a.HTTPClient(false).Get(requestURL) if err != nil { - l4g.Error("GetOpenGraphMetadata request failed for url=%v with err=%v", url, err.Error()) + l4g.Error("GetOpenGraphMetadata request failed for url=%v with err=%v", requestURL, err.Error()) return og } defer consumeAndClose(res) if err := og.ProcessHTML(res.Body); err != nil { - l4g.Error("GetOpenGraphMetadata processing failed for url=%v with err=%v", url, err.Error()) + l4g.Error("GetOpenGraphMetadata processing failed for url=%v with err=%v", requestURL, err.Error()) } + makeOpenGraphURLsAbsolute(og, requestURL) + return og } +func makeOpenGraphURLsAbsolute(og *opengraph.OpenGraph, requestURL string) { + parsedRequestURL, err := url.Parse(requestURL) + if err != nil { + l4g.Warn("makeOpenGraphURLsAbsolute failed to parse url=%v", requestURL) + return + } + + makeURLAbsolute := func(resultURL string) string { + if resultURL == "" { + return resultURL + } + + parsedResultURL, err := url.Parse(resultURL) + if err != nil { + l4g.Warn("makeOpenGraphURLsAbsolute failed to parse result url=%v", resultURL) + return resultURL + } + + if parsedResultURL.IsAbs() { + return resultURL + } + + return parsedRequestURL.ResolveReference(parsedResultURL).String() + } + + og.URL = makeURLAbsolute(og.URL) + + for _, image := range og.Images { + image.URL = makeURLAbsolute(image.URL) + image.SecureURL = makeURLAbsolute(image.SecureURL) + } + + for _, audio := range og.Audios { + audio.URL = makeURLAbsolute(audio.URL) + audio.SecureURL = makeURLAbsolute(audio.SecureURL) + } + + for _, video := range og.Videos { + video.URL = makeURLAbsolute(video.URL) + video.SecureURL = makeURLAbsolute(video.SecureURL) + } +} + func (a *App) DoPostAction(postId string, actionId string, userId string) *model.AppError { pchan := a.Srv.Store.Post().GetSingle(postId) @@ -904,18 +948,6 @@ func (a *App) ImageProxyAdder() func(string) string { mac.Write([]byte(url)) digest := hex.EncodeToString(mac.Sum(nil)) return proxyURL + digest + "/" + hex.EncodeToString([]byte(url)) - case "willnorris/imageproxy": - options := strings.Split(options, "|") - if len(options) > 1 { - mac := hmac.New(sha256.New, []byte(options[1])) - mac.Write([]byte(url)) - digest := base64.URLEncoding.EncodeToString(mac.Sum(nil)) - if options[0] == "" { - return proxyURL + "s" + digest + "/" + url - } - return proxyURL + options[0] + ",s" + digest + "/" + url - } - return proxyURL + options[0] + "/" + url } return url @@ -938,12 +970,6 @@ func (a *App) ImageProxyRemover() (f func(string) string) { } } } - case "willnorris/imageproxy": - if strings.HasPrefix(url, proxyURL) { - if slash := strings.IndexByte(url[len(proxyURL):], '/'); slash >= 0 { - return url[len(proxyURL)+slash+1:] - } - } } return url diff --git a/app/post_test.go b/app/post_test.go index 409bc043d6415f4eb67a5a8e3abd4a4b088c3048..2472e40c64f7cd3ef06e98e37b545753ce30cc00 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -8,9 +8,11 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "testing" "time" + "github.com/dyatlov/go-opengraph/opengraph" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -208,38 +210,27 @@ func TestImageProxy(t *testing.T) { ImageURL: "http://mydomain.com/myimage", ProxiedImageURL: "https://127.0.0.1/f8dace906d23689e8d5b12c3cefbedbf7b9b72f5/687474703a2f2f6d79646f6d61696e2e636f6d2f6d79696d616765", }, - "willnorris/imageproxy": { - ProxyType: "willnorris/imageproxy", - ProxyURL: "https://127.0.0.1", - ProxyOptions: "x1000", - ImageURL: "http://mydomain.com/myimage", - ProxiedImageURL: "https://127.0.0.1/x1000/http://mydomain.com/myimage", - }, - "willnorris/imageproxy_SameSite": { - ProxyType: "willnorris/imageproxy", + "atmos/camo_SameSite": { + ProxyType: "atmos/camo", ProxyURL: "https://127.0.0.1", + ProxyOptions: "foo", ImageURL: "http://mymattermost.com/myimage", ProxiedImageURL: "http://mymattermost.com/myimage", }, - "willnorris/imageproxy_PathOnly": { - ProxyType: "willnorris/imageproxy", + "atmos/camo_PathOnly": { + ProxyType: "atmos/camo", ProxyURL: "https://127.0.0.1", + ProxyOptions: "foo", ImageURL: "/myimage", ProxiedImageURL: "/myimage", }, - "willnorris/imageproxy_EmptyImageURL": { - ProxyType: "willnorris/imageproxy", + "atmos/camo_EmptyImageURL": { + ProxyType: "atmos/camo", ProxyURL: "https://127.0.0.1", + ProxyOptions: "foo", ImageURL: "", ProxiedImageURL: "", }, - "willnorris/imageproxy_WithSigning": { - ProxyType: "willnorris/imageproxy", - ProxyURL: "https://127.0.0.1", - ProxyOptions: "x1000|foo", - ImageURL: "http://mydomain.com/myimage", - ProxiedImageURL: "https://127.0.0.1/x1000,sbhHVoG5d60UvnNtGh6Iy6x4PaMmnsh8JfZ7JfErKjGU=/http://mydomain.com/myimage", - }, } { t.Run(name, func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { @@ -266,25 +257,92 @@ func TestImageProxy(t *testing.T) { } } -var imageProxyBenchmarkSink *model.Post - -func BenchmarkPostWithProxyRemovedFromImageURLs(b *testing.B) { - th := Setup().InitBasic() - defer th.TearDown() - - th.App.UpdateConfig(func(cfg *model.Config) { - cfg.ServiceSettings.ImageProxyType = model.NewString("willnorris/imageproxy") - cfg.ServiceSettings.ImageProxyOptions = model.NewString("x1000|foo") - cfg.ServiceSettings.ImageProxyURL = model.NewString("https://127.0.0.1") - }) +func TestMakeOpenGraphURLsAbsolute(t *testing.T) { + for name, tc := range map[string]struct { + HTML string + RequestURL string + URL string + ImageURL string + }{ + "absolute URLs": { + HTML: ` + + + + + + `, + RequestURL: "https://example.com", + URL: "https://example.com/apps/mattermost", + ImageURL: "https://images.example.com/image.png", + }, + "URLs starting with /": { + HTML: ` + + + + + + `, + RequestURL: "http://example.com", + URL: "http://example.com/apps/mattermost", + ImageURL: "http://example.com/image.png", + }, + "HTTPS URLs starting with /": { + HTML: ` + + + + + + `, + RequestURL: "https://example.com", + URL: "https://example.com/apps/mattermost", + ImageURL: "https://example.com/image.png", + }, + "missing image URL": { + HTML: ` + + + + + `, + RequestURL: "http://example.com", + URL: "http://example.com/apps/mattermost", + ImageURL: "", + }, + "relative URLs": { + HTML: ` + + + + + + `, + RequestURL: "http://example.com/content/index.html", + URL: "http://example.com/content/index.html", + ImageURL: "http://example.com/resources/image.png", + }, + } { + t.Run(name, func(t *testing.T) { + og := opengraph.NewOpenGraph() + if err := og.ProcessHTML(strings.NewReader(tc.HTML)); err != nil { + t.Fatal(err) + } - post := &model.Post{ - Message: "![foo](http://mydomain.com/myimage)", - } + makeOpenGraphURLsAbsolute(og, tc.RequestURL) - b.ResetTimer() + if og.URL != tc.URL { + t.Fatalf("incorrect url, expected %v, got %v", tc.URL, og.URL) + } - for i := 0; i < b.N; i++ { - imageProxyBenchmarkSink = th.App.PostWithProxyAddedToImageURLs(post) + if len(og.Images) > 0 { + if og.Images[0].URL != tc.ImageURL { + t.Fatalf("incorrect image url, expected %v, got %v", tc.ImageURL, og.Images[0].URL) + } + } else if tc.ImageURL != "" { + t.Fatalf("missing image url, expected %v, got nothing", tc.ImageURL) + } + }) } } diff --git a/model/config.go b/model/config.go index 9010eaeae573e276e6aa9b3bea167f6f538beaa4..898099d121b84cd7e43837ae863a3c4d04178b76 100644 --- a/model/config.go +++ b/model/config.go @@ -2096,7 +2096,7 @@ func (ss *ServiceSettings) isValid() *AppError { } switch *ss.ImageProxyType { - case "", "willnorris/imageproxy": + case "": case "atmos/camo": if *ss.ImageProxyOptions == "" { return NewAppError("Config.IsValid", "model.config.is_valid.atmos_camo_image_proxy_options.app_error", nil, "", http.StatusBadRequest)