Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
path: root/workhorse/internal/upload
diff options
context:
space:
mode:
Diffstat (limited to 'workhorse/internal/upload')
-rw-r--r--workhorse/internal/upload/accelerate.go11
-rw-r--r--workhorse/internal/upload/body_uploader.go35
-rw-r--r--workhorse/internal/upload/body_uploader_test.go12
-rw-r--r--workhorse/internal/upload/rewrite.go14
-rw-r--r--workhorse/internal/upload/rewrite_test.go81
-rw-r--r--workhorse/internal/upload/skip_rails_authorizer.go10
-rw-r--r--workhorse/internal/upload/uploads.go8
-rw-r--r--workhorse/internal/upload/uploads_test.go114
8 files changed, 236 insertions, 49 deletions
diff --git a/workhorse/internal/upload/accelerate.go b/workhorse/internal/upload/accelerate.go
index 81f44d33a82..28d3b3dee2e 100644
--- a/workhorse/internal/upload/accelerate.go
+++ b/workhorse/internal/upload/accelerate.go
@@ -17,16 +17,21 @@ type MultipartClaims struct {
jwt.StandardClaims
}
-func Accelerate(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
+// Multipart is a request middleware. If the request has a MIME multipart
+// request body, the middleware will iterate through the multipart parts.
+// When it finds a file part (filename != ""), the middleware will save
+// the file contents to a temporary location and replace the file part
+// with a reference to the temporary location.
+func Multipart(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
s := &SavedFileTracker{Request: r}
opts, _, err := p.Prepare(a)
if err != nil {
- helper.Fail500(w, r, fmt.Errorf("Accelerate: error preparing file storage options"))
+ helper.Fail500(w, r, fmt.Errorf("Multipart: error preparing file storage options"))
return
}
- HandleFileUploads(w, r, h, a, s, opts)
+ InterceptMultipartFiles(w, r, h, a, s, opts)
}, "/authorize")
}
diff --git a/workhorse/internal/upload/body_uploader.go b/workhorse/internal/upload/body_uploader.go
index 4849d9bae75..6c53bd9241b 100644
--- a/workhorse/internal/upload/body_uploader.go
+++ b/workhorse/internal/upload/body_uploader.go
@@ -16,16 +16,23 @@ type PreAuthorizer interface {
PreAuthorizeHandler(next api.HandleFunc, suffix string) http.Handler
}
-// Verifier allows to check an upload before sending it to rails
+// Verifier is an optional pluggable behavior for upload paths. If
+// Verify() returns an error, Workhorse will return an error response to
+// the client instead of propagating the request to Rails. The motivating
+// use case is Git LFS, where Workhorse checks the size and SHA256
+// checksum of the uploaded file.
type Verifier interface {
- // Verify can abort the upload returning an error
+ // Verify can abort the upload by returning an error
Verify(handler *filestore.FileHandler) error
}
-// Preparer allows to customize BodyUploader configuration
+// Preparer is a pluggable behavior that interprets a Rails API response
+// and either tells Workhorse how to handle the upload, via the
+// SaveFileOpts and Verifier, or it rejects the request by returning a
+// non-nil error. Its intended use is to make sure the upload gets stored
+// in the right location: either a local directory, or one of several
+// supported object storage backends.
type Preparer interface {
- // Prepare converts api.Response into a *SaveFileOpts, it can optionally return an Verifier that will be
- // invoked after the real upload, before the finalization with rails
Prepare(a *api.Response) (*filestore.SaveFileOpts, Verifier, error)
}
@@ -36,26 +43,26 @@ func (s *DefaultPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, Ver
return opts, nil, err
}
-// BodyUploader is an http.Handler that perform a pre authorization call to rails before hijacking the request body and
-// uploading it.
-// Providing an Preparer allows to customize the upload process
-func BodyUploader(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
+// RequestBody is a request middleware. It will store the request body to
+// a location by determined an api.Response value. It then forwards the
+// request to gitlab-rails without the original request body.
+func RequestBody(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
opts, verifier, err := p.Prepare(a)
if err != nil {
- helper.Fail500(w, r, fmt.Errorf("BodyUploader: preparation failed: %v", err))
+ helper.Fail500(w, r, fmt.Errorf("RequestBody: preparation failed: %v", err))
return
}
fh, err := filestore.SaveFileFromReader(r.Context(), r.Body, r.ContentLength, opts)
if err != nil {
- helper.Fail500(w, r, fmt.Errorf("BodyUploader: upload failed: %v", err))
+ helper.Fail500(w, r, fmt.Errorf("RequestBody: upload failed: %v", err))
return
}
if verifier != nil {
if err := verifier.Verify(fh); err != nil {
- helper.Fail500(w, r, fmt.Errorf("BodyUploader: verification failed: %v", err))
+ helper.Fail500(w, r, fmt.Errorf("RequestBody: verification failed: %v", err))
return
}
}
@@ -63,7 +70,7 @@ func BodyUploader(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler
data := url.Values{}
fields, err := fh.GitLabFinalizeFields("file")
if err != nil {
- helper.Fail500(w, r, fmt.Errorf("BodyUploader: finalize fields failed: %v", err))
+ helper.Fail500(w, r, fmt.Errorf("RequestBody: finalize fields failed: %v", err))
return
}
@@ -80,7 +87,7 @@ func BodyUploader(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler
sft := SavedFileTracker{Request: r}
sft.Track("file", fh.LocalPath)
if err := sft.Finalize(r.Context()); err != nil {
- helper.Fail500(w, r, fmt.Errorf("BodyUploader: finalize failed: %v", err))
+ helper.Fail500(w, r, fmt.Errorf("RequestBody: finalize failed: %v", err))
return
}
diff --git a/workhorse/internal/upload/body_uploader_test.go b/workhorse/internal/upload/body_uploader_test.go
index aeb366616ca..b3d561ac131 100644
--- a/workhorse/internal/upload/body_uploader_test.go
+++ b/workhorse/internal/upload/body_uploader_test.go
@@ -24,7 +24,7 @@ const (
fileLen = len(fileContent)
)
-func TestBodyUploader(t *testing.T) {
+func TestRequestBody(t *testing.T) {
testhelper.ConfigureSecret()
body := strings.NewReader(fileContent)
@@ -38,7 +38,7 @@ func TestBodyUploader(t *testing.T) {
require.Equal(t, fileContent, string(uploadEcho))
}
-func TestBodyUploaderCustomPreparer(t *testing.T) {
+func TestRequestBodyCustomPreparer(t *testing.T) {
body := strings.NewReader(fileContent)
resp := testUpload(&rails{}, &alwaysLocalPreparer{}, echoProxy(t, fileLen), body)
@@ -49,7 +49,7 @@ func TestBodyUploaderCustomPreparer(t *testing.T) {
require.Equal(t, fileContent, string(uploadEcho))
}
-func TestBodyUploaderCustomVerifier(t *testing.T) {
+func TestRequestBodyCustomVerifier(t *testing.T) {
body := strings.NewReader(fileContent)
verifier := &mockVerifier{}
@@ -62,11 +62,11 @@ func TestBodyUploaderCustomVerifier(t *testing.T) {
require.True(t, verifier.invoked, "Verifier.Verify not invoked")
}
-func TestBodyUploaderAuthorizationFailure(t *testing.T) {
+func TestRequestBodyAuthorizationFailure(t *testing.T) {
testNoProxyInvocation(t, http.StatusUnauthorized, &rails{unauthorized: true}, &alwaysLocalPreparer{})
}
-func TestBodyUploaderErrors(t *testing.T) {
+func TestRequestBodyErrors(t *testing.T) {
tests := []struct {
name string
preparer *alwaysLocalPreparer
@@ -95,7 +95,7 @@ func testUpload(auth PreAuthorizer, preparer Preparer, proxy http.Handler, body
req := httptest.NewRequest("POST", "http://example.com/upload", body)
w := httptest.NewRecorder()
- BodyUploader(auth, proxy, preparer).ServeHTTP(w, req)
+ RequestBody(auth, proxy, preparer).ServeHTTP(w, req)
return w.Result()
}
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/skip_rails_authorizer.go b/workhorse/internal/upload/skip_rails_authorizer.go
index c8055a673eb..e74048fb6e3 100644
--- a/workhorse/internal/upload/skip_rails_authorizer.go
+++ b/workhorse/internal/upload/skip_rails_authorizer.go
@@ -6,15 +6,15 @@ import (
"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
)
-// SkipRailsAuthorizer implements a fake PreAuthorizer that do not calls rails API and
-// authorize each call as a local only upload to TempPath
+// SkipRailsAuthorizer implements a fake PreAuthorizer that does not call
+// the gitlab-rails API. It must be fast because it gets called on each
+// request proxied to Rails.
type SkipRailsAuthorizer struct {
- // TempPath is the temporary path for a local only upload
+ // TempPath is a directory where workhorse can store files that can later
+ // be accessed by gitlab-rails.
TempPath string
}
-// PreAuthorizeHandler implements PreAuthorizer. It always grant the upload.
-// The fake API response contains only TempPath
func (l *SkipRailsAuthorizer) PreAuthorizeHandler(next api.HandleFunc, _ string) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
next(w, r, &api.Response{TempPath: l.TempPath})
diff --git a/workhorse/internal/upload/uploads.go b/workhorse/internal/upload/uploads.go
index b408d260379..1806e7563ce 100644
--- a/workhorse/internal/upload/uploads.go
+++ b/workhorse/internal/upload/uploads.go
@@ -15,7 +15,8 @@ import (
"gitlab.com/gitlab-org/gitlab/workhorse/internal/zipartifacts"
)
-// These methods are allowed to have thread-unsafe implementations.
+// MultipartFormProcessor abstracts away implementation differences
+// between generic MIME multipart file uploads and CI artifact uploads.
type MultipartFormProcessor interface {
ProcessFile(ctx context.Context, formName string, file *filestore.FileHandler, writer *multipart.Writer) error
ProcessField(ctx context.Context, formName string, writer *multipart.Writer) error
@@ -24,7 +25,10 @@ type MultipartFormProcessor interface {
Count() int
}
-func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, preauth *api.Response, filter MultipartFormProcessor, opts *filestore.SaveFileOpts) {
+// InterceptMultipartFiles is the core of the implementation of
+// Multipart. Because it is also used for CI artifact uploads it is a
+// public function.
+func InterceptMultipartFiles(w http.ResponseWriter, r *http.Request, h http.Handler, preauth *api.Response, filter MultipartFormProcessor, opts *filestore.SaveFileOpts) {
var body bytes.Buffer
writer := multipart.NewWriter(&body)
defer writer.Close()
diff --git a/workhorse/internal/upload/uploads_test.go b/workhorse/internal/upload/uploads_test.go
index 7c22b3b4e28..f262bf94b08 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"
@@ -75,7 +78,7 @@ func TestUploadHandlerForwardingRawData(t *testing.T) {
opts, _, err := preparer.Prepare(apiResponse)
require.NoError(t, err)
- HandleFileUploads(response, httpRequest, handler, apiResponse, nil, opts)
+ InterceptMultipartFiles(response, httpRequest, handler, apiResponse, nil, opts)
require.Equal(t, 202, response.Code)
require.Equal(t, "RESPONSE", response.Body.String(), "response body")
@@ -146,7 +149,7 @@ func TestUploadHandlerRewritingMultiPartData(t *testing.T) {
opts, _, err := preparer.Prepare(apiResponse)
require.NoError(t, err)
- HandleFileUploads(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts)
+ InterceptMultipartFiles(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts)
require.Equal(t, 202, response.Code)
cancel() // this will trigger an async cleanup
@@ -215,7 +218,7 @@ func TestUploadHandlerDetectingInjectedMultiPartData(t *testing.T) {
opts, _, err := preparer.Prepare(apiResponse)
require.NoError(t, err)
- HandleFileUploads(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts)
+ InterceptMultipartFiles(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts)
require.Equal(t, test.response, response.Code)
cancel() // this will trigger an async cleanup
@@ -245,7 +248,7 @@ func TestUploadProcessingField(t *testing.T) {
opts, _, err := preparer.Prepare(apiResponse)
require.NoError(t, err)
- HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
+ InterceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
require.Equal(t, 500, response.Code)
}
@@ -276,7 +279,7 @@ func TestUploadingMultipleFiles(t *testing.T) {
opts, _, err := preparer.Prepare(apiResponse)
require.NoError(t, err)
- HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
+ InterceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
require.Equal(t, 400, response.Code)
require.Equal(t, "upload request contains more than 10 files\n", response.Body.String())
@@ -332,7 +335,7 @@ func TestUploadProcessingFile(t *testing.T) {
opts, _, err := preparer.Prepare(apiResponse)
require.NoError(t, err)
- HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
+ InterceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &testFormProcessor{}, opts)
require.Equal(t, 200, response.Code)
})
@@ -378,12 +381,95 @@ func TestInvalidFileNames(t *testing.T) {
opts, _, err := preparer.Prepare(apiResponse)
require.NoError(t, err)
- HandleFileUploads(response, httpRequest, nilHandler, apiResponse, &SavedFileTracker{Request: httpRequest}, opts)
+ InterceptMultipartFiles(response, httpRequest, nilHandler, apiResponse, &SavedFileTracker{Request: httpRequest}, opts)
require.Equal(t, testCase.code, response.Code)
require.Equal(t, testCase.expectedPrefix, opts.TempFilePrefix)
}
}
+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)
+
+ InterceptMultipartFiles(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)
@@ -484,7 +570,7 @@ func runUploadTest(t *testing.T, image []byte, filename string, httpCode int, ts
opts, _, err := preparer.Prepare(apiResponse)
require.NoError(t, err)
- HandleFileUploads(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts)
+ InterceptMultipartFiles(response, httpRequest, handler, apiResponse, &testFormProcessor{}, opts)
require.Equal(t, httpCode, response.Code)
}
@@ -495,15 +581,9 @@ func newProxy(url string) *proxy.Proxy {
func waitUntilDeleted(t *testing.T, path string) {
var err error
-
- // Poll because the file removal is async
- for i := 0; i < 100; i++ {
+ require.Eventually(t, func() bool {
_, err = os.Stat(path)
- if err != nil {
- break
- }
- time.Sleep(100 * time.Millisecond)
- }
-
+ return err != nil
+ }, 10*time.Second, 10*time.Millisecond)
require.True(t, os.IsNotExist(err), "expected the file to be deleted")
}
generated by cgit v1.2.3 (git 2.25.1) at 2024-09-11 21:50:14 +0300