diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index f27ad62..1000300 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -758,6 +758,7 @@ func UploadFilePost(ctx *context.Context) { TreePath: form.TreePath, Message: message, Files: form.Files, + FullPaths: form.FullPaths, Signoff: form.Signoff, Author: gitIdentity, Committer: gitIdentity, diff --git a/routers/web/web.go b/routers/web/web.go index db0015f..93ca5ba 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -51,6 +51,7 @@ import ( _ "code.gitea.io/gitea/modules/session" // to registers all internal adapters + "code.forgejo.org/go-chi/binding" "code.forgejo.org/go-chi/captcha" chi_middleware "github.com/go-chi/chi/v5/middleware" "github.com/go-chi/cors" @@ -356,20 +357,6 @@ func registerRoutes(m *web.Route) { } } - annexEnabled := func(ctx *context.Context) { - if !setting.Annex.Enabled { - ctx.Error(http.StatusNotFound) - return - } - } - - annexP2PHTTPEnabled := func(ctx *context.Context) { - if setting.Annex.DisableP2PHTTP { - ctx.Error(http.StatusNotFound) - return - } - } - federationEnabled := func(ctx *context.Context) { if !setting.Federation.Enabled { ctx.Error(http.StatusNotFound) @@ -969,9 +956,6 @@ func registerRoutes(m *web.Route) { // ***** END: Organization ***** // ***** START: Repository ***** - m.Group("", func() { - m.Methods("GET,POST", "/git-annex-p2phttp/git-annex/{uuid}/*", repo.AnnexP2PHTTP) - }, ignSignInAndCsrf, annexEnabled, annexP2PHTTPEnabled) m.Group("/repo", func() { m.Get("/create", repo.Create) m.Post("/create", web.Bind(forms.CreateRepoForm{}), repo.CreatePost) @@ -1272,7 +1256,7 @@ func registerRoutes(m *web.Route) { Post(web.Bind(forms.DeleteRepoFileForm{}), repo.DeleteFilePost) m.Combo("/_upload/*", repo.MustBeAbleToUpload). Get(repo.UploadFile). - Post(web.Bind(forms.UploadRepoFileForm{}), repo.UploadFilePost) + Post(BindUpload(forms.UploadRepoFileForm{}), repo.UploadFilePost) m.Combo("/_diffpatch/*").Get(repo.NewDiffPatch). Post(web.Bind(forms.EditRepoFileForm{}), repo.NewDiffPatchPost) m.Combo("/_cherrypick/{sha:([a-f0-9]{4,64})}/*").Get(repo.CherryPick). @@ -1652,12 +1636,6 @@ func registerRoutes(m *web.Route) { }) }, ignSignInAndCsrf, lfsServerEnabled) - m.Group("", func() { - // for git-annex - m.Methods("GET,OPTIONS", "/config", repo.GetConfig) // needed by clients reading annex.uuid during `git annex initremote` - m.Methods("GET,OPTIONS", "/annex/objects/{hash1}/{hash2}/{keyDir}/{key}", repo.GetAnnexObject) - }, ignSignInAndCsrf, annexEnabled, context.UserAssignmentWeb()) - gitHTTPRouters(m) }) }) @@ -1694,3 +1672,20 @@ func registerRoutes(m *web.Route) { ctx.NotFound("", nil) }) } + +func BindUpload(f forms.UploadRepoFileForm) http.HandlerFunc { + return func(resp http.ResponseWriter, req *http.Request) { + theObj := new(forms.UploadRepoFileForm) // create a new form obj for every request but not use obj directly + data := middleware.GetContextData(req.Context()) + binding.Bind(req, theObj) + files := theObj.Files + var fullpaths []string + for _, fileID := range files { + fullPath := req.Form.Get("files_fullpath[" + fileID + "]") + fullpaths = append(fullpaths, fullPath) + } + theObj.FullPaths = fullpaths + data.GetData()["__form"] = theObj + middleware.AssignForm(theObj, data) + } +} diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 1ce9b29..0e97807 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -671,6 +671,7 @@ type UploadRepoFileForm struct { CommitChoice string `binding:"Required;MaxSize(50)"` NewBranchName string `binding:"GitRefName;MaxSize(100)"` Files []string + FullPaths []string CommitMailID int64 `binding:"Required"` Signoff bool } @@ -740,17 +741,6 @@ type SaveTopicForm struct { Topics []string `binding:"topics;Required;"` } -// DeadlineForm hold the validation rules for deadlines -type DeadlineForm struct { - DateString string `form:"date" binding:"Required;Size(10)"` -} - -// Validate validates the fields -func (f *DeadlineForm) Validate(req *http.Request, errs binding.Errors) binding.Errors { - ctx := context.GetValidateContext(req) - return middleware.Validate(errs, ctx.Data, f, ctx.Locale) -} - type CommitNotesForm struct { Notes string } diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index 21cd5a8..0d6be75 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -6,16 +6,15 @@ package files import ( "context" "fmt" - "io" + "html" "os" "path" - "path/filepath" + "regexp" "strings" git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/annex" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/setting" @@ -31,6 +30,7 @@ type UploadRepoFileOptions struct { Author *IdentityOptions Committer *IdentityOptions Files []string // In UUID format. + FullPaths []string Signoff bool } @@ -59,16 +59,24 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return nil } - uploads, err := repo_model.GetUploadsByUUIDs(ctx, opts.Files) - if err != nil { - return fmt.Errorf("GetUploadsByUUIDs [uuids: %v]: %w", opts.Files, err) + if len(opts.Files) != len(opts.FullPaths) { + return nil } - names := make([]string, len(uploads)) - infos := make([]uploadInfo, len(uploads)) - for i, upload := range uploads { + uploads := make([]*repo_model.Upload, len(opts.Files)) + names := make([]string, len(opts.Files)) + infos := make([]uploadInfo, len(opts.Files)) + + for i := 0; i < len(opts.Files); i++ { + var err error + uploads[i], err = repo_model.GetUploadByUUID(ctx, opts.Files[i]) + if err != nil { + return fmt.Errorf("GetUploadByUUID [uuids: %v]: %w", opts.Files[i], err) + } + uploads[i].Name = fileNameSanitize(html.UnescapeString(opts.FullPaths[i])) + // Check file is not lfs locked, will return nil if lock setting not enabled - filepath := path.Join(opts.TreePath, upload.Name) + filepath := path.Join(opts.TreePath, uploads[i].Name) lfsLock, err := git_model.GetTreePathLock(ctx, repo.ID, filepath) if err != nil { return err @@ -81,8 +89,8 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return git_model.ErrLFSFileLocked{RepoID: repo.ID, Path: filepath, UserName: u.Name} } - names[i] = upload.Name - infos[i] = uploadInfo{upload: upload} + names[i] = uploads[i].Name + infos[i] = uploadInfo{upload: uploads[i]} } t, err := NewTemporaryUploadRepository(ctx, repo) @@ -92,7 +100,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use defer t.Close() hasOldBranch := true - if err = t.Clone(opts.OldBranch, false); err != nil { + if err = t.Clone(opts.OldBranch, true); err != nil { if !git.IsErrBranchNotExist(err) || !repo.IsEmpty { return err } @@ -108,30 +116,10 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use } } - r, err := git.OpenRepository(ctx, repo.RepoPath()) - if err != nil { + // Copy uploaded files into repository. + if err := copyUploadedLFSFilesIntoRepository(infos, t, opts.TreePath); err != nil { return err } - if annex.IsAnnexRepo(r) { - // Initialize annex privately in temporary clone - if err := t.InitPrivateAnnex(); err != nil { - return err - } - // Copy uploaded files into git-annex repository - if err := copyUploadedFilesIntoAnnexRepository(infos, t, opts.TreePath); err != nil { - return err - } - // Move all annexed content in the temporary repository, i.e. everything we have just added, to the origin - author, committer := GetAuthorAndCommitterUsers(opts.Author, opts.Committer, doer) - if err := moveAnnexedFilesToOrigin(t, author, committer); err != nil { - return err - } - } else { - // Copy uploaded files into repository. - if err := copyUploadedLFSFilesIntoRepository(infos, t, opts.TreePath); err != nil { - return err - } - } // Now write the tree treeHash, err := t.WriteTree() @@ -178,6 +166,19 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return repo_model.DeleteUploads(ctx, uploads...) } +// From forgejo/services/repository/generate.go (but now allows /) +var fileNameSanitizeRegexp = regexp.MustCompile(`(?i)\.\.|[<>:\"\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`) + +// Sanitize user input to valid OS filenames +// +// Based on https://github.com/sindresorhus/filename-reserved-regex +// Adds ".." to prevent directory traversal +func fileNameSanitize(s string) string { + // Added this because I am not sure what Windows will deliver us \ or / but we need /. + s = strings.ReplaceAll(s, "\\", "/") + return strings.TrimSpace(fileNameSanitizeRegexp.ReplaceAllString(s, "_")) +} + func copyUploadedLFSFilesIntoRepository(infos []uploadInfo, t *TemporaryUploadRepository, treePath string) error { var storeInLFSFunc func(string) (bool, error) @@ -269,57 +270,3 @@ func uploadToLFSContentStore(info uploadInfo, contentStore *lfs.ContentStore) er } return nil } - -func copyUploadedFilesIntoAnnexRepository(infos []uploadInfo, t *TemporaryUploadRepository, treePath string) error { - for i := range len(infos) { - if err := copyUploadedFileIntoAnnexRepository(&infos[i], t, treePath); err != nil { - return err - } - } - return nil -} - -func copyUploadedFileIntoAnnexRepository(info *uploadInfo, t *TemporaryUploadRepository, treePath string) error { - pathInRepo := path.Join(t.basePath, treePath, info.upload.Name) - if err := os.MkdirAll(filepath.Dir(pathInRepo), 0o700); err != nil { - return err - } - if err := os.Rename(info.upload.LocalPath(), pathInRepo); err != nil { - // Rename didn't work, try copy and remove - inputFile, err := os.Open(info.upload.LocalPath()) - if err != nil { - return fmt.Errorf("could not open source file: %v", err) - } - defer inputFile.Close() - outputFile, err := os.Create(pathInRepo) - if err != nil { - return fmt.Errorf("could not open dest file: %v", err) - } - defer outputFile.Close() - _, err = io.Copy(outputFile, inputFile) - if err != nil { - return fmt.Errorf("could not copy to dest from source: %v", err) - } - inputFile.Close() - err = os.Remove(info.upload.LocalPath()) - if err != nil { - return fmt.Errorf("could not remove source file: %v", err) - } - } - return t.AddAnnex(pathInRepo) -} - -func moveAnnexedFilesToOrigin(t *TemporaryUploadRepository, author, committer *user_model.User) error { - authorSig := author.NewGitSig() - committerSig := committer.NewGitSig() - env := append(os.Environ(), - "GIT_AUTHOR_NAME="+authorSig.Name, - "GIT_AUTHOR_EMAIL="+authorSig.Email, - "GIT_COMMITTER_NAME="+committerSig.Name, - "GIT_COMMITTER_EMAIL="+committerSig.Email, - ) - if _, _, err := git.NewCommand(t.ctx, "annex", "move", "--to", "origin").RunStdString(&git.RunOpts{Dir: t.basePath, Env: env}); err != nil { - return err - } - return nil -} diff --git a/templates/repo/editor/commit_form.tmpl b/templates/repo/editor/commit_form.tmpl index c42eed6..637185a 100644 --- a/templates/repo/editor/commit_form.tmpl +++ b/templates/repo/editor/commit_form.tmpl @@ -75,8 +75,6 @@ - + {{ctx.Locale.Tr "repo.editor.cancel"}} diff --git a/tests/e2e/declare_repos_test.go b/tests/e2e/declare_repos_test.go index c55a42a..a93c2b8 100644 --- a/tests/e2e/declare_repos_test.go +++ b/tests/e2e/declare_repos_test.go @@ -48,6 +48,10 @@ func DeclareGitRepos(t *testing.T) func() { CommitMsg: "Another commit which mentions @user1 in the title\nand @user2 in the text", }, }), + newRepo(t, 2, "file-uploads", []FileChanges{{ + Filename: "README.md", + Versions: []string{"# File upload test\nUse this repo to test various file upload features in new branches."}, + }}), // add your repo declarations here } @@ -68,7 +72,7 @@ func newRepo(t *testing.T, userID int64, repoName string, fileChanges []FileChan for _, file := range fileChanges { for i, version := range file.Versions { operation := "update" - if i == 0 { + if i == 0 && file.Filename != "README.md" { operation = "create" } diff --git a/tests/e2e/repo-files.test.e2e.ts b/tests/e2e/repo-files.test.e2e.ts new file mode 100644 index 0000000..ab8813e --- /dev/null +++ b/tests/e2e/repo-files.test.e2e.ts @@ -0,0 +1,46 @@ +// @watch start +// templates/repo/editor/** +// web_src/js/features/common-global.js +// routers/web/web.go +// services/repository/files/upload.go +// @watch end + +/// + +import { expect } from '@playwright/test'; +import { test, dynamic_id, save_visual } from './utils_e2e.ts'; + +test.use({ user: 'user2' }); + +test('drap and drop upload', async ({ page }, workerInfo) => { + const response = await page.goto(`/user2/file-uploads/_upload/main/`); + expect(response?.status()).toBe(200); // Status OK + + const testID = dynamic_id(); + const dropzone = page.getByRole('button', { name: 'Drop files or click here to upload.' }); + + // create the virtual files + const dataTransfer = await page.evaluateHandle(() => { + const dt = new DataTransfer(); + + // add items in different folders + dt.items.add(new File(["Filecontent (dir1/file1.txt)"], 'dir1/file1.txt', { type: 'text/plain' })); + dt.items.add(new File(["Another file's content (double/nested/file.txt)"], 'double/nested/file.txt', { type: 'text/plain' })); + dt.items.add(new File(["Root file (root_file.txt)"], 'root_file.txt', { type: 'text/plain' })); + + return dt; + }); + // and drop them to the upload area + await dropzone.dispatchEvent('drop', { dataTransfer }); + + await page.getByText('new branch').click(); + await save_visual(page); + await page.getByRole('textbox', { name: 'Name the new branch for this' }).fill(testID); + await page.getByRole('button', { name: 'Propose file change' }).click(); + + // check that nested file structure is preserved + await expect(page.getByRole('link', { name: 'dir1/file1.txt' })).toBeVisible(); + await expect(page.getByRole('link', { name: 'double/nested/file.txt' })).toBeVisible(); + await expect(page.locator('#diff-file-boxes').getByRole('link', { name: 'root_file.txt', exact: true })).toBeVisible(); + await save_visual(page); +}); diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index 5a304d9..a4f8f70 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -229,7 +229,8 @@ export function initDropzone(el) { this.on('success', (file, data) => { file.uuid = data.uuid; const $input = $(``).val(data.uuid); - $dropzone.find('.files').append($input); + const $inputPath = $(``); + $dropzone.find('.files').append($input).append($inputPath); // Create a "Copy Link" element, to conveniently copy the image // or file link as Markdown to the clipboard const copyLinkElement = document.createElement('div'); @@ -250,7 +251,12 @@ export function initDropzone(el) { file.previewTemplate.append(copyLinkElement); }); this.on('removedfile', (file) => { + // Remove the hidden input for the file $(`#${file.uuid}`).remove(); + + // Remove the hidden input for files_fullpath + $(`input[name="files_fullpath[${file.uuid}]"]`).remove(); + if ($dropzone.data('remove-url')) { POST($dropzone.data('remove-url'), { data: new URLSearchParams({file: file.uuid}), @@ -357,7 +363,7 @@ export function initGlobalLinkActions() { }); } -function initGlobalShowModal() { +export function initGlobalShowModal() { // A ".show-modal" button will show a modal dialog defined by its "data-modal" attribute. // Each "data-modal-{target}" attribute will be filled to target element's value or text-content. // * First, try to query '#target'