diff --git a/.deadcode-out b/.deadcode-out index f71ef41ea2..20f13a03a1 100644 --- a/.deadcode-out +++ b/.deadcode-out @@ -238,6 +238,7 @@ forgejo.org/services/repository forgejo.org/services/repository/files ContentType.String + RepoFileOptionMode forgejo.org/services/repository/gitgraph Parser.Reset diff --git a/modules/git/blob.go b/modules/git/blob.go index e2dc624e86..020a591a23 100644 --- a/modules/git/blob.go +++ b/modules/git/blob.go @@ -141,6 +141,28 @@ func (b *Blob) Name() string { return b.name } +// NewReader return a blob-reader which fails immediately with [BlobTooLargeError] if the file is bigger than the limit +func (b *Blob) NewReader(limit int64) (rc io.ReadCloser, actualSize int64, err error) { + actualSize = b.Size() + if actualSize > limit { + return nil, actualSize, BlobTooLargeError{ + Size: actualSize, + Limit: limit, + } + } + r, _, cancel, err := b.newReader() + if err != nil { + return nil, actualSize, err + } + + return &blobReader{ + rd: r, + n: actualSize, + additionalDiscard: 0, + cancel: cancel, + }, actualSize, nil +} + // NewTruncatedReader return a blob-reader which silently truncates when the limit is reached (io.EOF will be returned) func (b *Blob) NewTruncatedReader(limit int64) (rc io.ReadCloser, fullSize int64, err error) { r, fullSize, cancel, err := b.newReader() @@ -168,14 +190,7 @@ func (b BlobTooLargeError) Error() string { // GetContentBase64 Reads the content of the blob and returns it as base64 encoded string. // Returns [BlobTooLargeError] if the (unencoded) content is larger than the limit. func (b *Blob) GetContentBase64(limit int64) (string, error) { - if b.Size() > limit { - return "", BlobTooLargeError{ - Size: b.Size(), - Limit: limit, - } - } - - rc, size, err := b.NewTruncatedReader(limit) + rc, size, err := b.NewReader(limit) if err != nil { return "", err } diff --git a/modules/git/submodule.go b/modules/git/submodule.go index 4ea97d66eb..3d05657478 100644 --- a/modules/git/submodule.go +++ b/modules/git/submodule.go @@ -5,6 +5,7 @@ package git import ( + "errors" "fmt" "io" "net" @@ -19,6 +20,8 @@ import ( "gopkg.in/ini.v1" //nolint:depguard // used to read .gitmodules ) +const MaxGitmodulesFileSize = 64 * 1024 + // GetSubmodule returns the Submodule of a given path func (c *Commit) GetSubmodule(path string, entry *TreeEntry) (Submodule, error) { err := c.readSubmodules() @@ -55,8 +58,12 @@ func (c *Commit) readSubmodules() error { return err } - rc, _, err := entry.Blob().NewTruncatedReader(10 * 1024) + rc, _, err := entry.Blob().NewReader(MaxGitmodulesFileSize) if err != nil { + if errors.As(err, &BlobTooLargeError{}) { + c.submodules = make(map[string]Submodule) + return nil + } return err } defer rc.Close() diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 40fc64b5d0..51920dda8f 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -247,5 +247,6 @@ "editor.toggle_case": "Toggle case sensitivity", "editor.toggle_regex": "Toggle using regular expressions", "editor.toggle_whole_word": "Toggle matching whole words", + "repo.view.gitmodules_too_large": "The .gitmodules file is too large and will be ignored (on API calls for instance)", "meta.last_line": "Thank you for translating Forgejo! This line isn't seen by the users but it serves other purposes in the translation management. You can place a fun fact in the translation instead of translating it." } diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 74ed38de65..4d44a485b9 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -445,6 +445,10 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) { ctx.Data["FileWarning"] = strings.Join(warnings, "\n") } } + } else if ctx.Repo.TreePath == ".gitmodules" { + if fInfo.fileSize > git.MaxGitmodulesFileSize { + ctx.Data["FileWarning"] = ctx.Locale.Tr("repo.view.gitmodules_too_large") + } } isDisplayingSource := ctx.FormString("display") == "source" diff --git a/services/repository/files/update.go b/services/repository/files/update.go index f45ed384e7..9c2fde1c0e 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -43,7 +43,6 @@ type ChangeRepoFile struct { ContentReader io.ReadSeeker SHA string Options *RepoFileOptions - Symlink bool } // ChangeRepoFilesOptions holds the repository files update options @@ -62,8 +61,13 @@ type ChangeRepoFilesOptions struct { type RepoFileOptions struct { treePath string fromTreePath string - executable bool - symlink bool + entryMode git.EntryMode +} + +func RepoFileOptionMode(entryMode git.EntryMode) *RepoFileOptions { + return &RepoFileOptions{ + entryMode: entryMode, + } } // ChangeRepoFiles adds, updates or removes multiple files in the given repository @@ -114,11 +118,14 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } } + mode := git.EntryModeBlob + if file.Options != nil && file.Options.entryMode != 0 { + mode = file.Options.entryMode + } file.Options = &RepoFileOptions{ treePath: treePath, fromTreePath: fromTreePath, - executable: false, - symlink: file.Symlink, + entryMode: mode, } treePaths = append(treePaths, treePath) } @@ -320,7 +327,7 @@ func handleCheckErrors(file *ChangeRepoFile, actualBaseCommit *git.Commit, lastK // haven't been made. We throw an error if one wasn't provided. return models.ErrSHAOrCommitIDNotProvided{} } - file.Options.executable = fromEntry.IsExecutable() + file.Options.entryMode = fromEntry.Mode() } if file.Operation == "create" || file.Operation == "update" { // For the path where this file will be created/updated, we need to make @@ -430,13 +437,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file } // Add the object to the index - mode := "100644" // regular file - if file.Options.executable { - mode = "100755" - } else if file.Options.symlink { - mode = "120644" - } - if err := t.AddObjectToIndex(mode, objectHash, file.Options.treePath); err != nil { + if err := t.AddObjectToIndex(file.Options.entryMode.String(), objectHash, file.Options.treePath); err != nil { return err } diff --git a/tests/integration/repo_generate_test.go b/tests/integration/repo_generate_test.go index 22c2cc1633..af3415566a 100644 --- a/tests/integration/repo_generate_test.go +++ b/tests/integration/repo_generate_test.go @@ -343,7 +343,7 @@ func TestRepoGenerateTemplatingSymlink(t *testing.T) { Operation: "create", TreePath: "problem/Readme.md", ContentReader: strings.NewReader(tc.symlinkTarget), - Symlink: true, + Options: files_service.RepoFileOptionMode(git.EntryModeSymlink), }, }), }) @@ -420,7 +420,7 @@ func TestRepoGenerateTemplatingSymlinkGlobFile(t *testing.T) { Operation: "create", TreePath: ".forgejo/template", ContentReader: strings.NewReader("/etc/passwd"), - Symlink: true, + Options: files_service.RepoFileOptionMode(git.EntryModeSymlink), }, }), }) diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go index 159363668d..c7bfad32bd 100644 --- a/tests/integration/repo_test.go +++ b/tests/integration/repo_test.go @@ -1551,42 +1551,130 @@ func TestRepoIssueFilterLinks(t *testing.T) { func TestRepoSubmoduleView(t *testing.T) { onApplicationRun(t, func(t *testing.T, u *url.URL) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - repo, _, f := tests.CreateDeclarativeRepo(t, user2, "", []unit_model.Type{unit_model.TypeCode}, nil, nil) - defer f() + t.Run("FromGit", func(t *testing.T) { + repo, _, f := tests.CreateDeclarativeRepo(t, user2, "", []unit_model.Type{unit_model.TypeCode}, nil, nil) + defer f() - // Clone the repository, add a submodule and push it. - dstPath := t.TempDir() + // Clone the repository, add a submodule and push it. + dstPath := t.TempDir() - uClone := *u - uClone.Path = repo.FullName() - uClone.User = url.UserPassword(user2.Name, userPassword) + uClone := *u + uClone.Path = repo.FullName() + uClone.User = url.UserPassword(user2.Name, userPassword) - t.Run("Clone", doGitClone(dstPath, &uClone)) + t.Run("Clone", doGitClone(dstPath, &uClone)) - _, _, err := git.NewCommand(git.DefaultContext, "submodule", "add").AddDynamicArguments(u.JoinPath("/user2/repo1").String()).RunStdString(&git.RunOpts{Dir: dstPath}) - require.NoError(t, err) + _, _, err := git.NewCommand(git.DefaultContext, "submodule", "add").AddDynamicArguments(u.JoinPath("/user2/repo1").String()).RunStdString(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) - _, _, err = git.NewCommand(git.DefaultContext, "add", "repo1", ".gitmodules").RunStdString(&git.RunOpts{Dir: dstPath}) - require.NoError(t, err) + _, _, err = git.NewCommand(git.DefaultContext, "add", "repo1", ".gitmodules").RunStdString(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) - _, _, err = git.NewCommand(git.DefaultContext, "commit", "-m", "add submodule").RunStdString(&git.RunOpts{Dir: dstPath}) - require.NoError(t, err) + _, _, err = git.NewCommand(git.DefaultContext, "commit", "-m", "add submodule").RunStdString(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) - _, _, err = git.NewCommand(git.DefaultContext, "push").RunStdString(&git.RunOpts{Dir: dstPath}) - require.NoError(t, err) + _, _, err = git.NewCommand(git.DefaultContext, "push").RunStdString(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) - // Check that the submodule entry exist and the link is correct. - req := NewRequest(t, "GET", "/"+repo.FullName()) - resp := MakeRequest(t, req, http.StatusOK) + // Check that the submodule entry exist and the link is correct. + req := NewRequest(t, "GET", "/"+repo.FullName()) + resp := MakeRequest(t, req, http.StatusOK) - htmlDoc := NewHTMLParser(t, resp.Body) - htmlDoc.AssertElement(t, fmt.Sprintf(`tr[data-entryname="repo1"] a[href="%s"]`, u.JoinPath("/user2/repo1").String()), true) + htmlDoc := NewHTMLParser(t, resp.Body) + htmlDoc.AssertElement(t, fmt.Sprintf(`tr[data-entryname="repo1"] a[href="%s"]`, u.JoinPath("/user2/repo1").String()), true) - // Check that a link to the submodule returns a redirect and that the redirect link is correct. - req = NewRequest(t, "GET", "/"+repo.FullName()+"/src/branch/"+repo.DefaultBranch+"/repo1") - resp = MakeRequest(t, req, http.StatusSeeOther) + // Check that a link to the submodule returns a redirect and that the redirect link is correct. + req = NewRequest(t, "GET", "/"+repo.FullName()+"/src/branch/"+repo.DefaultBranch+"/repo1") + resp = MakeRequest(t, req, http.StatusSeeOther) - assert.Equal(t, u.JoinPath("/user2/repo1").String(), resp.Header().Get("Location")) + assert.Equal(t, u.JoinPath("/user2/repo1").String(), resp.Header().Get("Location")) + }) + + t.Run("Declarative", func(t *testing.T) { + repo, _, f := tests.CreateDeclarativeRepo(t, user2, "", []unit_model.Type{unit_model.TypeCode}, nil, []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: ".gitmodules", + ContentReader: strings.NewReader(`[submodule "relative-module"] + path = relative-module + url = https://git.example.org/submodule.git +`), + }, { + Operation: "create", + TreePath: "relative-module", + FromTreePath: "", + ContentReader: nil, + SHA: "95601d16476a", + Options: files_service.RepoFileOptionMode(git.EntryModeCommit), + }, + }) + defer f() + + // Check that the submodule entry exist and the link is correct. + req := NewRequest(t, "GET", "/"+repo.FullName()) + resp := MakeRequest(t, req, http.StatusOK) + + expectedDst := "https://git.example.org/submodule" + htmlDoc := NewHTMLParser(t, resp.Body) + + href, ok := htmlDoc.Find(`tr[data-entryname="relative-module"] a`).Attr("href") + assert.True(t, ok, "could not find entry 'relative-module' in file list") + assert.Equal(t, expectedDst, href) + + // Check that a link to the submodule returns a redirect and that the redirect link is correct. + req = NewRequest(t, "GET", "/"+repo.FullName()+"/src/branch/"+repo.DefaultBranch+"/relative-module") + resp = MakeRequest(t, req, http.StatusSeeOther) + + assert.Equal(t, expectedDst, resp.Header().Get("Location")) + }) + + t.Run("SubmodulesFileTooBig", func(t *testing.T) { + repo, _, f := tests.CreateDeclarativeRepo(t, user2, "", []unit_model.Type{unit_model.TypeCode}, nil, []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: ".gitmodules", + ContentReader: strings.NewReader(strings.Repeat("#", git.MaxGitmodulesFileSize-5) + // ensure that the partial read is invalid + ` +[submodule "relative-module"] + path = relative-module + url = https://git.example.org/submodule.git +`), + }, { + Operation: "create", + TreePath: "relative-module", + FromTreePath: "", + ContentReader: nil, + SHA: "95601d16476a", + Options: files_service.RepoFileOptionMode(git.EntryModeCommit), + }, + }) + defer f() + + // Check that the submodule entry exist and the link is correct. + req := NewRequest(t, "GET", "/"+repo.FullName()) + resp := MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + + _, ok := htmlDoc.Find(`tr[data-entryname="relative-module"] td.name a`).Attr("href") + assert.False(t, ok, "should not find a link to 'relative-module' in file list") + + // Check that a link to the submodule returns a redirect and that the redirect link is correct. + req = NewRequest(t, "GET", "/"+repo.FullName()+"/src/branch/"+repo.DefaultBranch+"/relative-module") + resp = MakeRequest(t, req, http.StatusSeeOther) + + assert.Equal(t, "/"+repo.FullName()+"/src/branch/"+repo.DefaultBranch+"/", resp.Header().Get("Location")) + + // Check that a warning is present + req = NewRequest(t, "GET", "/"+repo.FullName()+"/src/branch/"+repo.DefaultBranch+"/.gitmodules") + resp = MakeRequest(t, req, http.StatusOK) + + htmlDoc = NewHTMLParser(t, resp.Body) + + warn, err := htmlDoc.Find(`.non-diff-file-content .warning`).Html() + require.NoError(t, err) + assert.NotEmpty(t, warn) + }) }) }