diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-20 18:14:18 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-01-20 18:14:18 +0000 |
commit | 39cb2fdf01699eb5ac000c918f469c58dc75f7e8 (patch) | |
tree | 5de21a06dfe8b97c793f892032be45949aa482db /workhorse/internal/upload | |
parent | c17eb7c97062d25cdf1b44573e4c0241f52aa2fe (diff) | |
download | gitlab-ce-39cb2fdf01699eb5ac000c918f469c58dc75f7e8.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'workhorse/internal/upload')
-rw-r--r-- | workhorse/internal/upload/rewrite.go | 17 | ||||
-rw-r--r-- | workhorse/internal/upload/rewrite_test.go | 46 | ||||
-rw-r--r-- | workhorse/internal/upload/uploads.go | 2 | ||||
-rw-r--r-- | workhorse/internal/upload/uploads_test.go | 95 |
4 files changed, 1 insertions, 159 deletions
diff --git a/workhorse/internal/upload/rewrite.go b/workhorse/internal/upload/rewrite.go index 8101ca2b698..b9324ac8b7b 100644 --- a/workhorse/internal/upload/rewrite.go +++ b/workhorse/internal/upload/rewrite.go @@ -10,7 +10,6 @@ import ( "net/http" "os" "path/filepath" - "regexp" "strings" "github.com/prometheus/client_golang/prometheus" @@ -31,11 +30,8 @@ const maxFilesAllowed = 10 var ( ErrInjectedClientParam = errors.New("injected client parameter") ErrTooManyFilesUploaded = fmt.Errorf("upload request contains more than %v files", maxFilesAllowed) - ErrUnexpectedFilePart = errors.New("Content-Disposition contains unexpected filepart") ) -var filePartRegex = regexp.MustCompile(`;\s*filename\b`) - var ( multipartUploadRequests = promauto.NewCounterVec( prometheus.CounterOpts{ @@ -111,11 +107,6 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr if p.FileName() != "" { err = rew.handleFilePart(r.Context(), name, p, opts) } else { - err = verifyContentDisposition(p) - if err != nil { - return err - } - err = rew.copyPart(r.Context(), name, p) } @@ -127,14 +118,6 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, pr return nil } -func verifyContentDisposition(p *multipart.Part) error { - if filePartRegex.MatchString(p.Header.Get("Content-Disposition")) { - return ErrUnexpectedFilePart - } - - return nil -} - 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 9ae83d069b5..e3f33a02489 100644 --- a/workhorse/internal/upload/rewrite_test.go +++ b/workhorse/internal/upload/rewrite_test.go @@ -1,8 +1,6 @@ package upload import ( - "mime/multipart" - "net/textproto" "os" "runtime" "testing" @@ -56,47 +54,3 @@ func TestImageTypeRecongition(t *testing.T) { }) } } - -func TestVerifyContentDisposition(t *testing.T) { - tests := []struct { - desc string - contentDisposition string - error error - }{ - { - desc: "without content disposition", - contentDisposition: "", - error: nil, - }, { - desc: "content disposition without filename", - contentDisposition: `form-data; name="filename"`, - error: nil, - }, { - desc: "with filename", - contentDisposition: `form-data; name="file"; filename=foobar`, - error: ErrUnexpectedFilePart, - }, { - desc: "with filename*", - contentDisposition: `form-data; name="file"; filename*=UTF-8''foobar`, - error: ErrUnexpectedFilePart, - }, { - desc: "filename and filename*", - contentDisposition: `form-data; name="file"; filename=foobar; filename*=UTF-8''foobar`, - error: ErrUnexpectedFilePart, - }, - } - - for _, testCase := range tests { - t.Run(testCase.desc, func(t *testing.T) { - h := make(textproto.MIMEHeader) - - if testCase.contentDisposition != "" { - h.Set("Content-Disposition", testCase.contentDisposition) - h.Set("Content-Type", "application/octet-stream") - } - - require.Equal(t, testCase.error, verifyContentDisposition(&multipart.Part{Header: h})) - }) - } - -} diff --git a/workhorse/internal/upload/uploads.go b/workhorse/internal/upload/uploads.go index c7daa9bbf18..b408d260379 100644 --- a/workhorse/internal/upload/uploads.go +++ b/workhorse/internal/upload/uploads.go @@ -35,7 +35,7 @@ func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, p switch err { case ErrInjectedClientParam: helper.CaptureAndFail(w, r, err, "Bad Request", http.StatusBadRequest) - case ErrTooManyFilesUploaded, ErrUnexpectedFilePart: + case ErrTooManyFilesUploaded: helper.CaptureAndFail(w, r, err, err.Error(), http.StatusBadRequest) case http.ErrNotMultipart: h.ServeHTTP(w, r) diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go index d238895c2bd..7c22b3b4e28 100644 --- a/workhorse/internal/upload/uploads_test.go +++ b/workhorse/internal/upload/uploads_test.go @@ -4,12 +4,10 @@ import ( "bytes" "context" "fmt" - "io" "io/ioutil" "mime/multipart" "net/http" "net/http/httptest" - "net/textproto" "os" "regexp" "strconv" @@ -386,99 +384,6 @@ func TestInvalidFileNames(t *testing.T) { } } -func TestContentDisposition(t *testing.T) { - testhelper.ConfigureSecret() - - tempPath, err := ioutil.TempDir("", "uploads") - require.NoError(t, err) - defer os.RemoveAll(tempPath) - - tests := []struct { - desc string - contentDisposition string - code int - body string - }{ - { - desc: "empty header", - contentDisposition: "", - code: 200, - body: "", - }, - { - desc: "without filename", - contentDisposition: `form-data; name="filename"`, - code: 200, - body: "", - }, - { - desc: "with filename", - contentDisposition: `form-data; name="file"; filename=foobar`, - code: 200, - body: "", - }, - { - desc: "with filename* supported charset", - contentDisposition: `form-data; name="file"; filename*=UTF-8''foobar`, - code: 200, - body: "", - }, - { - desc: "with both filename and filename* supported charset", - contentDisposition: `form-data; name="file"; filename=foobar; filename*=UTF-8''foobar`, - code: 200, - body: "", - }, - { - desc: "with filename and filename* unsupported charset", - contentDisposition: `form-data; name="file"; filename=foobar; filename*=UTF-16''foobar`, - code: 200, - body: "", - }, - { - desc: "unsupported charset", - contentDisposition: `form-data; name="file"; filename*=UTF-16''foobar`, - code: 400, - body: ErrUnexpectedFilePart.Error() + "\n", - }, - } - - for _, testCase := range tests { - t.Run(testCase.desc, func(t *testing.T) { - // Create custom Content-Disposition with filename* and charset - // Example: filename*=UTF-8''application.log - h := make(textproto.MIMEHeader) - - h.Set("Content-Disposition", testCase.contentDisposition) - 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()) - - response := httptest.NewRecorder() - apiResponse := &api.Response{TempPath: tempPath} - preparer := &DefaultPreparer{} - opts, _, err := preparer.Prepare(apiResponse) - require.NoError(t, err) - - HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &SavedFileTracker{Request: httpRequest}, opts) - - body, err := io.ReadAll(response.Body) - require.NoError(t, err) - - require.Equal(t, testCase.code, response.Code) - require.Equal(t, testCase.body, string(body)) - }) - } -} - func TestUploadHandlerRemovingExif(t *testing.T) { content, err := ioutil.ReadFile("exif/testdata/sample_exif.jpg") require.NoError(t, err) |