diff --git a/pkg/handler/get_test.go b/pkg/handler/get_test.go index b3a26b54..a2b2e26b 100644 --- a/pkg/handler/get_test.go +++ b/pkg/handler/get_test.go @@ -110,7 +110,7 @@ func TestGet(t *testing.T) { upload.EXPECT().GetInfo(gomock.Any()).Return(FileInfo{ Offset: 0, MetaData: map[string]string{ - "filetype": "non-a-valid-mime-type", + "filetype": "non a valid mime type", }, }, nil), ) diff --git a/pkg/handler/unrouted_handler.go b/pkg/handler/unrouted_handler.go index 1e1998a7..20d0ed83 100644 --- a/pkg/handler/unrouted_handler.go +++ b/pkg/handler/unrouted_handler.go @@ -34,7 +34,6 @@ const ( var ( reForwardedHost = regexp.MustCompile(`host="?([^;"]+)`) reForwardedProto = regexp.MustCompile(`proto=(https?)`) - reMimeType = regexp.MustCompile(`^[a-z]+\/[a-z0-9\-\+\.]+$`) // We only allow certain URL-safe characters in upload IDs. URL-safe in this means // that their are allowed in a URI's path component according to RFC 3986. // See https://datatracker.ietf.org/doc/html/rfc3986#section-3.3 @@ -1112,9 +1111,9 @@ func (handler *UnroutedHandler) GetFile(w http.ResponseWriter, r *http.Request) // mimeInlineBrowserWhitelist is a map containing MIME types which should be // allowed to be rendered by browser inline, instead of being forced to be // downloaded. For example, HTML or SVG files are not allowed, since they may -// contain malicious JavaScript. In a similiar fashion PDF is not on this list +// contain malicious JavaScript. In a similar fashion, PDF is not on this list // as their parsers commonly contain vulnerabilities which can be exploited. -// The values of this map does not convey any meaning and are therefore just +// The values of this map do not convey any meaning and are therefore just // empty structs. var mimeInlineBrowserWhitelist = map[string]struct{}{ "text/plain": {}, @@ -1125,14 +1124,17 @@ var mimeInlineBrowserWhitelist = map[string]struct{}{ "image/bmp": {}, "image/webp": {}, - "audio/wave": {}, - "audio/wav": {}, - "audio/x-wav": {}, - "audio/x-pn-wav": {}, - "audio/webm": {}, - "video/webm": {}, - "audio/ogg": {}, - "video/ogg": {}, + "audio/wave": {}, + "audio/wav": {}, + "audio/x-wav": {}, + "audio/x-pn-wav": {}, + "audio/webm": {}, + "audio/ogg": {}, + + "video/mp4": {}, + "video/webm": {}, + "video/ogg": {}, + "application/ogg": {}, } @@ -1140,23 +1142,22 @@ var mimeInlineBrowserWhitelist = map[string]struct{}{ // Content-Disposition headers for a given upload. These values should be used // in responses for GET requests to ensure that only non-malicious file types // are shown directly in the browser. It will extract the file name and type -// from the "fileame" and "filetype". +// from the "filename" and "filetype". // See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition func filterContentType(info FileInfo) (contentType string, contentDisposition string) { filetype := info.MetaData["filetype"] - if reMimeType.MatchString(filetype) { - // If the filetype from metadata is well formed, we forward use this - // for the Content-Type header. However, only whitelisted mime types - // will be allowed to be shown inline in the browser + if ft, _, err := mime.ParseMediaType(filetype); err == nil { + // If the filetype from metadata is well-formed, we forward use this for the Content-Type header. + // However, only allowlisted mime types will be allowed to be shown inline in the browser contentType = filetype - if _, isWhitelisted := mimeInlineBrowserWhitelist[filetype]; isWhitelisted { + if _, isWhitelisted := mimeInlineBrowserWhitelist[ft]; isWhitelisted { contentDisposition = "inline" } else { contentDisposition = "attachment" } } else { - // If the filetype from the metadata is not well formed, we use a + // If the filetype from the metadata is not well-formed, we use a // default type and force the browser to download the content. contentType = "application/octet-stream" contentDisposition = "attachment" diff --git a/pkg/handler/unrouted_handler_test.go b/pkg/handler/unrouted_handler_test.go index 6800d5fd..5d1eb7e8 100644 --- a/pkg/handler/unrouted_handler_test.go +++ b/pkg/handler/unrouted_handler_test.go @@ -1,11 +1,9 @@ -package handler_test +package handler import ( "testing" "github.com/stretchr/testify/assert" - - . "github.com/tus/tusd/v2/pkg/handler" ) func TestParseMetadataHeader(t *testing.T) { @@ -33,3 +31,77 @@ func TestParseMetadataHeader(t *testing.T) { "k4": "world", }) } + +func TestFilterContentType(t *testing.T) { + tests := map[string]struct { + input FileInfo + contentType string + contentDisposition string + }{ + "without metadata": { + input: FileInfo{MetaData: map[string]string{}}, + contentType: "application/octet-stream", + contentDisposition: "attachment", + }, + "filetype allowlisted": { + input: FileInfo{MetaData: map[string]string{ + "filetype": "image/png", + "filename": "pet.png", + }}, + contentType: "image/png", + contentDisposition: "inline;filename=\"pet.png\"", + }, + "filetype not allowlisted": { + input: FileInfo{MetaData: map[string]string{ + "filetype": "application/zip", + "filename": "pets.zip", + }}, + contentType: "application/zip", + contentDisposition: "attachment;filename=\"pets.zip\"", + }, + "filetype with valid parameters": { + input: FileInfo{MetaData: map[string]string{ + "filetype": "audio/ogg; codecs=opus", + "filename": "pet.opus", + }}, + contentType: "audio/ogg; codecs=opus", + contentDisposition: "inline;filename=\"pet.opus\"", + }, + "filetype with invalid parameters": { + input: FileInfo{MetaData: map[string]string{ + "filetype": "text/plain; invalid-param", + "filename": "pet.txt", + }}, + contentType: "application/octet-stream", + contentDisposition: "attachment;filename=\"pet.txt\"", + }, + "filetype with duplicate parameters": { + input: FileInfo{MetaData: map[string]string{ + "filetype": "text/plain; charset=utf-8; charset=us-ascii", + "filename": "pet.txt", + }}, + contentType: "application/octet-stream", + contentDisposition: "attachment;filename=\"pet.txt\"", + }, + "filetype invalid": { + input: FileInfo{MetaData: map[string]string{ + "filetype": "invalid media type", + "filename": "pet.imt", + }}, + contentType: "application/octet-stream", + contentDisposition: "attachment;filename=\"pet.imt\"", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + a := assert.New(t) + + gotContentType, gotContentDisposition := filterContentType(test.input) + + a.Equal(test.contentType, gotContentType) + a.Equal(test.contentDisposition, gotContentDisposition) + }) + } +}