Merge branch 'revert-94fa6381' into 'master'

Revert "Merge branch '340366_fix_workhorse_content_disposition' into 'master'"

See merge request gitlab-org/gitlab!78695

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

workhorse/internal/upload/rewrite_test.go

 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}))
-		})
-	}
-
-}

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)

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)