Fixed downloading of image files (#6934)

* Fixed downloading of image files

* Fixed captitalization

* Fixed missing import

* Rename image to media

api/file.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 
 	l4g "github.com/alecthomas/log4go"
 	"github.com/gorilla/mux"
@@ -15,6 +16,15 @@ import (
 	"github.com/mattermost/platform/utils"
 )
 
+var UNSAFE_CONTENT_TYPES = [...]string{
+	"application/javascript",
+	"application/ecmascript",
+	"text/javascript",
+	"text/ecmascript",
+	"application/x-javascript",
+	"text/html",
+}
+
 func InitFile() {
 	l4g.Debug(utils.T("api.file.init.debug"))
 
@@ -282,13 +292,21 @@ func getPublicFileOld(c *Context, w http.ResponseWriter, r *http.Request) {
 func writeFileResponse(filename string, contentType string, bytes []byte, w http.ResponseWriter, r *http.Request) *model.AppError {
 	w.Header().Set("Cache-Control", "max-age=2592000, private")
 	w.Header().Set("Content-Length", strconv.Itoa(len(bytes)))
+	w.Header().Set("X-Content-Type-Options", "nosniff")
 
-	if contentType != "" {
+	if contentType == "" {
-		w.Header().Set("Content-Type", contentType)
+		contentType = "application/octet-stream"
 	} else {
-		w.Header().Del("Content-Type") // Content-Type will be set automatically by the http writer
+		for _, unsafeContentType := range UNSAFE_CONTENT_TYPES {
+			if strings.HasPrefix(contentType, unsafeContentType) {
+				contentType = "text/plain"
+				break
+			}
+		}
 	}
 
+	w.Header().Set("Content-Type", contentType)
+
 	w.Header().Set("Content-Disposition", "attachment;filename=\""+filename+"\"; filename*=UTF-8''"+url.QueryEscape(filename))
 
 	// prevent file links from being embedded in iframes

api4/file.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 
 	l4g "github.com/alecthomas/log4go"
 	"github.com/mattermost/platform/app"
@@ -18,6 +19,27 @@ const (
 	FILE_TEAM_ID = "noteam"
 )
 
+var UNSAFE_CONTENT_TYPES = [...]string{
+	"application/javascript",
+	"application/ecmascript",
+	"text/javascript",
+	"text/ecmascript",
+	"application/x-javascript",
+	"text/html",
+}
+
+var MEDIA_CONTENT_TYPES = [...]string{
+	"image/jpeg",
+	"image/png",
+	"image/bmp",
+	"image/gif",
+	"video/avi",
+	"video/mpeg",
+	"video/mp4",
+	"audio/mpeg",
+	"audio/wav",
+}
+
 func InitFile() {
 	l4g.Debug(utils.T("api.file.init.debug"))
 
@@ -82,9 +104,9 @@ func getFile(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	toDownload, failConv := strconv.ParseBool(r.URL.Query().Get("download"))
+	forceDownload, convErr := strconv.ParseBool(r.URL.Query().Get("download"))
-	if failConv != nil {
+	if convErr != nil {
-		toDownload = false
+		forceDownload = false
 	}
 
 	info, err := app.GetFileInfo(c.Params.FileId)
@@ -105,22 +127,7 @@ func getFile(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	contentTypeToCheck := []string{"image/jpeg", "image/png", "image/bmp", "image/gif",
+	err = writeFileResponse(info.Name, info.MimeType, data, forceDownload, w, r)
-		"video/avi", "video/mpeg", "audio/mpeg3", "audio/wav"}
-
-	contentType := http.DetectContentType(data)
-	foundContentType := false
-	for _, contentTypeFromList := range contentTypeToCheck {
-		if contentType == contentTypeFromList && toDownload == false {
-			foundContentType = true
-			break
-		}
-	}
-	if !foundContentType {
-		toDownload = true
-	}
-
-	err = writeFileResponse(info.Name, info.MimeType, data, toDownload, w, r)
 	if err != nil {
 		c.Err = err
 		return
@@ -133,9 +140,9 @@ func getFileThumbnail(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	toDownload, failConv := strconv.ParseBool(r.URL.Query().Get("download"))
+	forceDownload, convErr := strconv.ParseBool(r.URL.Query().Get("download"))
-	if failConv != nil {
+	if convErr != nil {
-		toDownload = false
+		forceDownload = false
 	}
 
 	info, err := app.GetFileInfo(c.Params.FileId)
@@ -158,7 +165,7 @@ func getFileThumbnail(c *Context, w http.ResponseWriter, r *http.Request) {
 	if data, err := app.ReadFile(info.ThumbnailPath); err != nil {
 		c.Err = err
 		c.Err.StatusCode = http.StatusNotFound
-	} else if err := writeFileResponse(info.Name, info.MimeType, data, toDownload, w, r); err != nil {
+	} else if err := writeFileResponse(info.Name, info.MimeType, data, forceDownload, w, r); err != nil {
 		c.Err = err
 		return
 	}
@@ -205,9 +212,9 @@ func getFilePreview(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	toDownload, failConv := strconv.ParseBool(r.URL.Query().Get("download"))
+	forceDownload, convErr := strconv.ParseBool(r.URL.Query().Get("download"))
-	if failConv != nil {
+	if convErr != nil {
-		toDownload = false
+		forceDownload = false
 	}
 
 	info, err := app.GetFileInfo(c.Params.FileId)
@@ -230,7 +237,7 @@ func getFilePreview(c *Context, w http.ResponseWriter, r *http.Request) {
 	if data, err := app.ReadFile(info.PreviewPath); err != nil {
 		c.Err = err
 		c.Err.StatusCode = http.StatusNotFound
-	} else if err := writeFileResponse(info.Name, info.MimeType, data, toDownload, w, r); err != nil {
+	} else if err := writeFileResponse(info.Name, info.MimeType, data, forceDownload, w, r); err != nil {
 		c.Err = err
 		return
 	}
@@ -298,14 +305,38 @@ func getPublicFile(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func writeFileResponse(filename string, contentType string, bytes []byte, toDownload bool, w http.ResponseWriter, r *http.Request) *model.AppError {
+func writeFileResponse(filename string, contentType string, bytes []byte, forceDownload bool, w http.ResponseWriter, r *http.Request) *model.AppError {
 	w.Header().Set("Cache-Control", "max-age=2592000, private")
 	w.Header().Set("Content-Length", strconv.Itoa(len(bytes)))
+	w.Header().Set("X-Content-Type-Options", "nosniff")
 
-	if contentType != "" {
+	if contentType == "" {
-		w.Header().Set("Content-Type", contentType)
+		contentType = "application/octet-stream"
 	} else {
-		w.Header().Del("Content-Type") // Content-Type will be set automatically by the http writer
+		for _, unsafeContentType := range UNSAFE_CONTENT_TYPES {
+			if strings.HasPrefix(contentType, unsafeContentType) {
+				contentType = "text/plain"
+				break
+			}
+		}
+	}
+
+	w.Header().Set("Content-Type", contentType)
+
+	var toDownload bool
+	if forceDownload {
+		toDownload = true
+	} else {
+		isMediaType := false
+
+		for _, mediaContentType := range MEDIA_CONTENT_TYPES {
+			if strings.HasPrefix(contentType, mediaContentType) {
+				isMediaType = true
+				break
+			}
+		}
+
+		toDownload = !isMediaType
 	}
 
 	if toDownload {