diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index 9bdf37f03eeeb..49b10850ce891 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -24,7 +24,7 @@ func NewAttachment(ctx context.Context, attach *repo_model.Attachment, file io.R return nil, fmt.Errorf("attachment %s should belong to a repository", attach.Name) } - err := db.WithTx(ctx, func(ctx context.Context) error { + if err := db.WithTx(ctx, func(ctx context.Context) error { attach.UUID = uuid.New().String() size, err := storage.Attachments.Save(attach.RelativePath(), file, size) if err != nil { @@ -33,9 +33,11 @@ func NewAttachment(ctx context.Context, attach *repo_model.Attachment, file io.R attach.Size = size return db.Insert(ctx, attach) - }) + }); err != nil { + return nil, err + } - return attach, err + return attach, nil } type ErrAttachmentSizeExceed struct { @@ -43,14 +45,22 @@ type ErrAttachmentSizeExceed struct { Size int64 } -func (e ErrAttachmentSizeExceed) Error() string { +func (e *ErrAttachmentSizeExceed) Error() string { + if e.Size == 0 { + return fmt.Sprintf("attachment size exceeds limit %d", e.MaxSize) + } return fmt.Sprintf("attachment size %d exceeds limit %d", e.Size, e.MaxSize) } -func (e ErrAttachmentSizeExceed) Unwrap() error { +func (e *ErrAttachmentSizeExceed) Unwrap() error { return util.ErrContentTooLarge } +func (e *ErrAttachmentSizeExceed) Is(target error) bool { + _, ok := target.(*ErrAttachmentSizeExceed) + return ok +} + // UploadAttachment upload new attachment into storage and update database func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, maxFileSize, fileSize int64, attach *repo_model.Attachment) (*repo_model.Attachment, error) { buf := make([]byte, 1024) @@ -61,11 +71,20 @@ func UploadAttachment(ctx context.Context, file io.Reader, allowedTypes string, return nil, err } - if maxFileSize >= 0 && fileSize > maxFileSize { - return nil, ErrAttachmentSizeExceed{MaxSize: maxFileSize, Size: fileSize} + reader := io.MultiReader(bytes.NewReader(buf), file) + + // enforce file size limit + if maxFileSize >= 0 { + if fileSize > maxFileSize { + return nil, &ErrAttachmentSizeExceed{MaxSize: maxFileSize, Size: fileSize} + } + // limit reader to max file size with additional 1k more, + // to allow side-cases where encoding tells us its exactly maxFileSize but the actual created file is bit more, + // while still make sure the limit is enforced + reader = attachmentLimitedReader(reader, maxFileSize+1024) } - return NewAttachment(ctx, attach, io.MultiReader(bytes.NewReader(buf), file), fileSize) + return NewAttachment(ctx, attach, reader, fileSize) } // UpdateAttachment updates an attachment, verifying that its name is among the allowed types. diff --git a/services/attachment/attachment_test.go b/services/attachment/attachment_test.go index 8ecac8d7a33ec..522bf10476e63 100644 --- a/services/attachment/attachment_test.go +++ b/services/attachment/attachment_test.go @@ -15,31 +15,77 @@ import ( _ "code.gitea.io/gitea/models/actions" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMain(m *testing.M) { unittest.MainTest(m) } -func TestUploadAttachment(t *testing.T) { +func TestNewAttachment(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) fPath := "./attachment_test.go" f, err := os.Open(fPath) - assert.NoError(t, err) + require.NoError(t, err) defer f.Close() + fs, err := f.Stat() + require.NoError(t, err) attach, err := NewAttachment(t.Context(), &repo_model.Attachment{ RepoID: 1, UploaderID: user.ID, Name: filepath.Base(fPath), - }, f, -1) + }, f, fs.Size()) assert.NoError(t, err) + assert.Equal(t, fs.Size(), attach.Size) attachment, err := repo_model.GetAttachmentByUUID(t.Context(), attach.UUID) assert.NoError(t, err) assert.Equal(t, user.ID, attachment.UploaderID) assert.Equal(t, int64(0), attachment.DownloadCount) } + +func TestUploadAttachment(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + fPath := "./attachment_test.go" + f, err := os.Open(fPath) + require.NoError(t, err) + defer f.Close() + fs, err := f.Stat() + require.NoError(t, err) + + t.Run("size to big", func(t *testing.T) { + attach, err := UploadAttachment(t.Context(), f, "", 10, fs.Size(), &repo_model.Attachment{ + RepoID: 1, + UploaderID: user.ID, + Name: filepath.Base(fPath), + }) + assert.ErrorIs(t, err, &ErrAttachmentSizeExceed{}) + assert.Nil(t, attach) + }) + + t.Run("size was lied about", func(t *testing.T) { + attach, err := UploadAttachment(t.Context(), f, "", 10, 10, &repo_model.Attachment{ + RepoID: 1, + UploaderID: user.ID, + Name: filepath.Base(fPath), + }) + assert.ErrorIs(t, err, &ErrAttachmentSizeExceed{}) + assert.Nil(t, attach) + }) + + t.Run("size was correct", func(t *testing.T) { + attach, err := UploadAttachment(t.Context(), f, "", fs.Size(), fs.Size(), &repo_model.Attachment{ + RepoID: 1, + UploaderID: user.ID, + Name: filepath.Base(fPath), + }) + assert.NoError(t, err) + require.NotNil(t, attach) + assert.Equal(t, user.ID, attach.UploaderID) + }) +} diff --git a/services/attachment/reader.go b/services/attachment/reader.go new file mode 100644 index 0000000000000..db0e44301eec0 --- /dev/null +++ b/services/attachment/reader.go @@ -0,0 +1,35 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package attachment + +import "io" + +// modified version of io.LimitReader: https://cs.opensource.google/go/go/+/refs/tags/go1.25.1:src/io/io.go;l=458-482 + +// attachmentLimitedReader returns a Reader that reads from r +// but errors with ErrAttachmentSizeExceed after n bytes. +// The underlying implementation is a *attachmentReader. +func attachmentLimitedReader(r io.Reader, n int64) io.Reader { return &attachmentReader{r, n} } + +// A attachmentReader reads from R but limits the amount of +// data returned to just N bytes. Each call to Read +// updates N to reflect the new amount remaining. +// Read returns ErrAttachmentSizeExceed when N <= 0. +// Underlying errors are passed through. +type attachmentReader struct { + R io.Reader // underlying reader + N int64 // max bytes remaining +} + +func (l *attachmentReader) Read(p []byte) (n int, err error) { + if l.N <= 0 { + return 0, &ErrAttachmentSizeExceed{MaxSize: l.N} + } + if int64(len(p)) > l.N { + p = p[0:l.N] + } + n, err = l.R.Read(p) + l.N -= int64(n) + return +}