Cleaning up the commit history. Last relevent commits were: Cleaned the comments and used test.describe to nest the tests ; fix the 0ko conflict

models/repo/upload.go

@@ -104,17 +104,6 @@ func GetUploadByUUID(ctx context.Context, uuid string) (*Upload, error) {
 	return upload, nil
 }
 
-// GetUploadsByUUIDs returns multiple uploads by UUIDS
-func GetUploadsByUUIDs(ctx context.Context, uuids []string) ([]*Upload, error) {
-	if len(uuids) == 0 {
-		return []*Upload{}, nil
-	}
-
-	// Silently drop invalid uuids.
-	uploads := make([]*Upload, 0, len(uuids))
-	return uploads, db.GetEngine(ctx).In("uuid", uuids).Find(&uploads)
-}
-
 // DeleteUploads deletes multiple uploads
 func DeleteUploads(ctx context.Context, uploads ...*Upload) (err error) {
 	if len(uploads) == 0 {

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,

routers/web/web.go

@@ -51,6 +51,7 @@ import (
 
 	_ "forgejo.org/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"
@@ -1259,7 +1260,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).
@@ -1678,3 +1679,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)
+	}
+}

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
 }

services/repository/files/pathutils.go

@@ -0,0 +1,55 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+package files
+
+import (
+	"fmt"
+	"path"
+	"regexp"
+	"strings"
+)
+
+var fileNameComponentSanitizeRegexp = regexp.MustCompile(`(?i)[<>:\"/\\|?*\x{0000}-\x{001F}]|^(con|prn|aux|nul|com\d|lpt\d)$`)
+
+// SanitizePath cleans and validates a file path
+func SanitizePath(inputPath string) (string, error) {
+	// Normalize path separators
+	s := strings.ReplaceAll(inputPath, "\\", "/")
+
+	// We don't want a / or \\ as the beginning of a path
+	if strings.HasPrefix(inputPath, "/") {
+		return "", fmt.Errorf("path starts with / : %s", inputPath)
+	}
+
+	// Clean the path
+	s = path.Clean(s)
+	// Split the path components
+	pathComponents := strings.Split(s, "/")
+	// Sanitize each path component
+	var sanitizedComponents []string
+	for _, component := range pathComponents {
+		// There is no reason why there should be a path segment with ..
+		if component == ".." {
+			return "", fmt.Errorf("path contains directory traversal: %s", s)
+		}
+		// There is no reason why there should be a path segment with .
+		if component == "." {
+			return "", fmt.Errorf("path contains directory traversal: %s", s)
+		}
+
+		// Trim whitespace and apply regex sanitization
+		sanitizedComponent := strings.TrimSpace(fileNameComponentSanitizeRegexp.ReplaceAllString(component, "_"))
+
+		// Skip empty components after sanitization
+		if sanitizedComponent != "" {
+			sanitizedComponents = append(sanitizedComponents, sanitizedComponent)
+		}
+	}
+	// Check if we have any components left after sanitization
+	if len(sanitizedComponents) == 0 {
+		return "", fmt.Errorf("path became empty after sanitization")
+	}
+	// Reconstruct the path
+	reconstructedPath := path.Join(sanitizedComponents...)
+	return reconstructedPath, nil
+}

services/repository/files/upload.go

@@ -28,6 +28,7 @@ type UploadRepoFileOptions struct {
 	Author       *IdentityOptions
 	Committer    *IdentityOptions
 	Files        []string // In UUID format.
+	FullPaths    []string
 	Signoff      bool
 }
 
@@ -56,16 +57,25 @@ 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 fmt.Errorf("the length of opts.Files and opts.FullPaths is not the same")
 	}
 
-	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))
+	var err error
+	for i := 0; i < len(opts.Files); i++ {
+		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, err = SanitizePath(opts.FullPaths[i])
+		if err != nil {
+			return fmt.Errorf("SanitizePath [path: %v]: %w", opts.FullPaths[i], err)
+		}
 		// 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
@@ -78,8 +88,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)

templates/repo/editor/commit_form.tmpl

@@ -75,8 +75,6 @@
 			</select>
 		</div>
 	</div>
-	<button id="commit-button" type="submit" class="ui primary button">
-		{{if eq .commit_choice "commit-to-new-branch"}}{{ctx.Locale.Tr "repo.editor.propose_file_change"}}{{else}}{{ctx.Locale.Tr "repo.editor.commit_changes"}}{{end}}
-	</button>
+	<button id="commit-button" type="submit" class="ui primary button">{{if eq .commit_choice "commit-to-new-branch"}}{{ctx.Locale.Tr "repo.editor.propose_file_change"}}{{else}}{{ctx.Locale.Tr "repo.editor.commit_changes"}}{{end}}</button>
 	<a class="ui button red" href="{{$.BranchLink}}/{{PathEscapeSegments .TreePath}}">{{ctx.Locale.Tr "repo.editor.cancel"}}</a>
 </div>

tests/e2e/declare_repos_test.go

@@ -53,6 +53,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."},
+		}}),
 		newRepo(t, 2, "unicode-escaping", []FileChanges{{
 			Filename: "a-file",
 			Versions: []string{"{a}{а}"},
@@ -77,7 +81,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"
 			}
 

tests/e2e/repo-files.test.e2e.ts

@@ -0,0 +1,139 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+// @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.describe('Drag and Drop upload', () => {
+  test('normal and special characters', async ({page}) => {
+    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 dataTransferA = 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'}));
+      dt.items.add(new File(['Umlaut test'], 'special/äüöÄÜÖß.txt', {type: 'text/plain'}));
+      dt.items.add(new File(['Unicode test'], 'special/Ʉ₦ł₵ØĐɆ.txt', {type: 'text/plain'}));
+      return dt;
+    });
+    // and drop them to the upload area
+    await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA});
+
+    await page.getByText('new branch').click();
+
+    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.getByRole('link', {name: 'special/äüöÄÜÖß.txt'})).toBeVisible();
+    await expect(page.getByRole('link', {name: 'special/Ʉ₦ł₵ØĐɆ.txt'})).toBeVisible();
+    // Since this is a file in root, there two links with the same label
+    // we take the on in #diff-file-tree
+    await expect(page.locator('#diff-file-boxes').getByRole('link', {name: 'root_file.txt'})).toBeVisible();
+  });
+
+  test('strange paths and spaces', async ({page}) => {
+    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 dataTransferA = await page.evaluateHandle(() => {
+      const dt = new DataTransfer();
+      // add items in different folders
+      dt.items.add(new File(['1'], '..dots.txt', {type: 'text/plain'}));
+      dt.items.add(new File(['2'], '.dots.vanish.txt', {type: 'text/plain'}));
+      dt.items.add(new File(['3'], 'special/S P  A   C   E    !.txt', {type: 'text/plain'}));
+      return dt;
+    });
+    // and drop them to the upload area
+    await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA});
+
+    await page.getByText('new branch').click();
+
+    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
+    // Since this is a file in root, there two links with the same label
+    // we take the on in #diff-file-tree
+    await expect(page.locator('#diff-file-boxes').getByRole('link', {name: '.dots.vanish.txt'})).toBeVisible();
+    await expect(page.getByRole('link', {name: 'special/S P  A   C   E    !.txt'})).toBeVisible();
+    // Since this is a file in root, there two links with the same label
+    // we take the on in #diff-file-tree
+    await expect(page.locator('#diff-file-boxes').getByRole('link', {name: '..dots.txt'})).toBeVisible();
+  });
+
+  test('broken path slash in front', async ({page}) => {
+    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 dataTransferA = await page.evaluateHandle(() => {
+      const dt = new DataTransfer();
+      // add items in different folders
+      dt.items.add(new File(['1'], '/special/badfirstslash.txt', {type: 'text/plain'}));
+      return dt;
+    });
+    // and drop them to the upload area
+    await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA});
+
+    await page.getByText('new branch').click();
+
+    await page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID);
+    await page.getByRole('button', {name: 'Propose file change'}).click();
+
+    await expect(page.getByText('Failed to upload files to')).toBeVisible();
+
+    await save_visual(page);
+  });
+
+  test('broken path with traversal', async ({page}) => {
+    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 dataTransferA = await page.evaluateHandle(() => {
+      const dt = new DataTransfer();
+      // add items in different folders
+      dt.items.add(new File(['1'], '../baddots.txt', {type: 'text/plain'}));
+      return dt;
+    });
+    // and drop them to the upload area
+    await dropzone.dispatchEvent('drop', {dataTransfer: dataTransferA});
+
+    await page.getByText('new branch').click();
+
+    await page.getByRole('textbox', {name: 'Name the new branch for this'}).fill(testID);
+    await page.getByRole('button', {name: 'Propose file change'}).click();
+
+    await expect(page.getByText('Failed to upload files to')).toBeVisible();
+
+    await save_visual(page);
+  });
+});

tests/integration/editor_test.go

@@ -407,22 +407,26 @@ func TestCommitMail(t *testing.T) {
 
 			file1UUID := uploadFile(t, "upload_file_1", "Uploaded a file!")
 			file2UUID := uploadFile(t, "upload_file_2", "Uploaded another file!")
+			file1UUIDFullpathKey := fmt.Sprintf("files_fullpath[%s]", file1UUID)
+			file2UUIDFullpathKey := fmt.Sprintf("files_fullpath[%s]", file2UUID)
 
 			assertCase(t, caseOpts{
 				fileName:       "upload_file_1",
 				link:           "user2/repo1/_upload/master",
 				skipLastCommit: true,
 				base: map[string]string{
-					"commit_choice": "direct",
-					"files":         file1UUID,
+					"commit_choice":      "direct",
+					"files":              file1UUID,
+					file1UUIDFullpathKey: "upload_file_1",
 				},
 			}, caseOpts{
 				fileName:       "upload_file_2",
 				link:           "user2/repo1/_upload/master",
 				skipLastCommit: true,
 				base: map[string]string{
-					"commit_choice": "direct",
-					"files":         file2UUID,
+					"commit_choice":      "direct",
+					"files":              file2UUID,
+					file2UUIDFullpathKey: "upload_file_2",
 				},
 			},
 			)

tests/integration/empty_repo_test.go

@@ -6,6 +6,7 @@ package integration
 import (
 	"bytes"
 	"encoding/base64"
+	"fmt"
 	"io"
 	"mime/multipart"
 	"net/http"
@@ -88,11 +89,12 @@ func TestEmptyRepoUploadFile(t *testing.T) {
 	resp = session.MakeRequest(t, req, http.StatusOK)
 	respMap := map[string]string{}
 	require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &respMap))
-
+	filesFullpathKey := fmt.Sprintf("files_fullpath[%s]", respMap["uuid"])
 	req = NewRequestWithValues(t, "POST", "/user30/empty/_upload/"+setting.Repository.DefaultBranch, map[string]string{
 		"_csrf":          GetCSRF(t, session, "/user/settings"),
 		"commit_choice":  "direct",
 		"files":          respMap["uuid"],
+		filesFullpathKey: "uploaded-file.txt",
 		"tree_path":      "",
 		"commit_mail_id": "-1",
 	})

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 = $(`<input id="${data.uuid}" name="files" type="hidden">`).val(data.uuid);
-        $dropzone.find('.files').append($input);
+        const $inputPath = $(`<input type="hidden" name="files_fullpath[${data.uuid}]" value="${htmlEscape(file.fullPath || file.name)}">`);
+        $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}),