MM-38634: Simplify image proxy (#19603)

We copy over the necessary code to simplify
the local image proxy feature.

https://mattermost.atlassian.net/browse/MM-38634

```release-note
NONE
```

services/imageproxy/atmos_camo.go

@@ -16,6 +16,7 @@ type AtmosCamoBackend struct {
 	proxy     *ImageProxy
 	siteURL   *url.URL
 	remoteURL *url.URL
+	client    *http.Client
 }
 
 func makeAtmosCamoBackend(proxy *ImageProxy) *AtmosCamoBackend {
@@ -28,6 +29,7 @@ func makeAtmosCamoBackend(proxy *ImageProxy) *AtmosCamoBackend {
 		proxy:     proxy,
 		siteURL:   siteURL,
 		remoteURL: remoteURL,
+		client:    proxy.HTTPService.MakeClient(false),
 	}
 }
 
@@ -41,9 +43,7 @@ func (backend *AtmosCamoBackend) GetImageDirect(imageURL string) (io.ReadCloser,
 		return nil, "", Error{err}
 	}
 
-	client := backend.proxy.HTTPService.MakeClient(false)
-
-	resp, err := client.Do(req)
+	resp, err := backend.client.Do(req)
 	if err != nil {
 		return nil, "", Error{err}
 	}

services/imageproxy/atmos_camo_test.go

@@ -76,6 +76,7 @@ func TestAtmosCamoBackend_GetImageDirect(t *testing.T) {
 		proxy:     proxy,
 		siteURL:   parsedURL,
 		remoteURL: remoteURL,
+		client:    proxy.HTTPService.MakeClient(false),
 	}
 
 	body, contentType, err := backend.GetImageDirect("https://example.com/image.png")

services/imageproxy/local.go

@@ -4,19 +4,22 @@
 package imageproxy
 
 import (
+	"bufio"
 	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"mime"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"path"
 	"path/filepath"
+	"regexp"
+	"strings"
+	"time"
 
-	"willnorris.com/go/imageproxy"
-
-	"github.com/mattermost/mattermost-server/v6/services/httpservice"
 	"github.com/mattermost/mattermost-server/v6/shared/mlog"
 )
 
@@ -33,35 +36,42 @@ var imageContentTypes = []string{
 	"image/x-quicktime", "image/x-rgb", "image/x-xbitmap", "image/x-xpixmap", "image/x-xwindowdump",
 }
 
+var msgNotAllowed = "requested URL is not allowed"
+
 var ErrLocalRequestFailed = Error{errors.New("imageproxy.LocalBackend: failed to request proxied image")}
 
 type LocalBackend struct {
 	proxy *ImageProxy
 
-	// The underlying image proxy implementation provided by the third party library
-	impl *imageproxy.Proxy
+	client  *http.Client
+	baseURL *url.URL
+}
+
+// URLError reports a malformed URL error.
+type URLError struct {
+	Message string
+	URL     *url.URL
+}
+
+func (e URLError) Error() string {
+	return fmt.Sprintf("malformed URL %q: %s", e.URL, e.Message)
 }
 
 func makeLocalBackend(proxy *ImageProxy) *LocalBackend {
-	impl := imageproxy.NewProxy(proxy.HTTPService.MakeTransport(false), nil)
-
-	if proxy.Logger != nil {
-		impl.Logger = proxy.Logger.With(mlog.String("image_proxy", "local")).StdLogger(mlog.LvlDebug)
-	}
-
 	baseURL, err := url.Parse(*proxy.ConfigService.Config().ServiceSettings.SiteURL)
 	if err != nil {
 		mlog.Warn("Failed to set base URL for image proxy. Relative image links may not work.", mlog.Err(err))
-	} else {
-		impl.DefaultBaseURL = baseURL
 	}
 
-	impl.Timeout = httpservice.RequestTimeout
-	impl.ContentTypes = imageContentTypes
+	client := proxy.HTTPService.MakeClient(false)
+	client.CheckRedirect = func(newreq *http.Request, via []*http.Request) error {
+		return http.ErrUseLastResponse
+	}
 
 	return &LocalBackend{
-		proxy: proxy,
-		impl:  impl,
+		proxy:   proxy,
+		client:  client,
+		baseURL: baseURL,
 	}
 }
 
@@ -109,7 +119,7 @@ func (backend *LocalBackend) GetImage(w http.ResponseWriter, r *http.Request, im
 	w.Header().Set("Content-Security-Policy", "default-src 'none'; img-src data:; style-src 'unsafe-inline'")
 
 	rec := contentTypeRecorder{w, filepath.Base(u.Path)}
-	backend.impl.ServeHTTP(&rec, req)
+	backend.ServeImage(&rec, req)
 }
 
 func (backend *LocalBackend) GetImageDirect(imageURL string) (io.ReadCloser, string, error) {
@@ -121,7 +131,7 @@ func (backend *LocalBackend) GetImageDirect(imageURL string) (io.ReadCloser, str
 
 	recorder := httptest.NewRecorder()
 
-	backend.impl.ServeHTTP(recorder, req)
+	backend.ServeImage(recorder, req)
 
 	if recorder.Code != http.StatusOK {
 		return nil, "", ErrLocalRequestFailed
@@ -129,3 +139,194 @@ func (backend *LocalBackend) GetImageDirect(imageURL string) (io.ReadCloser, str
 
 	return ioutil.NopCloser(recorder.Body), recorder.Header().Get("Content-Type"), nil
 }
+
+func (backend *LocalBackend) ServeImage(w http.ResponseWriter, req *http.Request) {
+	proxyReq, err := newProxyRequest(req, backend.baseURL)
+	if err != nil {
+		http.Error(w, fmt.Sprintf("invalid request URL: %v", err), http.StatusBadRequest)
+		return
+	}
+
+	actualReq, err := http.NewRequest("GET", proxyReq.String(), nil)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	actualReq.Header.Set("Accept", strings.Join(imageContentTypes, ", "))
+
+	resp, err := backend.client.Do(actualReq)
+	if err != nil {
+		msg := fmt.Sprintf("error fetching remote image: %v", err)
+		mlog.Warn(msg)
+		statusCode := http.StatusInternalServerError
+		if e, ok := err.(net.Error); ok && e.Timeout() {
+			statusCode = http.StatusGatewayTimeout
+		}
+		http.Error(w, msg, statusCode)
+		return
+	}
+	// close the original resp.Body, even if we wrap it in a NopCloser below
+	defer resp.Body.Close()
+
+	copyHeader(w.Header(), resp.Header, "Cache-Control", "Last-Modified", "Expires", "Etag", "Link")
+
+	if should304(req, resp) {
+		w.WriteHeader(http.StatusNotModified)
+		return
+	}
+
+	contentType, _, _ := mime.ParseMediaType(resp.Header.Get("Content-Type"))
+	if contentType == "" || contentType == "application/octet-stream" || contentType == "binary/octet-stream" {
+		// try to detect content type
+		b := bufio.NewReader(resp.Body)
+		resp.Body = ioutil.NopCloser(b)
+		contentType = peekContentType(b)
+	}
+	if resp.ContentLength != 0 && !contentTypeMatches(imageContentTypes, contentType) {
+		http.Error(w, msgNotAllowed, http.StatusForbidden)
+		return
+	}
+	w.Header().Set("Content-Type", contentType)
+
+	copyHeader(w.Header(), resp.Header, "Content-Length")
+
+	// Enable CORS for 3rd party applications
+	w.Header().Set("Access-Control-Allow-Origin", "*")
+
+	// Add a Content-Security-Policy to prevent stored-XSS attacks via SVG files
+	w.Header().Set("Content-Security-Policy", "script-src 'none'")
+
+	// Disable Content-Type sniffing
+	w.Header().Set("X-Content-Type-Options", "nosniff")
+
+	// Block potential XSS attacks especially in legacy browsers which do not support CSP
+	w.Header().Set("X-XSS-Protection", "1; mode=block")
+
+	w.WriteHeader(resp.StatusCode)
+	if _, err := io.Copy(w, resp.Body); err != nil {
+		mlog.Warn("error copying response", mlog.Err(err))
+	}
+}
+
+// copyHeader copies header values from src to dst, adding to any existing
+// values with the same header name.  If keys is not empty, only those header
+// keys will be copied.
+func copyHeader(dst, src http.Header, keys ...string) {
+	if len(keys) == 0 {
+		for k := range src {
+			keys = append(keys, k)
+		}
+	}
+	for _, key := range keys {
+		k := http.CanonicalHeaderKey(key)
+		for _, v := range src[k] {
+			dst.Add(k, v)
+		}
+	}
+}
+
+func should304(req *http.Request, resp *http.Response) bool {
+	etag := resp.Header.Get("Etag")
+	if etag != "" && etag == req.Header.Get("If-None-Match") {
+		return true
+	}
+
+	lastModified, err := time.Parse(time.RFC1123, resp.Header.Get("Last-Modified"))
+	if err != nil {
+		return false
+	}
+	ifModSince, err := time.Parse(time.RFC1123, req.Header.Get("If-Modified-Since"))
+	if err != nil {
+		return false
+	}
+	if lastModified.Before(ifModSince) || lastModified.Equal(ifModSince) {
+		return true
+	}
+
+	return false
+}
+
+// peekContentType peeks at the first 512 bytes of p, and attempts to detect
+// the content type.  Returns empty string if error occurs.
+func peekContentType(p *bufio.Reader) string {
+	byt, err := p.Peek(512)
+	if err != nil && err != bufio.ErrBufferFull && err != io.EOF {
+		return ""
+	}
+	return http.DetectContentType(byt)
+}
+
+// contentTypeMatches returns whether contentType matches one of the allowed patterns.
+func contentTypeMatches(patterns []string, contentType string) bool {
+	if len(patterns) == 0 {
+		return true
+	}
+
+	for _, pattern := range patterns {
+		if ok, err := path.Match(pattern, contentType); ok && err == nil {
+			return true
+		}
+	}
+
+	return false
+}
+
+// proxyRequest is an imageproxy request which includes a remote URL of an image to
+// proxy.
+type proxyRequest struct {
+	URL      *url.URL      // URL of the image to proxy
+	Original *http.Request // The original HTTP request
+}
+
+// String returns the request URL as a string, with r.Options encoded in the
+// URL fragment.
+func (r proxyRequest) String() string {
+	return r.URL.String()
+}
+
+func newProxyRequest(r *http.Request, baseURL *url.URL) (*proxyRequest, error) {
+	var err error
+	req := &proxyRequest{Original: r}
+
+	path := r.URL.EscapedPath()[1:] // strip leading slash
+	req.URL, err = parseURL(path)
+	if err != nil || !req.URL.IsAbs() {
+		// first segment should be options
+		parts := strings.SplitN(path, "/", 2)
+		if len(parts) != 2 {
+			return nil, URLError{"too few path segments", r.URL}
+		}
+
+		var err error
+		req.URL, err = parseURL(parts[1])
+		if err != nil {
+			return nil, URLError{fmt.Sprintf("unable to parse remote URL: %v", err), r.URL}
+		}
+
+	}
+
+	if baseURL != nil {
+		req.URL = baseURL.ResolveReference(req.URL)
+	}
+
+	if !req.URL.IsAbs() {
+		return nil, URLError{"must provide absolute remote URL", r.URL}
+	}
+
+	if req.URL.Scheme != "http" && req.URL.Scheme != "https" {
+		return nil, URLError{"remote URL must have http or https scheme", r.URL}
+	}
+
+	// query string is always part of the remote URL
+	req.URL.RawQuery = r.URL.RawQuery
+	return req, nil
+}
+
+var reCleanedURL = regexp.MustCompile(`^(https?):/+([^/])`)
+
+// parseURL parses s as a URL, handling URLs that have been munged by
+// path.Clean or a webserver that collapses multiple slashes.
+func parseURL(s string) (*url.URL, error) {
+	s = reCleanedURL.ReplaceAllString(s, "$1://$2")
+	return url.Parse(s)
+}

services/imageproxy/local_test.go

@@ -154,7 +154,7 @@ func TestLocalBackend_GetImage(t *testing.T) {
 		proxy := makeTestLocalProxy()
 
 		// Modify the timeout to be much shorter than the default 30 seconds
-		proxy.backend.(*LocalBackend).impl.Timeout = time.Millisecond
+		proxy.backend.(*LocalBackend).client.Timeout = time.Millisecond
 
 		recorder := httptest.NewRecorder()
 		request, _ := http.NewRequest(http.MethodGet, "", nil)
@@ -310,7 +310,7 @@ func TestLocalBackend_GetImageDirect(t *testing.T) {
 		proxy := makeTestLocalProxy()
 
 		// Modify the timeout to be much shorter than the default 30 seconds
-		proxy.backend.(*LocalBackend).impl.Timeout = time.Millisecond
+		proxy.backend.(*LocalBackend).client.Timeout = time.Millisecond
 
 		body, contentType, err := proxy.GetImageDirect(mock.URL + "/image.png")