Commit c3cf9c0f authored by Stan Hu's avatar Stan Hu

Support encoded Content-Disposition fields

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24919/diffs adds
support for Rails sending the right `Content-Disposition` header when
there are non-ASCII filenames (e.g. `テスト.txt`). That example file
would render the following Content-Disposition:

```
attachment; filename="テスト.txt", filename*=UTF-8''%E3%83%86%E3%82%B9%E3%83%88.txt
```

Since mime.ParseMediaType appears to strip this header to a canonical
filename, it's not possible to use this to generate the right
Content-Disposition. Instead, we can preserve the existing
Content-Disposition and convert types just by doing a search and replace
(i.e. change `attachment` to `inline` and vice versa when necessary).

Closes https://gitlab.com/gitlab-org/gitlab-workhorse/issues/207
parent ff2634a5
...@@ -2,6 +2,10 @@ ...@@ -2,6 +2,10 @@
Formerly known as 'gitlab-git-http-server'. Formerly known as 'gitlab-git-http-server'.
v 8.3.0
- Support encoded Content-Disposition fields !360
v 8.2.0 v 8.2.0
- Sign LFS upload requests that have been handled by workhorse - Sign LFS upload requests that have been handled by workhorse
......
package headers package headers
import ( import (
"mime"
"net/http" "net/http"
"regexp" "regexp"
...@@ -17,6 +16,7 @@ var ( ...@@ -17,6 +16,7 @@ var (
VideoTypeRegex = regexp.MustCompile(`^video/*`) VideoTypeRegex = regexp.MustCompile(`^video/*`)
AttachmentRegex = regexp.MustCompile(`^attachment`) AttachmentRegex = regexp.MustCompile(`^attachment`)
InlineRegex = regexp.MustCompile(`^inline`)
) )
// Mime types that can't be inlined. Usually subtypes of main types // Mime types that can't be inlined. Usually subtypes of main types
...@@ -54,61 +54,52 @@ func safeContentType(data []byte) string { ...@@ -54,61 +54,52 @@ func safeContentType(data []byte) string {
} }
func safeContentDisposition(contentType string, contentDisposition string) string { func safeContentDisposition(contentType string, contentDisposition string) string {
existingDisposition, file := extractContentDispositionFile(contentDisposition)
// If the existing disposition is attachment we return that. This allow us // If the existing disposition is attachment we return that. This allow us
// to force a download from GitLab (ie: RawController) // to force a download from GitLab (ie: RawController)
if AttachmentRegex.MatchString(existingDisposition) { if AttachmentRegex.MatchString(contentDisposition) {
return attachmentDisposition(file) return contentDisposition
} }
// Checks for mime types that are forbidden to be inline // Checks for mime types that are forbidden to be inline
for _, element := range forbiddenInlineTypes { for _, element := range forbiddenInlineTypes {
if isType(contentType, element) { if isType(contentType, element) {
return attachmentDisposition(file) return attachmentDisposition(contentDisposition)
} }
} }
// Checks for mime types allowed to be inline // Checks for mime types allowed to be inline
for _, element := range allowedInlineTypes { for _, element := range allowedInlineTypes {
if isType(contentType, element) { if isType(contentType, element) {
return inlineDisposition(file) return inlineDisposition(contentDisposition)
} }
} }
// Anything else is set to attachment // Anything else is set to attachment
return attachmentDisposition(file) return attachmentDisposition(contentDisposition)
} }
func extractContentDispositionFile(disposition string) (string, string) { func attachmentDisposition(contentDisposition string) string {
if disposition == "" { if contentDisposition == "" {
return "", "" return "attachment"
} }
existingDisposition, params, err := mime.ParseMediaType(disposition) if InlineRegex.MatchString(contentDisposition) {
if err != nil { return InlineRegex.ReplaceAllString(contentDisposition, "attachment")
return "", ""
} }
return existingDisposition, params["filename"] return contentDisposition
}
func attachmentDisposition(file string) string {
return disposition("attachment", file)
} }
func inlineDisposition(file string) string { func inlineDisposition(contentDisposition string) string {
return disposition("inline", file) if contentDisposition == "" {
} return "inline"
}
func disposition(disposition string, file string) string {
params := map[string]string{}
if file != "" { if AttachmentRegex.MatchString(contentDisposition) {
params["filename"] = file return AttachmentRegex.ReplaceAllString(contentDisposition, "inline")
} }
return mime.FormatMediaType(disposition, params) return contentDisposition
} }
func isType(contentType string, mimeType *regexp.Regexp) bool { func isType(contentType string, mimeType *regexp.Regexp) bool {
......
...@@ -21,7 +21,7 @@ func TestFailSetContentTypeAndDisposition(t *testing.T) { ...@@ -21,7 +21,7 @@ func TestFailSetContentTypeAndDisposition(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
}) })
resp := makeRequest(t, h, testCaseBody) resp := makeRequest(t, h, testCaseBody, "")
require.Equal(t, "", resp.Header.Get(headers.ContentDispositionHeader)) require.Equal(t, "", resp.Header.Get(headers.ContentDispositionHeader))
require.Equal(t, "", resp.Header.Get(headers.ContentTypeHeader)) require.Equal(t, "", resp.Header.Get(headers.ContentTypeHeader))
...@@ -30,7 +30,7 @@ func TestFailSetContentTypeAndDisposition(t *testing.T) { ...@@ -30,7 +30,7 @@ func TestFailSetContentTypeAndDisposition(t *testing.T) {
func TestSuccessSetContentTypeAndDispositionFeatureEnabled(t *testing.T) { func TestSuccessSetContentTypeAndDispositionFeatureEnabled(t *testing.T) {
testCaseBody := "Hello world!" testCaseBody := "Hello world!"
resp := makeRequest(t, nil, testCaseBody) resp := makeRequest(t, nil, testCaseBody, "")
require.Equal(t, "inline", resp.Header.Get(headers.ContentDispositionHeader)) require.Equal(t, "inline", resp.Header.Get(headers.ContentDispositionHeader))
require.Equal(t, "text/plain; charset=utf-8", resp.Header.Get(headers.ContentTypeHeader)) require.Equal(t, "text/plain; charset=utf-8", resp.Header.Get(headers.ContentTypeHeader))
...@@ -145,11 +145,17 @@ func TestSetProperContentTypeAndDisposition(t *testing.T) { ...@@ -145,11 +145,17 @@ func TestSetProperContentTypeAndDisposition(t *testing.T) {
contentDisposition: "attachment", contentDisposition: "attachment",
body: testhelper.LoadFile(t, "testdata/file.sketch"), body: testhelper.LoadFile(t, "testdata/file.sketch"),
}, },
{
desc: "PDF file with non-ASCII characters in filename",
contentType: "application/pdf",
contentDisposition: `attachment; filename="file-ä.pdf"; filename*=UTF-8''file-%c3.pdf`,
body: testhelper.LoadFile(t, "testdata/file-ä.pdf"),
},
} }
for _, tc := range testCases { for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) { t.Run(tc.desc, func(t *testing.T) {
resp := makeRequest(t, nil, tc.body) resp := makeRequest(t, nil, tc.body, tc.contentDisposition)
require.Equal(t, tc.contentType, resp.Header.Get(headers.ContentTypeHeader)) require.Equal(t, tc.contentType, resp.Header.Get(headers.ContentTypeHeader))
require.Equal(t, tc.contentDisposition, resp.Header.Get(headers.ContentDispositionHeader)) require.Equal(t, tc.contentDisposition, resp.Header.Get(headers.ContentDispositionHeader))
...@@ -174,7 +180,7 @@ func TestFailOverrideContentType(t *testing.T) { ...@@ -174,7 +180,7 @@ func TestFailOverrideContentType(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
}) })
resp := makeRequest(t, h, testCase.body) resp := makeRequest(t, h, testCase.body, "")
require.Equal(t, testCase.contentType, resp.Header.Get(headers.ContentTypeHeader)) require.Equal(t, testCase.contentType, resp.Header.Get(headers.ContentTypeHeader))
} }
...@@ -190,7 +196,7 @@ func TestSuccessOverrideContentDispositionFromInlineToAttachment(t *testing.T) { ...@@ -190,7 +196,7 @@ func TestSuccessOverrideContentDispositionFromInlineToAttachment(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
}) })
resp := makeRequest(t, h, testCaseBody) resp := makeRequest(t, h, testCaseBody, "")
require.Equal(t, "attachment", resp.Header.Get(headers.ContentDispositionHeader)) require.Equal(t, "attachment", resp.Header.Get(headers.ContentDispositionHeader))
} }
...@@ -206,7 +212,7 @@ func TestFailOverrideContentDispositionFromAttachmentToInline(t *testing.T) { ...@@ -206,7 +212,7 @@ func TestFailOverrideContentDispositionFromAttachmentToInline(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
}) })
resp := makeRequest(t, h, testCaseBody) resp := makeRequest(t, h, testCaseBody, "")
require.Equal(t, "attachment", resp.Header.Get(headers.ContentDispositionHeader)) require.Equal(t, "attachment", resp.Header.Get(headers.ContentDispositionHeader))
} }
...@@ -240,11 +246,12 @@ func TestWriteHeadersCalledOnce(t *testing.T) { ...@@ -240,11 +246,12 @@ func TestWriteHeadersCalledOnce(t *testing.T) {
require.Equal(t, 400, rw.status) require.Equal(t, 400, rw.status)
} }
func makeRequest(t *testing.T, handler http.HandlerFunc, body string) *http.Response { func makeRequest(t *testing.T, handler http.HandlerFunc, body string, disposition string) *http.Response {
if handler == nil { if handler == nil {
handler = http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { handler = http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
// We are pretending to be upstream // We are pretending to be upstream
w.Header().Set(headers.GitlabWorkhorseDetectContentTypeHeader, "true") w.Header().Set(headers.GitlabWorkhorseDetectContentTypeHeader, "true")
w.Header().Set(headers.ContentDispositionHeader, disposition)
_, err := io.WriteString(w, body) _, err := io.WriteString(w, body)
require.NoError(t, err) require.NoError(t, err)
}) })
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment