Drag and Drop UI: Modifies the SanitizePath approach for path handed over from dropzone.js. Add tests to check the algorithm.

This commit is contained in:
David Rotermund 2025-04-04 01:43:23 +02:00
parent 8f5c3b2227
commit 4383c403c9
3 changed files with 105 additions and 34 deletions

View file

@ -0,0 +1,39 @@
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, "\\", "/")
// 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 {
// 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
}

View file

@ -6,10 +6,8 @@ package files
import (
"context"
"fmt"
"html"
"os"
"path"
"regexp"
"strings"
git_model "code.gitea.io/gitea/models/git"
@ -72,8 +70,10 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
if err != nil {
return fmt.Errorf("GetUploadByUUID [uuids: %v]: %w", opts.Files[i], err)
}
uploads[i].Name = fileNameSanitize(html.UnescapeString(opts.FullPaths[i]))
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, uploads[i].Name)
lfsLock, err := git_model.GetTreePathLock(ctx, repo.ID, filepath)
@ -165,19 +165,6 @@ 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)

View file

@ -5,40 +5,85 @@
// services/repository/files/upload.go
// @watch end
import {expect} from '@playwright/test';
import {test, dynamic_id, save_visual} from './utils_e2e.ts';
// https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md
// lies. Don't use
// make TAGS="sqlite sqlite_unlock_notify" backend
// use
// make TAGS="sqlite sqlite_unlock_notify" build
// otherwise the test fails.
test.use({user: 'user2'});
test('drap and drop upload', async ({page}) => {
import { expect } from '@playwright/test';
import { test, dynamic_id, save_visual } from './utils_e2e.ts';
test.use({ user: 'user2' });
test('drag and drop upload a', 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.'});
const dropzone = page.getByRole('button', { name: 'Drop files or click here to upload.' });
// create the virtual files
const dataTransfer = await page.evaluateHandle(() => {
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(['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});
await dropzone.dispatchEvent('drop', { dataTransfer: dataTransferA });
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();
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 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();
await expect(page.locator('#diff-file-boxes').getByRole('link', { name: 'root_file.txt', exact: true })).toBeVisible();
await save_visual(page);
});
test('drag and drop upload b', 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'], 'special/../../dots_vanish.txt', { type: 'text/plain' }));
dt.items.add(new File(['3'], '\\windows\\windows_slash.txt', { type: 'text/plain' }));
dt.items.add(new File(['4'], '/special/badfirstslash.txt', { type: 'text/plain' }));
dt.items.add(new File(['5'], '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 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: 'windows/windows_slash.txt' })).toBeVisible();
await expect(page.getByRole('link', { name: '_/dots_vanish.txt' })).toBeVisible();
await expect(page.getByRole('link', { name: 'special/badfirstslash.txt' })).toBeVisible();
await expect(page.getByRole('link', { name: 'special/S P A C E !.txt' })).toBeVisible();
await expect(page.locator('#diff-file-boxes').getByRole('link', { name: '_/_/dots.txt', exact: true })).toBeVisible();
await save_visual(page);
});