From 7e84b56289eeef4385ee207b9387e6c89a8547d1 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Thu, 8 Jun 2023 04:32:37 +0000 Subject: [PATCH 1/9] fix --- modules/git/blob.go | 11 +++++++++++ routers/web/user/profile.go | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/modules/git/blob.go b/modules/git/blob.go index 8864f54d1bf28..5e126bace1ae1 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -33,6 +33,17 @@ func (b *Blob) GetBlobContent() (string, error) { return string(buf), nil } +// GetBlobAll Gets the all content of the blob as bytes +func (b *Blob) GetBlobAll() ([]byte, error) { + dataRc, err := b.DataAsync() + if err != nil { + return nil, err + } + buf, _ := io.ReadAll(dataRc) + _ = dataRc.Close() + return buf, nil +} + // GetBlobLineCount gets line count of the blob func (b *Blob) GetBlobLineCount() (int, error) { reader, err := b.DataAsync() diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 42ae37e3ba302..49c2a0a228e43 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -107,7 +107,7 @@ func Profile(ctx *context.Context) { } blob, err := commit.GetBlobByPath("README.md") if err == nil { - bytes, err := blob.GetBlobContent() + bytes, err := blob.GetBlobAll() if err != nil { ctx.ServerError("GetBlobContent", err) return @@ -115,7 +115,7 @@ func Profile(ctx *context.Context) { profileContent, err := markdown.RenderString(&markup.RenderContext{ Ctx: ctx, GitRepo: gitRepo, - }, bytes) + }, string(bytes)) if err != nil { ctx.ServerError("RenderString", err) return From 71d84754e3444253b3630ba204de437942a0287d Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Thu, 8 Jun 2023 07:27:40 +0000 Subject: [PATCH 2/9] improve --- modules/git/blob.go | 15 +++++++++++---- routers/web/user/profile.go | 2 +- services/repository/files/content.go | 2 +- tests/integration/api_packages_cargo_test.go | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/modules/git/blob.go b/modules/git/blob.go index 5e126bace1ae1..4845e5892335f 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -21,15 +21,22 @@ func (b *Blob) Name() string { } // GetBlobContent Gets the content of the blob as raw text -func (b *Blob) GetBlobContent() (string, error) { +// limit=0 means no limit, limit>0 means limit the content length +func (b *Blob) GetBlobContent(limit int64) (string, error) { dataRc, err := b.DataAsync() if err != nil { return "", err } defer dataRc.Close() - buf := make([]byte, 1024) - n, _ := util.ReadAtMost(dataRc, buf) - buf = buf[:n] + + var buf []byte + if limit == 0 { + buf, _ = io.ReadAll(dataRc) + } else { + buf = make([]byte, limit) + n, _ := util.ReadAtMost(dataRc, buf) + buf = buf[:n] + } return string(buf), nil } diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 49c2a0a228e43..71f3677aa0c28 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -107,7 +107,7 @@ func Profile(ctx *context.Context) { } blob, err := commit.GetBlobByPath("README.md") if err == nil { - bytes, err := blob.GetBlobAll() + bytes, err := blob.GetBlobContent(0) if err != nil { ctx.ServerError("GetBlobContent", err) return diff --git a/services/repository/files/content.go b/services/repository/files/content.go index 6f6dc91d859ab..2dd692e69e2bd 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -203,7 +203,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref } else if entry.IsLink() { contentsResponse.Type = string(ContentTypeLink) // The target of a symlink file is the content of the file - targetFromContent, err := entry.Blob().GetBlobContent() + targetFromContent, err := entry.Blob().GetBlobContent(0) if err != nil { return nil, err } diff --git a/tests/integration/api_packages_cargo_test.go b/tests/integration/api_packages_cargo_test.go index 608f19296819f..59dae6ad00296 100644 --- a/tests/integration/api_packages_cargo_test.go +++ b/tests/integration/api_packages_cargo_test.go @@ -88,7 +88,7 @@ func testPackageCargo(t *testing.T, _ *neturl.URL) { blob, err := commit.GetBlobByPath(path) assert.NoError(t, err) - content, err := blob.GetBlobContent() + content, err := blob.GetBlobContent(0) assert.NoError(t, err) return content From d5af14723631efe4b0e3dcea5b9e9ce0ae3cd032 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Thu, 8 Jun 2023 07:28:08 +0000 Subject: [PATCH 3/9] remove code --- modules/git/blob.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/modules/git/blob.go b/modules/git/blob.go index 4845e5892335f..fd4220de2fb39 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -40,17 +40,6 @@ func (b *Blob) GetBlobContent(limit int64) (string, error) { return string(buf), nil } -// GetBlobAll Gets the all content of the blob as bytes -func (b *Blob) GetBlobAll() ([]byte, error) { - dataRc, err := b.DataAsync() - if err != nil { - return nil, err - } - buf, _ := io.ReadAll(dataRc) - _ = dataRc.Close() - return buf, nil -} - // GetBlobLineCount gets line count of the blob func (b *Blob) GetBlobLineCount() (int, error) { reader, err := b.DataAsync() From 110196bf9e65f2578bb5d0c0e6df89cb1d8059e1 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Thu, 8 Jun 2023 07:42:04 +0000 Subject: [PATCH 4/9] add 1024 limit for old use --- services/repository/files/content.go | 2 +- tests/integration/api_packages_cargo_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/repository/files/content.go b/services/repository/files/content.go index 2dd692e69e2bd..c701431d6785d 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -203,7 +203,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref } else if entry.IsLink() { contentsResponse.Type = string(ContentTypeLink) // The target of a symlink file is the content of the file - targetFromContent, err := entry.Blob().GetBlobContent(0) + targetFromContent, err := entry.Blob().GetBlobContent(1024) if err != nil { return nil, err } diff --git a/tests/integration/api_packages_cargo_test.go b/tests/integration/api_packages_cargo_test.go index 59dae6ad00296..03d8e0c5207e7 100644 --- a/tests/integration/api_packages_cargo_test.go +++ b/tests/integration/api_packages_cargo_test.go @@ -88,7 +88,7 @@ func testPackageCargo(t *testing.T, _ *neturl.URL) { blob, err := commit.GetBlobByPath(path) assert.NoError(t, err) - content, err := blob.GetBlobContent(0) + content, err := blob.GetBlobContent(1024) assert.NoError(t, err) return content From 888cb8bbbce2c4b71e9590c5e691187c765a60b5 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Thu, 8 Jun 2023 07:42:42 +0000 Subject: [PATCH 5/9] fix lint --- routers/web/user/profile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 71f3677aa0c28..0ea92f34c9693 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -115,7 +115,7 @@ func Profile(ctx *context.Context) { profileContent, err := markdown.RenderString(&markup.RenderContext{ Ctx: ctx, GitRepo: gitRepo, - }, string(bytes)) + }, bytes) if err != nil { ctx.ServerError("RenderString", err) return From 3afc0d298a918e1e35d3b3343a580a9ced47dc71 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 13 Jun 2023 02:56:58 +0000 Subject: [PATCH 6/9] depends on MAX_DISPLAY_FILE_SIZE --- modules/git/blob.go | 17 ++++++----------- routers/web/user/profile.go | 2 +- services/repository/files/content.go | 2 +- tests/integration/api_packages_cargo_test.go | 2 +- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/modules/git/blob.go b/modules/git/blob.go index fd4220de2fb39..dfaf0ff9e8ce9 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -9,6 +9,7 @@ import ( "encoding/base64" "io" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/typesniffer" "code.gitea.io/gitea/modules/util" ) @@ -21,22 +22,16 @@ func (b *Blob) Name() string { } // GetBlobContent Gets the content of the blob as raw text -// limit=0 means no limit, limit>0 means limit the content length -func (b *Blob) GetBlobContent(limit int64) (string, error) { +// The max content size depends on MAX_DISPLAY_FILE_SIZE in app.ini +func (b *Blob) GetBlobContent() (string, error) { dataRc, err := b.DataAsync() if err != nil { return "", err } defer dataRc.Close() - - var buf []byte - if limit == 0 { - buf, _ = io.ReadAll(dataRc) - } else { - buf = make([]byte, limit) - n, _ := util.ReadAtMost(dataRc, buf) - buf = buf[:n] - } + buf := make([]byte, setting.UI.MaxDisplayFileSize) + n, _ := util.ReadAtMost(dataRc, buf) + buf = buf[:n] return string(buf), nil } diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 0ea92f34c9693..42ae37e3ba302 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -107,7 +107,7 @@ func Profile(ctx *context.Context) { } blob, err := commit.GetBlobByPath("README.md") if err == nil { - bytes, err := blob.GetBlobContent(0) + bytes, err := blob.GetBlobContent() if err != nil { ctx.ServerError("GetBlobContent", err) return diff --git a/services/repository/files/content.go b/services/repository/files/content.go index c701431d6785d..6f6dc91d859ab 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -203,7 +203,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref } else if entry.IsLink() { contentsResponse.Type = string(ContentTypeLink) // The target of a symlink file is the content of the file - targetFromContent, err := entry.Blob().GetBlobContent(1024) + targetFromContent, err := entry.Blob().GetBlobContent() if err != nil { return nil, err } diff --git a/tests/integration/api_packages_cargo_test.go b/tests/integration/api_packages_cargo_test.go index 03d8e0c5207e7..608f19296819f 100644 --- a/tests/integration/api_packages_cargo_test.go +++ b/tests/integration/api_packages_cargo_test.go @@ -88,7 +88,7 @@ func testPackageCargo(t *testing.T, _ *neturl.URL) { blob, err := commit.GetBlobByPath(path) assert.NoError(t, err) - content, err := blob.GetBlobContent(1024) + content, err := blob.GetBlobContent() assert.NoError(t, err) return content From d16e328b4d2d0e6c9181c734f9c881fa070f611b Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 13 Jun 2023 04:14:35 +0000 Subject: [PATCH 7/9] fix --- models/issues/pull.go | 2 +- modules/git/blob.go | 11 ++++++----- routers/web/repo/view.go | 2 +- routers/web/user/profile.go | 2 +- services/repository/files/content.go | 2 +- tests/integration/api_packages_cargo_test.go | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/models/issues/pull.go b/models/issues/pull.go index 23b938f95ab76..2acc2b4226e0b 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -920,7 +920,7 @@ func PullRequestCodeOwnersReview(ctx context.Context, pull *Issue, pr *PullReque var data string for _, file := range files { if blob, err := commit.GetBlobByPath(file); err == nil { - data, err = blob.GetBlobContent() + data, err = blob.GetBlobContent(setting.UI.MaxDisplayFileSize) if err == nil { break } diff --git a/modules/git/blob.go b/modules/git/blob.go index dfaf0ff9e8ce9..ba14800acb42d 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -9,7 +9,6 @@ import ( "encoding/base64" "io" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/typesniffer" "code.gitea.io/gitea/modules/util" ) @@ -21,15 +20,17 @@ func (b *Blob) Name() string { return b.name } -// GetBlobContent Gets the content of the blob as raw text -// The max content size depends on MAX_DISPLAY_FILE_SIZE in app.ini -func (b *Blob) GetBlobContent() (string, error) { +// GetBlobContent Gets the limited content of the blob as raw text +func (b *Blob) GetBlobContent(limit int64) (string, error) { + if limit <= 0 { + return "", nil + } dataRc, err := b.DataAsync() if err != nil { return "", err } defer dataRc.Close() - buf := make([]byte, setting.UI.MaxDisplayFileSize) + buf := make([]byte, limit) n, _ := util.ReadAtMost(dataRc, buf) buf = buf[:n] return string(buf), nil diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 1d54f25884e39..cf719c49f0b29 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -363,7 +363,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st ctx.Data["FileError"] = ctx.Locale.Tr("actions.runs.invalid_workflow_helper", workFlowErr.Error()) } } else if util.SliceContains([]string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"}, ctx.Repo.TreePath) { - if data, err := blob.GetBlobContent(); err == nil { + if data, err := blob.GetBlobContent(setting.UI.MaxDisplayFileSize); err == nil { _, warnings := issue_model.GetCodeOwnersFromContent(ctx, data) if len(warnings) > 0 { ctx.Data["FileWarning"] = strings.Join(warnings, "\n") diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 42ae37e3ba302..6f9f84d60dbdf 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -107,7 +107,7 @@ func Profile(ctx *context.Context) { } blob, err := commit.GetBlobByPath("README.md") if err == nil { - bytes, err := blob.GetBlobContent() + bytes, err := blob.GetBlobContent(setting.UI.MaxDisplayFileSize) if err != nil { ctx.ServerError("GetBlobContent", err) return diff --git a/services/repository/files/content.go b/services/repository/files/content.go index 6f6dc91d859ab..2cd3e49be27ec 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -203,7 +203,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref } else if entry.IsLink() { contentsResponse.Type = string(ContentTypeLink) // The target of a symlink file is the content of the file - targetFromContent, err := entry.Blob().GetBlobContent() + targetFromContent, err := entry.Blob().GetBlobContent(setting.UI.MaxDisplayFileSize) if err != nil { return nil, err } diff --git a/tests/integration/api_packages_cargo_test.go b/tests/integration/api_packages_cargo_test.go index 608f19296819f..03d8e0c5207e7 100644 --- a/tests/integration/api_packages_cargo_test.go +++ b/tests/integration/api_packages_cargo_test.go @@ -88,7 +88,7 @@ func testPackageCargo(t *testing.T, _ *neturl.URL) { blob, err := commit.GetBlobByPath(path) assert.NoError(t, err) - content, err := blob.GetBlobContent() + content, err := blob.GetBlobContent(1024) assert.NoError(t, err) return content From 72c8f83c93f162660bf7099015658182b15cd763 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 13 Jun 2023 12:21:49 +0800 Subject: [PATCH 8/9] Update services/repository/files/content.go --- services/repository/files/content.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/repository/files/content.go b/services/repository/files/content.go index 2cd3e49be27ec..c701431d6785d 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -203,7 +203,7 @@ func GetContents(ctx context.Context, repo *repo_model.Repository, treePath, ref } else if entry.IsLink() { contentsResponse.Type = string(ContentTypeLink) // The target of a symlink file is the content of the file - targetFromContent, err := entry.Blob().GetBlobContent(setting.UI.MaxDisplayFileSize) + targetFromContent, err := entry.Blob().GetBlobContent(1024) if err != nil { return nil, err } From 0e2e0d8382123728fd8dd99e4e71b7a88f5f259e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 13 Jun 2023 12:55:57 +0800 Subject: [PATCH 9/9] fix memory usage --- modules/git/blob.go | 6 ++-- modules/util/io.go | 39 +++++++++++++++++++++++- modules/util/io_test.go | 66 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 modules/util/io_test.go diff --git a/modules/git/blob.go b/modules/git/blob.go index ba14800acb42d..bcecb42e16ebb 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -30,10 +30,8 @@ func (b *Blob) GetBlobContent(limit int64) (string, error) { return "", err } defer dataRc.Close() - buf := make([]byte, limit) - n, _ := util.ReadAtMost(dataRc, buf) - buf = buf[:n] - return string(buf), nil + buf, err := util.ReadWithLimit(dataRc, int(limit)) + return string(buf), err } // GetBlobLineCount gets line count of the blob diff --git a/modules/util/io.go b/modules/util/io.go index 69b1d63145a49..1559b019a0637 100644 --- a/modules/util/io.go +++ b/modules/util/io.go @@ -4,13 +4,14 @@ package util import ( + "bytes" "errors" "io" ) // ReadAtMost reads at most len(buf) bytes from r into buf. // It returns the number of bytes copied. n is only less than len(buf) if r provides fewer bytes. -// If EOF occurs while reading, err will be nil. +// If EOF or ErrUnexpectedEOF occurs while reading, err will be nil. func ReadAtMost(r io.Reader, buf []byte) (n int, err error) { n, err = io.ReadFull(r, buf) if err == io.EOF || err == io.ErrUnexpectedEOF { @@ -19,6 +20,42 @@ func ReadAtMost(r io.Reader, buf []byte) (n int, err error) { return n, err } +// ReadWithLimit reads at most "limit" bytes from r into buf. +// If EOF or ErrUnexpectedEOF occurs while reading, err will be nil. +func ReadWithLimit(r io.Reader, n int) (buf []byte, err error) { + return readWithLimit(r, 1024, n) +} + +func readWithLimit(r io.Reader, batch, limit int) ([]byte, error) { + if limit <= batch { + buf := make([]byte, limit) + n, err := ReadAtMost(r, buf) + if err != nil { + return nil, err + } + return buf[:n], nil + } + res := bytes.NewBuffer(make([]byte, 0, batch)) + bufFix := make([]byte, batch) + eof := false + for res.Len() < limit && !eof { + bufTmp := bufFix + if res.Len()+batch > limit { + bufTmp = bufFix[:limit-res.Len()] + } + n, err := io.ReadFull(r, bufTmp) + if err == io.EOF || err == io.ErrUnexpectedEOF { + eof = true + } else if err != nil { + return nil, err + } + if _, err = res.Write(bufTmp[:n]); err != nil { + return nil, err + } + } + return res.Bytes(), nil +} + // ErrNotEmpty is an error reported when there is a non-empty reader var ErrNotEmpty = errors.New("not-empty") diff --git a/modules/util/io_test.go b/modules/util/io_test.go new file mode 100644 index 0000000000000..275575463a632 --- /dev/null +++ b/modules/util/io_test.go @@ -0,0 +1,66 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package util + +import ( + "bytes" + "errors" + "testing" + + "github.com/stretchr/testify/assert" +) + +type readerWithError struct { + buf *bytes.Buffer +} + +func (r *readerWithError) Read(p []byte) (n int, err error) { + if r.buf.Len() < 2 { + return 0, errors.New("test error") + } + return r.buf.Read(p) +} + +func TestReadWithLimit(t *testing.T) { + bs := []byte("0123456789abcdef") + + // normal test + buf, err := readWithLimit(bytes.NewBuffer(bs), 5, 2) + assert.NoError(t, err) + assert.Equal(t, []byte("01"), buf) + + buf, err = readWithLimit(bytes.NewBuffer(bs), 5, 5) + assert.NoError(t, err) + assert.Equal(t, []byte("01234"), buf) + + buf, err = readWithLimit(bytes.NewBuffer(bs), 5, 6) + assert.NoError(t, err) + assert.Equal(t, []byte("012345"), buf) + + buf, err = readWithLimit(bytes.NewBuffer(bs), 5, len(bs)) + assert.NoError(t, err) + assert.Equal(t, []byte("0123456789abcdef"), buf) + + buf, err = readWithLimit(bytes.NewBuffer(bs), 5, 100) + assert.NoError(t, err) + assert.Equal(t, []byte("0123456789abcdef"), buf) + + // test with error + buf, err = readWithLimit(&readerWithError{bytes.NewBuffer(bs)}, 5, 10) + assert.NoError(t, err) + assert.Equal(t, []byte("0123456789"), buf) + + buf, err = readWithLimit(&readerWithError{bytes.NewBuffer(bs)}, 5, 100) + assert.ErrorContains(t, err, "test error") + assert.Empty(t, buf) + + // test public function + buf, err = ReadWithLimit(bytes.NewBuffer(bs), 2) + assert.NoError(t, err) + assert.Equal(t, []byte("01"), buf) + + buf, err = ReadWithLimit(bytes.NewBuffer(bs), 9999999) + assert.NoError(t, err) + assert.Equal(t, []byte("0123456789abcdef"), buf) +}