Merge branch 'release-4.7-t3merge3'

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"))
 }

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

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": {
+		"atmos/camo_SameSite": {
-			ProxyType:       "willnorris/imageproxy",
+			ProxyType:       "atmos/camo",
-			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",
 			ProxyURL:        "https://127.0.0.1",
+			ProxyOptions:    "foo",
 			ImageURL:        "http://mymattermost.com/myimage",
 			ProxiedImageURL: "http://mymattermost.com/myimage",
 		},
-		"willnorris/imageproxy_PathOnly": {
+		"atmos/camo_PathOnly": {
-			ProxyType:       "willnorris/imageproxy",
+			ProxyType:       "atmos/camo",
 			ProxyURL:        "https://127.0.0.1",
+			ProxyOptions:    "foo",
 			ImageURL:        "/myimage",
 			ProxiedImageURL: "/myimage",
 		},
-		"willnorris/imageproxy_EmptyImageURL": {
+		"atmos/camo_EmptyImageURL": {
-			ProxyType:       "willnorris/imageproxy",
+			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 TestMakeOpenGraphURLsAbsolute(t *testing.T) {
+	for name, tc := range map[string]struct {
+		HTML       string
+		RequestURL string
+		URL        string
+		ImageURL   string
+	}{
+		"absolute URLs": {
+			HTML: `
+				<html>
+					<head>
+						<meta property="og:url" content="https://example.com/apps/mattermost">
+						<meta property="og:image" content="https://images.example.com/image.png">
+					</head>
+				</html>`,
+			RequestURL: "https://example.com",
+			URL:        "https://example.com/apps/mattermost",
+			ImageURL:   "https://images.example.com/image.png",
+		},
+		"URLs starting with /": {
+			HTML: `
+				<html>
+					<head>
+						<meta property="og:url" content="/apps/mattermost">
+						<meta property="og:image" content="/image.png">
+					</head>
+				</html>`,
+			RequestURL: "http://example.com",
+			URL:        "http://example.com/apps/mattermost",
+			ImageURL:   "http://example.com/image.png",
+		},
+		"HTTPS URLs starting with /": {
+			HTML: `
+				<html>
+					<head>
+						<meta property="og:url" content="/apps/mattermost">
+						<meta property="og:image" content="/image.png">
+					</head>
+				</html>`,
+			RequestURL: "https://example.com",
+			URL:        "https://example.com/apps/mattermost",
+			ImageURL:   "https://example.com/image.png",
+		},
+		"missing image URL": {
+			HTML: `
+				<html>
+					<head>
+						<meta property="og:url" content="/apps/mattermost">
+					</head>
+				</html>`,
+			RequestURL: "http://example.com",
+			URL:        "http://example.com/apps/mattermost",
+			ImageURL:   "",
+		},
+		"relative URLs": {
+			HTML: `
+				<html>
+					<head>
+						<meta property="og:url" content="index.html">
+						<meta property="og:image" content="../resources/image.png">
+					</head>
+				</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)
+			}
 
-func BenchmarkPostWithProxyRemovedFromImageURLs(b *testing.B) {
+			makeOpenGraphURLsAbsolute(og, tc.RequestURL)
-	th := Setup().InitBasic()
-	defer th.TearDown()
 
-	th.App.UpdateConfig(func(cfg *model.Config) {
+			if og.URL != tc.URL {
-		cfg.ServiceSettings.ImageProxyType = model.NewString("willnorris/imageproxy")
+				t.Fatalf("incorrect url, expected %v, got %v", tc.URL, og.URL)
-		cfg.ServiceSettings.ImageProxyOptions = model.NewString("x1000|foo")
+			}
-		cfg.ServiceSettings.ImageProxyURL = model.NewString("https://127.0.0.1")
-	})
 
-	post := &model.Post{
+			if len(og.Images) > 0 {
-		Message: "![foo](http://mydomain.com/myimage)",
+				if og.Images[0].URL != tc.ImageURL {
-	}
+					t.Fatalf("incorrect image url, expected %v, got %v", tc.ImageURL, og.Images[0].URL)
-
+				}
-	b.ResetTimer()
+			} else if tc.ImageURL != "" {
-
+				t.Fatalf("missing image url, expected %v, got nothing", tc.ImageURL)
-	for i := 0; i < b.N; i++ {
+			}
-		imageProxyBenchmarkSink = th.App.PostWithProxyAddedToImageURLs(post)
+		})
 	}
 }

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)