fix: refuse .Trash as a name at the top of Files, where it would become the trash

This commit is contained in:
Linus Rath
2026-10-29 03:10:35 +01:00
parent e1e6af3cca
commit d908f95fd7
6 changed files with 63 additions and 8 deletions
+1
View File
@@ -480,6 +480,7 @@
"move_to": "Move to…",
"name_control_chars": "Names cannot contain control characters.",
"name_too_long": "This name is too long for the server.",
"name_trash": "\".Trash\" is the name of the trash folder at the top of your files. Choose another name.",
"open_error": "Could not open the file",
"open_with": "Open with…",
"preview_too_large": "This file is too large to preview.",
+12
View File
@@ -277,6 +277,18 @@ describe('restoreFromTrash', () => {
expect(byId('a2')).toMatchObject({ parentId: 'docs', name: 'a.txt' });
});
it('gives a folder named .Trash that goes back to the root another name', async () => {
store.push({ id: 'x', parentId: 'docs', name: '.Trash', blobId: null }, { id: 'xin', parentId: 'x', name: 'in.txt', blobId: 'bi' });
let nodes = listed();
await moveToTrash(nodes, nodes.filter((n) => n.id === 'x'), 1730112000007);
// Docs goes too: the folder comes back to the root.
await moveToTrash(listed(), listed().filter((n) => n.id === 'docs'), 1730112000008);
nodes = listed();
await restoreFromTrash(trashedItems(nodes).filter((i) => i.node.id === 'x'));
expect(byId('x')).toMatchObject({ parentId: null, name: '.Trash (1)' });
expect(store.filter((n) => n.parentId === null && n.name === '.Trash')).toHaveLength(1);
});
it('leaves an item alone that left the trash since the rows were listed', async () => {
let nodes = listed();
await moveToTrash(nodes, nodes.filter((n) => n.id === 'a'), 1730112000007);
+2
View File
@@ -202,6 +202,8 @@ export async function restoreFromTrash(items: TrashedItem[], drive?: TrashDrive)
names = new Set(nodes
.filter((n) => (n.parentId ?? null) === parentId && !n.isShared && !hidden.has(n.id))
.map((n) => n.name));
// Something named ".Trash" doesn't go back to the root under that name.
if (parentId === null) names.add(TRASH_FOLDER_NAME);
taken.set(parentId, names);
}
return names;
+14 -2
View File
@@ -1,8 +1,8 @@
import { describe, it, expect } from 'vitest';
import type { FileNode } from '../../api/types';
import {
findTrashFolder, parseTrashBatchName, restoreTarget, trashBatchName, trashOriginPath, trashSubtreeIds,
trashedItems, withoutTrash,
findTrashFolder, parseTrashBatchName, restoreTarget, takesTrashName, trashBatchName, trashOriginPath,
trashSubtreeIds, trashedItems, withoutTrash,
} from '../files-trash';
// The webmail's trash convention (its lib/files-trash.ts), read the same way here.
@@ -76,6 +76,18 @@ describe('withoutTrash', () => {
});
});
describe('takesTrashName', () => {
it('refuses .Trash at the root of the own drive only', () => {
expect(takesTrashName('.Trash', null)).toBe(true);
expect(takesTrashName('.Trash', null, 'docs')).toBe(true);
expect(takesTrashName('.Trash', 'docs')).toBe(false);
expect(takesTrashName('.trash', null)).toBe(false);
expect(takesTrashName('.Trash (1)', null)).toBe(false);
// A node of a shared drive stays there.
expect(takesTrashName('.Trash', null, 'acc-2:x')).toBe(false);
});
});
describe('restoreTarget', () => {
it('puts an item back into its folder while that is still there', () => {
expect(restoreTarget({ fromParentId: 'tax' }, drive)).toBe('tax');
+11
View File
@@ -28,6 +28,17 @@ export function findTrashFolder(nodes: FileNode[]): FileNode | undefined {
&& isFolder(n));
}
/**
* Whether naming a node `name` in `parentId` (create, rename, move or upload)
* would put it in the trash's place, `.Trash` at the root of the user's own
* drive: a folder there becomes the trash, and a file there keeps the trash
* from being made. `nodeId` is the node renamed or moved, if any; one of a
* shared drive stays in that drive.
*/
export function takesTrashName(name: string, parentId: string | null, nodeId?: string): boolean {
return name === TRASH_FOLDER_NAME && parentId === null && !(nodeId?.includes(':'));
}
/** The name of the folder one delete's items go into. */
export function trashBatchName(parentId: string | null, deletedAt: number = Date.now()): string {
return `${deletedAt}~${parentId ?? ''}`;
+23 -6
View File
@@ -55,7 +55,9 @@ import {
TrashUnavailableError, emptyTrash, moveToTrash, purgeFromTrash, restoreFromTrash, undoMoveToTrash,
type TrashMoveResult,
} from '../api/files-trash';
import { trashOriginPath, trashedItems, withoutTrash, type TrashedItem } from '../lib/files-trash';
import {
TRASH_FOLDER_NAME, takesTrashName, trashOriginPath, trashedItems, withoutTrash, type TrashedItem,
} from '../lib/files-trash';
import { toast } from '../stores/toast-store';
import { jmapClient } from '../api/jmap-client';
import { downloadAttachment, shareAttachment } from '../lib/email-export';
@@ -178,6 +180,11 @@ function fileNameProblemText(problem: FileNameProblem, t: Translate): string {
}
}
// Why ".Trash" can't go to the root (see takesTrashName).
function trashNameText(t: Translate): string {
return t('files.name_trash', '".Trash" is the name of the trash folder at the top of your files. Choose another name.');
}
// The files an upload could not store, the first three by name, and what the
// server said about the first one.
function uploadFailureDetail(failed: Array<{ name: string; error: unknown }>, t: Translate): string {
@@ -604,12 +611,16 @@ export default function FilesScreen() {
// Explains a name the server would refuse before anything is sent: the
// naming rules it publishes (Stalwart 0.16.6+) with 1.0's stricter ones on
// top, or at least the "/" every server refuses. True when the name can go.
const checkName = useCallback((name: string): boolean => {
// top, or at least the "/" every server refuses. `.Trash` is refused at the
// root, where it would become the trash (lib/files-trash). True when the
// name can go.
const checkName = useCallback((name: string, parentId: string | null, nodeId?: string): boolean => {
const problem = fileNameProblem(name, getFileNameRules());
const message = problem
? fileNameProblemText(problem, t)
: name.includes('/') ? t('files.invalid_name_slash', 'Names cannot contain "/".') : null;
: name.includes('/') ? t('files.invalid_name_slash', 'Names cannot contain "/".')
: takesTrashName(name, parentId, nodeId) ? trashNameText(t)
: null;
if (message) Alert.alert(t('files.invalid_name', 'Invalid name'), message);
return !message;
}, [t]);
@@ -617,7 +628,7 @@ export default function FilesScreen() {
const submitNewFolder = useCallback(async () => {
const name = newFolderName.trim();
if (!name) return;
if (!checkName(name)) return;
if (!checkName(name, currentParentId)) return;
setNewFolderOpen(false);
setNewFolderName('');
try {
@@ -635,7 +646,7 @@ export default function FilesScreen() {
setRenameTarget(null);
return;
}
if (!checkName(name)) return;
if (!checkName(name, renameTarget.parentId ?? null, renameTarget.id)) return;
const target = renameTarget;
setRenameTarget(null);
setRenameValue('');
@@ -666,6 +677,10 @@ export default function FilesScreen() {
const performMove = useCallback(async (row: FileRow, destinationId: string | null) => {
setMoveTarget(null);
if ((row.parentId ?? null) === destinationId) return;
if (takesTrashName(row.name, destinationId, row.id)) {
Alert.alert(t('files.move_error', 'Failed to move'), trashNameText(t));
return;
}
setBusyId(row.id);
try {
await moveFileNode(row.id, destinationId);
@@ -730,6 +745,8 @@ export default function FilesScreen() {
uploadAbortRef.current = controller;
const parentId = currentParentId;
const siblings = allNodes.filter((n) => (n.parentId ?? null) === parentId).map((n) => n.name);
// A file picked as ".Trash" gets a number at the root, where the name is the trash's.
if (takesTrashName(TRASH_FOLDER_NAME, parentId)) siblings.push(TRASH_FOLDER_NAME);
// A refused file no longer stops the rest; the ones that failed are
// reported once the batch is done.
const { failed } = await uploadFileBatch(accepted, parentId, siblings, {