diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-02-04 18:17:50 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-02-04 18:17:50 +0000 |
commit | 17a58755f31fcca5f6f42a4f7de1bbf1934dd038 (patch) | |
tree | 42b09381de05e0a8b046242f109d451ddf5cfff5 /workhorse/internal/upload | |
parent | 1b723130416e59bdaef5d662a33f22ac1d3ce953 (diff) | |
download | gitlab-ce-17a58755f31fcca5f6f42a4f7de1bbf1934dd038.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'workhorse/internal/upload')
-rw-r--r-- | workhorse/internal/upload/rewrite.go | 14 | ||||
-rw-r--r-- | workhorse/internal/upload/rewrite_test.go | 81 | ||||
-rw-r--r-- | workhorse/internal/upload/uploads_test.go | 86 |
3 files changed, 179 insertions, 2 deletions
diff --git a/workhorse/internal/upload/rewrite.go b/workhorse/internal/upload/rewrite.go index b9324ac8b7b..bbabe840ef5 100644 --- a/workhorse/internal/upload/rewrite.go +++ b/workhorse/internal/upload/rewrite.go @@ -6,8 +6,10 @@ import ( "fmt" "io" "io/ioutil" + "mime" "mime/multipart" "net/http" + "net/textproto" "os" "path/filepath" "strings" @@ -95,7 +97,8 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr return err } - name := p.FormName() + name, filename := parseAndNormalizeContentDisposition(p.Header) + if name == "" { continue } @@ -104,7 +107,7 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr return ErrInjectedClientParam } - if p.FileName() != "" { + if filename != "" { err = rew.handleFilePart(r.Context(), name, p, opts) } else { err = rew.copyPart(r.Context(), name, p) @@ -118,6 +121,13 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr return nil } +func parseAndNormalizeContentDisposition(header textproto.MIMEHeader) (string, string) { + const key = "Content-Disposition" + mediaType, params, _ := mime.ParseMediaType(header.Get(key)) + header.Set(key, mime.FormatMediaType(mediaType, params)) + return params["name"], params["filename"] +} + func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipart.Part, opts *filestore.SaveFileOpts) error { if rew.filter.Count() >= maxFilesAllowed { return ErrTooManyFilesUploaded diff --git a/workhorse/internal/upload/rewrite_test.go b/workhorse/internal/upload/rewrite_test.go index e3f33a02489..145f62ee910 100644 --- a/workhorse/internal/upload/rewrite_test.go +++ b/workhorse/internal/upload/rewrite_test.go @@ -1,6 +1,7 @@ package upload import ( + "net/textproto" "os" "runtime" "testing" @@ -54,3 +55,83 @@ func TestImageTypeRecongition(t *testing.T) { }) } } + +func TestParseAndNormalizeContentDisposition(t *testing.T) { + tests := []struct { + desc string + header string + name string + filename string + sanitizedHeader string + }{ + { + desc: "without content disposition", + header: "", + name: "", + filename: "", + sanitizedHeader: "", + }, { + desc: "content disposition without filename", + header: `form-data; name="filename"`, + name: "filename", + filename: "", + sanitizedHeader: `form-data; name=filename`, + }, { + desc: "with filename", + header: `form-data; name="file"; filename=foobar`, + name: "file", + filename: "foobar", + sanitizedHeader: `form-data; filename=foobar; name=file`, + }, { + desc: "with filename*", + header: `form-data; name="file"; filename*=UTF-8''bar`, + name: "file", + filename: "bar", + sanitizedHeader: `form-data; filename=bar; name=file`, + }, { + desc: "filename and filename*", + header: `form-data; name="file"; filename=foobar; filename*=UTF-8''bar`, + name: "file", + filename: "bar", + sanitizedHeader: `form-data; filename=bar; name=file`, + }, { + desc: "with empty filename", + header: `form-data; name="file"; filename=""`, + name: "file", + filename: "", + sanitizedHeader: `form-data; filename=""; name=file`, + }, { + desc: "with complex filename*", + header: `form-data; name="file"; filename*=UTF-8''viel%20Spa%C3%9F`, + name: "file", + filename: "viel Spaß", + sanitizedHeader: `form-data; filename*=utf-8''viel%20Spa%C3%9F; name=file`, + }, { + desc: "with unsupported charset", + header: `form-data; name="file"; filename*=UTF-16''bar`, + name: "file", + filename: "", + sanitizedHeader: `form-data; name=file`, + }, { + desc: "with filename and filename* with unsupported charset", + header: `form-data; name="file"; filename=foobar; filename*=UTF-16''bar`, + name: "file", + filename: "foobar", + sanitizedHeader: `form-data; filename=foobar; name=file`, + }, + } + + for _, testCase := range tests { + t.Run(testCase.desc, func(t *testing.T) { + h := make(textproto.MIMEHeader) + h.Set("Content-Disposition", testCase.header) + h.Set("Content-Type", "application/octet-stream") + + name, filename := parseAndNormalizeContentDisposition(h) + + require.Equal(t, testCase.name, name) + require.Equal(t, testCase.filename, filename) + require.Equal(t, testCase.sanitizedHeader, h.Get("Content-Disposition")) + }) + } +} diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go index 7c22b3b4e28..31fd3126dd4 100644 --- a/workhorse/internal/upload/uploads_test.go +++ b/workhorse/internal/upload/uploads_test.go @@ -1,13 +1,16 @@ package upload import ( + "bufio" "bytes" "context" "fmt" + "io" "io/ioutil" "mime/multipart" "net/http" "net/http/httptest" + "net/textproto" "os" "regexp" "strconv" @@ -384,6 +387,89 @@ func TestInvalidFileNames(t *testing.T) { } } +func TestContentDispositionRewrite(t *testing.T) { + testhelper.ConfigureSecret() + + tempPath, err := ioutil.TempDir("", "uploads") + require.NoError(t, err) + defer os.RemoveAll(tempPath) + + tests := []struct { + desc string + header string + code int + sanitizedHeader string + }{ + { + desc: "with name", + header: `form-data; name="foo"`, + code: 200, + sanitizedHeader: `form-data; name=foo`, + }, + { + desc: "with name and name*", + header: `form-data; name="foo"; name*=UTF-8''bar`, + code: 200, + sanitizedHeader: `form-data; name=bar`, + }, + { + desc: "with name and invalid name*", + header: `form-data; name="foo"; name*=UTF-16''bar`, + code: 200, + sanitizedHeader: `form-data; name=foo`, + }, + } + + for _, testCase := range tests { + t.Run(testCase.desc, func(t *testing.T) { + h := make(textproto.MIMEHeader) + h.Set("Content-Disposition", testCase.header) + h.Set("Content-Type", "application/octet-stream") + + buffer := &bytes.Buffer{} + writer := multipart.NewWriter(buffer) + file, err := writer.CreatePart(h) + require.NoError(t, err) + fmt.Fprint(file, "test") + writer.Close() + + httpRequest := httptest.NewRequest("POST", "/example", buffer) + httpRequest.Header.Set("Content-Type", writer.FormDataContentType()) + + var upstreamRequestBuffer bytes.Buffer + customHandler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + r.Write(&upstreamRequestBuffer) + }) + + response := httptest.NewRecorder() + apiResponse := &api.Response{TempPath: tempPath} + preparer := &DefaultPreparer{} + opts, _, err := preparer.Prepare(apiResponse) + require.NoError(t, err) + + HandleFileUploads(response, httpRequest, customHandler, apiResponse, &SavedFileTracker{Request: httpRequest}, opts) + + upstreamRequest, err := http.ReadRequest(bufio.NewReader(&upstreamRequestBuffer)) + require.NoError(t, err) + + reader, err := upstreamRequest.MultipartReader() + require.NoError(t, err) + + for i := 0; ; i++ { + p, err := reader.NextPart() + if err == io.EOF { + require.Equal(t, i, 1) + break + } + require.NoError(t, err) + require.Equal(t, testCase.sanitizedHeader, p.Header.Get("Content-Disposition")) + } + + require.Equal(t, testCase.code, response.Code) + }) + } +} + func TestUploadHandlerRemovingExif(t *testing.T) { content, err := ioutil.ReadFile("exif/testdata/sample_exif.jpg") require.NoError(t, err) |