From 6b49a0bea724104a3e650204f07252988e8131e4 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Wed, 7 Oct 2026 08:25:13 +0200 Subject: [PATCH] Give each account its own folder of uploaded files to delete (#344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uploaded images, fonts and sounds belonged to nobody. Every account saw every file in the pickers, and only an administrator could delete one, through the API alone. Now every account uploads into a folder of its own, sees its own files and nobody's, and can delete a file of its own that no project uses. - Files are stored as data///., and the folder is part of asset.file. A name without a folder belongs to nobody. - The folder is a random 12-character name kept in users.json and given with the first upload. Renaming an account moves no file, and a released game's URLs carry no account name. - The list shows the caller's own files and nobody's. An administrator sees all, each with owner and deletable. - DELETE takes the name with its folder. It answers 403 unless the file is the caller's or the caller is an administrator, and 409 while a project, snapshot or release of any account names it (FileUsage). The 409 names the caller's own projects only. - The pickers have a Delete button. The first press asks, the second deletes, and a refusal shows the server's reason. - A deleted account's folder stays and only administrators see it. - tools/migrate-file-owners.php moved every used file into the folder of the account using it (a copy each for two accounts) and rewrote projects, snapshots and releases. A project's revision goes up, so an open editor gets a conflict instead of saving old names back. - Font families are named after the hash alone, so a folder never ends up in a CSS family name. Run on stein2 on 2026-10-07, backup in /root/config-backup-20261007/data-before-344: 25 files used, 27 copies, 8 projects, 53 snapshots and 1 release rewritten, 30 unused files stay nobody's. Afterwards all 600 references resolve to a file in their owner's folder. Found while building: the Delete button cut its new label to "Del…" because a button does not measure itself again on a new text. The picker now lays its button row out again. Tests: - PHP files.test.php (new): 46 checks, counter-check 3 red - Unit core + editor: 220 files, 3403 tests - e2e 34 (melektaus-atx): ALL PASSED - 67 checks (2 new) - Gate (melektaus-atx): CHECK EXIT 0, 256 files, 3977 tests Co-Authored-By: Claude Opus 5.5 --- .gitea/workflows/ci.yml | 1 + api/index.php | 79 ++++++++---- api/lib/FileUsage.php | 127 +++++++++++++++++++ api/lib/FontStore.php | 10 +- api/lib/HashedFileStore.php | 131 +++++++++++++------ api/lib/SnapshotStore.php | 3 +- api/lib/SoundStore.php | 10 +- api/lib/TextureStore.php | 15 +-- api/lib/UserStore.php | 62 ++++++++- src/core/model/Asset.ts | 6 +- src/core/model/font.ts | 9 +- src/core/model/generated.ts | 2 +- src/core/model/sound.ts | 4 +- src/core/model/storedFile.ts | 13 ++ src/core/model/texture.ts | 4 +- src/core/store/FileStore.ts | 10 +- src/core/store/FontStore.ts | 2 +- src/core/store/HttpFileStore.ts | 8 ++ src/core/store/SoundStore.ts | 2 +- src/core/store/TextureStore.ts | 2 +- src/editor/FilePicker.ts | 86 ++++++++++++- src/editor/FormDialog.ts | 10 +- src/editor/font/FontPicker.ts | 1 + src/editor/sound/SoundPicker.ts | 1 + src/editor/texture/ImagePicker.ts | 1 + tests/core/fonts.test.ts | 2 + tests/core/http-store.test.ts | 14 +++ tests/core/texture.test.ts | 9 ++ tests/php/files.test.php | 148 ++++++++++++++++++++++ tools/e2e/12-textures.mjs | 2 +- tools/e2e/30-font.mjs | 2 +- tools/e2e/34-sound.mjs | 42 ++++++- tools/e2e/65-song.mjs | 2 +- tools/migrate-file-owners.php | 201 ++++++++++++++++++++++++++++++ 34 files changed, 910 insertions(+), 111 deletions(-) create mode 100644 api/lib/FileUsage.php create mode 100644 src/core/model/storedFile.ts create mode 100644 tests/php/files.test.php create mode 100644 tools/migrate-file-owners.php diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index d3681309..46b889d5 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -42,6 +42,7 @@ jobs: php tests/php/accounts.test.php php tests/php/snapshots.test.php php tests/php/releases.test.php + php tests/php/files.test.php suite: runs-on: game-maker-wsl diff --git a/api/index.php b/api/index.php index 84636ee4..1c32c4d0 100644 --- a/api/index.php +++ b/api/index.php @@ -51,17 +51,23 @@ declare(strict_types=1); * * GET ?r=releases/{token} → {name,publishedAt,project} | 404 * - * GET ?r=textures → [{file,name,width,height,bytes,updatedAt}] + * GET ?r=textures → [{file,name,width,height,bytes,updatedAt,owner,deletable}] * POST ?r=textures&name=x.png raw image bytes → {file,name,width,height,bytes} - * DELETE ?r=textures/{file} → 204 (administrators only, 403 for everybody else) + * DELETE ?r=textures/{file} → 204 | 403 | 404 | 409 (still used) * - * GET ?r=fonts → [{file,name,bytes,updatedAt}] + * GET ?r=fonts → [{file,name,bytes,updatedAt,owner,deletable}] * POST ?r=fonts&name=x.ttf raw font bytes → {file,name,bytes} - * DELETE ?r=fonts/{file} → 204 (administrators only) + * DELETE ?r=fonts/{file} → 204 | 403 | 404 | 409 * - * GET ?r=sounds → [{file,name,bytes,updatedAt}] + * GET ?r=sounds → [{file,name,bytes,updatedAt,owner,deletable}] * POST ?r=sounds&name=x.wav raw sound bytes → {file,name,bytes} - * DELETE ?r=sounds/{file} → 204 (administrators only) + * DELETE ?r=sounds/{file} → 204 | 403 | 404 | 409 + * + * Uploads land in the uploader's own folder, and `file` carries it: `{folder}/{sha256}.{ext}` + * (#344). A `file` without a folder belongs to nobody. A list shows the files of the one asking and + * nobody's, an administrator's every file; `owner` is the account a file belongs to (null for + * nobody, '' for a deleted account). A file goes only at its owner's or an administrator's hand, + * and only while no project, snapshot or release uses it. * * POST ?r=pixels raw cel bytes → {file,bytes} (pixel art, #130) * @@ -69,7 +75,8 @@ declare(strict_types=1); * GET ?r=plugins/{file} → the module source as text/javascript * * `name` is the file's original name, kept beside it as `{file}.json` (see OriginalName) and - * empty when none is known. It is a caption, never a path: the stored file is always `{hash}.{ext}`. + * empty when none is known. It is a caption, never a path: the stored file is always `{hash}.{ext}` + * in a folder. * * Stored images, fonts, sounds and cels are served straight from `data/textures/`, `data/fonts/`, * `data/sounds/` and `data/pixels/` as static files; the API only writes them. @@ -78,6 +85,7 @@ declare(strict_types=1); require __DIR__ . '/lib/AccountRules.php'; require __DIR__ . '/lib/AtomicFile.php'; require __DIR__ . '/lib/Auth.php'; +require __DIR__ . '/lib/FileUsage.php'; require __DIR__ . '/lib/HashedFileStore.php'; // before the font, sound and texture stores built on it require __DIR__ . '/lib/FontStore.php'; require __DIR__ . '/lib/OriginalName.php'; @@ -482,9 +490,10 @@ function routeUsers(array $parts, string $method, Auth $auth, UserStore $users): } /** - * Images, fonts and sounds: shared between the accounts and stored under the hash of their bytes. - * Everybody signed in uploads and lists them. Deleting one is for administrators only, because a - * project of another account may be wearing it – the editor itself never deletes a file. + * Images, fonts and sounds, stored under the hash of their bytes in a folder per account (#344). + * Everybody signed in uploads into their own folder and lists their own files and nobody's; an + * administrator lists every file. Deleting one is for its owner or an administrator, and refused + * while a project of any account still uses it. * * @param list $parts * @param class-string $invalid what the store throws for bytes it cannot take @@ -492,7 +501,9 @@ function routeUsers(array $parts, string $method, Auth $auth, UserStore $users): function routeFiles( array $parts, string $method, - Auth $auth, + string $user, + bool $isAdmin, + UserStore $users, HashedFileStore $store, int $maxBytes, string $noun, @@ -500,7 +511,7 @@ function routeFiles( ): never { if (count($parts) === 1) { if ($method === 'GET') { - Response::json($store->list()); + Response::json($store->list($users->folderOf($user), $isAdmin, $users->folderOwners())); } if ($method === 'POST') { $bytes = Response::rawBody(); @@ -508,7 +519,7 @@ function routeFiles( Response::error("{$noun} must be at most " . ($maxBytes / 1024 / 1024) . ' MB', 413); } try { - Response::json($store->store($bytes, (string) ($_GET['name'] ?? '')), 201); + Response::json($store->store($bytes, (string) ($_GET['name'] ?? ''), $users->folderOf($user, true)), 201); } catch (RuntimeException $e) { if (!$e instanceof $invalid) { throw $e; @@ -520,10 +531,19 @@ function routeFiles( } if ($method === 'DELETE') { - $auth->requireAdmin(); - if (!$store->delete($parts[1])) { + $file = implode('/', array_slice($parts, 1)); + if (!$store->exists($file)) { Response::error("{$noun} not found", 404); } + $folder = HashedFileStore::folderOf($file); + if (!$isAdmin && ($folder === '' || $folder !== $users->folderOf($user))) { + Response::error('Only its owner may delete this file', 403); + } + $uses = (new FileUsage(dirname(__DIR__) . '/data'))->of($file); + if ($uses !== []) { + Response::error(FileUsage::refusal($uses, $user, $isAdmin), 409); + } + $store->delete($file); Response::noContent(); } @@ -587,8 +607,14 @@ try { $route = trim((string) ($_GET['r'] ?? ''), '/'); $parts = $route === '' ? [] : explode('/', $route); - // Only a project reaches deeper than two parts: `projects/{id}/snapshots/{snapshotId}/restore`. - if (count($parts) < 1 || count($parts) > ($parts[0] === 'projects' ? 5 : 2)) { + // A project reaches deepest, `projects/{id}/snapshots/{snapshotId}/restore`; a stored file is + // `textures/{folder}/{file}`; everything else stops at two parts. + $depth = match ($parts[0] ?? '') { + 'projects' => 5, + 'textures', 'fonts', 'sounds' => 3, + default => 2, + }; + if (count($parts) < 1 || count($parts) > $depth) { Response::error('Not found', 404); } @@ -603,17 +629,20 @@ try { if ($parts[0] === 'releases') { routeReleases($parts, $method); } - // Images, fonts, sounds, cels and plugins are shared between accounts; being signed in is still the - // price of admission, so an uploaded file cannot be dropped in by a stranger. + // Being signed in is the price of admission for everything below, so an uploaded file cannot be + // dropped in by a stranger. $user = $auth->require(); + $isAdmin = $auth->role($user) === 'admin'; $data = dirname(__DIR__) . '/data'; match ($parts[0]) { - 'projects' => routeProjects($parts, $method, $user, $auth->role($user) === 'admin'), + 'projects' => routeProjects($parts, $method, $user, $isAdmin), 'textures' => routeFiles( $parts, $method, - $auth, + $user, + $isAdmin, + $users, new TextureStore("{$data}/textures"), MAX_TEXTURE_BYTES, 'Image', @@ -622,7 +651,9 @@ try { 'fonts' => routeFiles( $parts, $method, - $auth, + $user, + $isAdmin, + $users, new FontStore("{$data}/fonts"), MAX_FONT_BYTES, 'Font', @@ -631,7 +662,9 @@ try { 'sounds' => routeFiles( $parts, $method, - $auth, + $user, + $isAdmin, + $users, new SoundStore("{$data}/sounds"), MAX_SOUND_BYTES, 'Sound', diff --git a/api/lib/FileUsage.php b/api/lib/FileUsage.php new file mode 100644 index 00000000..a807ee87 --- /dev/null +++ b/api/lib/FileUsage.php @@ -0,0 +1,127 @@ + + */ + public function of(string $file): array + { + $used = []; + foreach (glob("{$this->data}/projects/*.json") ?: [] as $path) { + if ($this->names($path, $file)) { + $used[basename($path, '.json')] = true; + } + } + foreach (glob("{$this->data}/snapshots/*/*.json") ?: [] as $path) { + if ($this->names($path, $file)) { + $used[basename(dirname($path))] = true; + } + } + foreach (glob("{$this->data}/releases/*.json") ?: [] as $path) { + $release = $this->decoded($path, $file); + if ($release !== null && is_string($release['projectId'] ?? null) && self::uses($release['project'] ?? null, $file)) { + $used[$release['projectId']] = true; + } + } + $uses = []; + foreach (array_keys($used) as $id) { + $uses[] = ['id' => (string) $id, ...$this->project((string) $id)]; + } + return $uses; + } + + /** + * Why a file cannot go. The asker's own projects are named; somebody else's only for an + * administrator, everybody else learns no more than that there is one. + * + * @param list $uses + */ + public static function refusal(array $uses, string $asker, bool $seesAll): string + { + /** @var array $labels each name once, with how many projects carry it */ + $labels = []; + $others = 0; + foreach ($uses as $use) { + if (strcasecmp($use['owner'], $asker) === 0) { + $label = "“{$use['name']}”"; + } elseif ($seesAll) { + $label = "“{$use['name']}” of " . ($use['owner'] === '' ? 'nobody' : $use['owner']); + } else { + $others++; + continue; + } + $labels[$label] = ($labels[$label] ?? 0) + 1; + } + $named = []; + foreach ($labels as $label => $count) { + $named[] = $count === 1 ? $label : "{$label} ({$count} projects)"; + } + if ($others > 0) { + $named[] = $others === 1 ? 'a project of another account' : "{$others} projects of other accounts"; + } + $list = count($named) > 1 ? implode(', ', array_slice($named, 0, -1)) . ' and ' . end($named) : $named[0]; + return "Still used by {$list}, counting snapshots and releases"; + } + + /** Whether a value holds `$file` as a string anywhere inside it. */ + public static function uses(mixed $value, string $file): bool + { + if (is_string($value)) { + return $value === $file; + } + if (is_array($value)) { + foreach ($value as $item) { + if (self::uses($item, $file)) { + return true; + } + } + } + return false; + } + + private function names(string $path, string $file): bool + { + $doc = $this->decoded($path, $file); + return $doc !== null && self::uses($doc['project'] ?? null, $file); + } + + /** + * The document, or null when it cannot name the file. The text is searched for the hash first, + * which has no slash to be escaped: most documents are never decoded. + */ + private function decoded(string $path, string $file): ?array + { + $raw = file_get_contents($path); + if ($raw === false || !str_contains($raw, basename($file))) { + return null; + } + $doc = json_decode($raw, true); + return is_array($doc) ? $doc : null; + } + + /** @return array{owner:string,name:string} */ + private function project(string $id): array + { + $path = "{$this->data}/projects/{$id}.json"; + $raw = is_file($path) ? file_get_contents($path) : false; + $doc = $raw === false ? null : json_decode($raw, true); + return [ + 'owner' => is_string($doc['owner'] ?? null) ? $doc['owner'] : '', + 'name' => is_string($doc['name'] ?? null) ? $doc['name'] : 'Untitled', + ]; + } +} diff --git a/api/lib/FontStore.php b/api/lib/FontStore.php index 7960bec0..87647ab2 100644 --- a/api/lib/FontStore.php +++ b/api/lib/FontStore.php @@ -2,7 +2,7 @@ declare(strict_types=1); /** - * Content-addressed font store: `data/fonts/{sha256}.{ext}` (see HashedFileStore). + * Content-addressed font store: `data/fonts/{folder}/{sha256}.{ext}` (see HashedFileStore). * * A font is stored byte for byte. There is no re-encoding step as there is for images, so what * the browser loads is exactly what was picked. The picker shows each face by rendering with it, @@ -23,9 +23,9 @@ final class FontStore extends HashedFileStore 'wOF2' => 'woff2', ]; - public static function isValidName(string $name): bool + protected static function filePattern(): string { - return (bool) preg_match('/^[a-f0-9]{64}\.(ttf|otf|woff|woff2)$/', $name); + return '[a-f0-9]{64}\.(ttf|otf|woff|woff2)'; } /** @@ -42,7 +42,7 @@ final class FontStore extends HashedFileStore * @return array{file:string,name:string,bytes:int} * @throws InvalidFontException when the bytes are not a supported font */ - public function store(string $bytes, string $originalName = ''): array + public function store(string $bytes, string $originalName, string $folder): array { $extension = self::SIGNATURES[substr($bytes, 0, 4)] ?? ''; if ($extension === '') { @@ -53,7 +53,7 @@ final class FontStore extends HashedFileStore throw new InvalidFontException('Font file is truncated'); } - return $this->keep($bytes, $extension, $originalName); + return $this->keep($bytes, $extension, $originalName, $folder); } } diff --git a/api/lib/HashedFileStore.php b/api/lib/HashedFileStore.php index 27b10c86..88755a58 100644 --- a/api/lib/HashedFileStore.php +++ b/api/lib/HashedFileStore.php @@ -2,13 +2,17 @@ declare(strict_types=1); /** - * The shared half of the image, font and sound stores: `{dir}/{sha256}.{ext}`. + * The shared half of the image, font and sound stores: `{dir}/{folder}/{sha256}.{ext}`. * - * The name of a file is the hash of its bytes, which buys four things at once: uploading the same + * The name of a file is the hash of its bytes, which buys three things at once: uploading the same * file twice costs one file, a stored file can be cached forever (its URL cannot ever mean - * different bytes), projects reference a short string instead of embedding megabytes of base64, - * and deleting an asset is undoable for free because nothing here is removed when it stops being - * referenced. What a store adds is which bytes it takes, and what its records say beyond the name. + * different bytes), and projects reference a short string instead of embedding megabytes of base64. + * What a store adds is which bytes it takes, and what its records say beyond the name. + * + * Every account uploads into a folder of its own (#344, see `UserStore::folderOf`), and the folder + * is part of the name a project stores: `{folder}/{sha256}.{ext}`. A name without a folder is a + * file that belongs to nobody – what everything uploaded before #344 and used by no project is. + * The picker shows an account its own files and nobody's; an administrator sees them all. */ abstract class HashedFileStore { @@ -19,30 +23,45 @@ abstract class HashedFileStore } } - /** A stored file name, as it appears in a project document. */ - abstract public static function isValidName(string $name): bool; + /** The hash-and-extension half of a stored name, as a regular expression without delimiters. */ + abstract protected static function filePattern(): string; + + /** A stored file name as it appears in a project document, with or without its folder. */ + public static function isValidName(string $name): bool + { + return (bool) preg_match('#^(?:' . UserStore::FOLDER_PATTERN . '/)?' . static::filePattern() . '$#', $name); + } + + /** The folder a stored name lies in, '' for a file that belongs to nobody. */ + public static function folderOf(string $name): string + { + $slash = strpos($name, '/'); + return $slash === false ? '' : substr($name, 0, $slash); + } + + public function exists(string $file): bool + { + return static::isValidName($file) && is_file($this->dir . '/' . $file); + } /** - * Every stored file, newest first – what the picker of its kind offers. + * What the picker of its kind offers, newest first: the files of `$folder` and the ones that + * belong to nobody, or with `$seesAll` every file there is. `owner` is the account a file + * belongs to, null for nobody, '' for an account that was deleted; `deletable` says whether + * the one asking may try (whether a project still uses it is only known when they do). * - * @return list> + * @param array $owners folder => account, see `UserStore::folderOwners` + * @return list> */ - public function list(): array + public function list(string $folder, bool $seesAll, array $owners): array { - $items = []; - foreach (glob($this->dir . '/*.*') ?: [] as $path) { - $file = basename($path); - // Sidecars share the directory and are skipped: `{hash}.png.json` is not a valid name. - if (!static::isValidName($file)) { + $items = $this->listIn('', null, $seesAll); + foreach (glob($this->dir . '/*', GLOB_ONLYDIR) ?: [] as $path) { + $name = basename($path); + if (!preg_match('/^' . UserStore::FOLDER_PATTERN . '$/', $name) || (!$seesAll && $name !== $folder)) { continue; } - $items[] = [ - 'file' => $file, - 'name' => OriginalName::read($this->dir, $file), - ...$this->details($path), - 'bytes' => (int) filesize($path), - 'updatedAt' => gmdate('c', (int) filemtime($path)), - ]; + array_push($items, ...$this->listIn($name, $owners[$name] ?? '', true)); } usort($items, static fn(array $a, array $b): int => strcmp($b['updatedAt'], $a['updatedAt'])); return $items; @@ -50,35 +69,41 @@ abstract class HashedFileStore public function delete(string $file): bool { - if (!static::isValidName($file)) { + if (!$this->exists($file)) { return false; } - $path = $this->dir . '/' . $file; - if (!is_file($path) || !unlink($path)) { + [$dir, $base] = $this->split($file); + if (!unlink($dir . '/' . $base)) { return false; } - OriginalName::delete($this->dir, $file); + OriginalName::delete($dir, $base); return true; } /** - * Keeps bytes that passed the store's checks under their hash, with `$originalName` beside them - * as a caption (see OriginalName), and returns the record. Bytes stored before are not written - * again; a new name renames them. + * Keeps bytes that passed the store's checks under their hash in `$folder`, with `$originalName` + * beside them as a caption (see OriginalName), and returns the record. Bytes stored there before + * are not written again; a new name renames them. * * @param array $details what the record says between the name and the size * @return array */ - protected function keep(string $bytes, string $extension, string $originalName, array $details = []): array + protected function keep(string $bytes, string $extension, string $originalName, string $folder, array $details = []): array { - $file = hash('sha256', $bytes) . '.' . $extension; - $path = $this->dir . '/' . $file; - if (!is_file($path)) { - AtomicFile::write($path, $bytes, AtomicFile::SERVED, 'stored file'); + if (!preg_match('/^' . UserStore::FOLDER_PATTERN . '$/', $folder)) { + throw new InvalidArgumentException("Invalid folder: {$folder}"); } - OriginalName::write($this->dir, $file, $originalName); + $dir = $this->dir . '/' . $folder; + if (!is_dir($dir) && !mkdir($dir, 0775, true) && !is_dir($dir)) { + throw new RuntimeException("Cannot create directory {$dir}"); + } + $base = hash('sha256', $bytes) . '.' . $extension; + if (!is_file($dir . '/' . $base)) { + AtomicFile::write($dir . '/' . $base, $bytes, AtomicFile::SERVED, 'stored file'); + } + OriginalName::write($dir, $base, $originalName); - return ['file' => $file, 'name' => OriginalName::read($this->dir, $file), ...$details, 'bytes' => strlen($bytes)]; + return ['file' => "{$folder}/{$base}", 'name' => OriginalName::read($dir, $base), ...$details, 'bytes' => strlen($bytes)]; } /** @@ -91,4 +116,38 @@ abstract class HashedFileStore { return []; } + + /** @return list> */ + private function listIn(string $folder, ?string $owner, bool $deletable): array + { + $dir = $folder === '' ? $this->dir : $this->dir . '/' . $folder; + $prefix = $folder === '' ? '' : $folder . '/'; + $items = []; + foreach (glob($dir . '/*.*') ?: [] as $path) { + $base = basename($path); + // Sidecars share the directory and are skipped: `{hash}.png.json` is not a valid name. + if (!static::isValidName($base)) { + continue; + } + $items[] = [ + 'file' => $prefix . $base, + 'name' => OriginalName::read($dir, $base), + ...$this->details($path), + 'bytes' => (int) filesize($path), + 'updatedAt' => gmdate('c', (int) filemtime($path)), + 'owner' => $owner, + 'deletable' => $deletable, + ]; + } + return $items; + } + + /** @return array{0:string,1:string} the directory a stored name lies in, and its last part */ + private function split(string $file): array + { + $folder = self::folderOf($file); + return $folder === '' + ? [$this->dir, $file] + : [$this->dir . '/' . $folder, substr($file, strlen($folder) + 1)]; + } } diff --git a/api/lib/SnapshotStore.php b/api/lib/SnapshotStore.php index 79d1f980..3e32409b 100644 --- a/api/lib/SnapshotStore.php +++ b/api/lib/SnapshotStore.php @@ -6,7 +6,8 @@ declare(strict_types=1); * project as it was stored, with { id, name, revision, createdAt, project }. * * A copy of the project document is all a snapshot needs: images, fonts, sounds and cels are - * content-addressed and never removed, so whatever the copy names is still there to load. + * content-addressed, and a file a snapshot names cannot be deleted (#344, see FileUsage), so + * whatever the copy names is still there to load. * * The store checks nothing about who asks. The router reaches it only after `Store::load` found * the project for that user, so the snapshots of a project are seen by exactly those who see it. diff --git a/api/lib/SoundStore.php b/api/lib/SoundStore.php index 16195470..259e2b3e 100644 --- a/api/lib/SoundStore.php +++ b/api/lib/SoundStore.php @@ -2,7 +2,7 @@ declare(strict_types=1); /** - * Content-addressed sound store: `data/sounds/{sha256}.{ext}` (see HashedFileStore). + * Content-addressed sound store: `data/sounds/{folder}/{sha256}.{ext}` (see HashedFileStore). * * A sound is stored byte for byte. There is no re-encoding step as there is for images: the * browser decodes what was picked, and nothing here could encode audio anyway. The picker plays a @@ -20,9 +20,9 @@ final class SoundStore extends HashedFileStore 'ID3' => 'mp3', ]; - public static function isValidName(string $name): bool + protected static function filePattern(): string { - return (bool) preg_match('/^[a-f0-9]{64}\.(mp3|ogg|wav|m4a)$/', $name); + return '[a-f0-9]{64}\.(mp3|ogg|wav|m4a)'; } /** @@ -38,14 +38,14 @@ final class SoundStore extends HashedFileStore * @return array{file:string,name:string,bytes:int} * @throws InvalidSoundException when the bytes are not a supported sound */ - public function store(string $bytes, string $originalName = ''): array + public function store(string $bytes, string $originalName, string $folder): array { $extension = self::extensionOf($bytes); if ($extension === '') { throw new InvalidSoundException('Not a sound file – use MP3, OGG, WAV or M4A'); } - return $this->keep($bytes, $extension, $originalName); + return $this->keep($bytes, $extension, $originalName, $folder); } /** The format these bytes are, `''` when they are none we take. */ diff --git a/api/lib/TextureStore.php b/api/lib/TextureStore.php index a909a3d3..f39593f4 100644 --- a/api/lib/TextureStore.php +++ b/api/lib/TextureStore.php @@ -2,11 +2,12 @@ declare(strict_types=1); /** - * Content-addressed image store: `data/textures/{sha256}.{ext}` (see HashedFileStore). + * Content-addressed image store: `data/textures/{folder}/{sha256}.{ext}` (see HashedFileStore). * * The flip side of hashed names is that files outlive the projects that used them: nothing - * collects the unreferenced ones. They stay pickable instead – `GET ?r=textures` is what the - * editor's image picker offers, so a picture one project dropped is still there for the next. + * collects the unreferenced ones by itself. They stay pickable instead – `GET ?r=textures` is what + * the editor's image picker offers, so a picture one project dropped is still there for the next – + * until their owner deletes them (#344). */ final class TextureStore extends HashedFileStore { @@ -32,9 +33,9 @@ final class TextureStore extends HashedFileStore */ private const MAX_DIMENSION = 8192; - public static function isValidName(string $name): bool + protected static function filePattern(): string { - return (bool) preg_match('/^[a-f0-9]{64}\.(png|jpg|webp|gif)$/', $name); + return '[a-f0-9]{64}\.(png|jpg|webp|gif)'; } /** @@ -50,7 +51,7 @@ final class TextureStore extends HashedFileStore * @return array{file:string,name:string,width:int,height:int,bytes:int} * @throws InvalidImageException when the bytes are not a supported image */ - public function store(string $bytes, string $originalName = ''): array + public function store(string $bytes, string $originalName, string $folder): array { $size = @getimagesizefromstring($bytes); if ($size === false) { @@ -73,7 +74,7 @@ final class TextureStore extends HashedFileStore throw new InvalidImageException('Image must be between 1 and ' . self::MAX_DIMENSION . ' pixels per side'); } - return $this->keep($bytes, $extension, $originalName, ['width' => $width, 'height' => $height]); + return $this->keep($bytes, $extension, $originalName, $folder, ['width' => $width, 'height' => $height]); } /** diff --git a/api/lib/UserStore.php b/api/lib/UserStore.php index a4759f2b..73a1638e 100644 --- a/api/lib/UserStore.php +++ b/api/lib/UserStore.php @@ -2,13 +2,18 @@ declare(strict_types=1); /** - * The accounts, as one JSON file: `data/users.json` holding `{username, password, role, createdAt}` - * per user, where `password` is a hash from `password_hash` – the plain one is never stored. + * The accounts, as one JSON file: `data/users.json` holding `{username, password, role, createdAt, + * folder}` per user, where `password` is a hash from `password_hash` – the plain one is never stored. * * An entry without `role` is a `user` (#180): the weaker of the two is what a missing field means, * so a hand-written file never hands out rights by accident. `createdAt` is missing on the accounts * from before #179 and stays empty for them. * + * `folder` names the account's own directory under `data/textures/`, `data/fonts/` and + * `data/sounds/` (#344). It is random rather than the name, so renaming an account moves no file + * and rewrites no project, and the URL of a released game's files says nothing about whose they + * are. An account gets one with its first upload. + * * Accounts are made from the command line (`tools/make-user.php`) or by an administrator through * the `users` routes (#179); nobody can sign up over the wire. */ @@ -23,6 +28,9 @@ final class UserStore public const ROLES = ['admin', 'user']; + /** What a folder name looks like, for the file stores to check a path against. */ + public const FOLDER_PATTERN = '[a-f0-9]{12}'; + public function __construct(private readonly string $file) {} public static function isValidName(string $name): bool @@ -78,6 +86,44 @@ final class UserStore ); } + /** + * The account's own folder for uploads. With `$create` an account without one is given one; + * without it, '' means it has never uploaded anything. + */ + public function folderOf(string $username, bool $create = false): string + { + $user = $this->find($username); + if ($user === null) { + return ''; + } + if ($user['folder'] !== '' || !$create) { + return $user['folder']; + } + $taken = array_column($this->all(), 'folder'); + do { + $folder = bin2hex(random_bytes(6)); + } while (in_array($folder, $taken, true)); + $this->change($username, static fn(array $u): array => ['folder' => $folder] + $u); + return $folder; + } + + /** + * Which account each folder belongs to. A folder missing here belonged to an account that was + * deleted; its files stay, for administrators to see (#344). + * + * @return array + */ + public function folderOwners(): array + { + $owners = []; + foreach ($this->all() as $user) { + if ($user['folder'] !== '') { + $owners[$user['folder']] = $user['username']; + } + } + return $owners; + } + public function adminCount(): int { return count(array_filter($this->all(), static fn(array $u): bool => $u['role'] === 'admin')); @@ -103,6 +149,7 @@ final class UserStore 'password' => password_hash($password, PASSWORD_DEFAULT), 'role' => $role ?? $existing['role'] ?? 'user', 'createdAt' => $existing !== null ? $existing['createdAt'] : gmdate('c'), + 'folder' => $existing['folder'] ?? '', ]; $this->write($users); } @@ -159,7 +206,7 @@ final class UserStore return true; } - /** @return list */ + /** @return list */ private function without(string $username): array { return array_values( @@ -167,7 +214,7 @@ final class UserStore ); } - /** @return array{username:string,password:string,role:string,createdAt:string}|null */ + /** @return array{username:string,password:string,role:string,createdAt:string,folder:string}|null */ private function find(string $username): ?array { foreach ($this->all() as $user) { @@ -178,7 +225,7 @@ final class UserStore return null; } - /** @return list */ + /** @return list */ private function all(): array { if (!is_file($this->file)) { @@ -198,13 +245,16 @@ final class UserStore 'password' => $user['password'], 'role' => is_string($role) && self::isValidRole($role) ? $role : 'user', 'createdAt' => is_string($user['createdAt'] ?? null) ? $user['createdAt'] : '', + 'folder' => is_string($user['folder'] ?? null) && preg_match('/^' . self::FOLDER_PATTERN . '$/', $user['folder']) + ? $user['folder'] + : '', ]; } } return $users; } - /** @param list $users */ + /** @param list $users */ private function write(array $users): void { usort($users, static fn(array $a, array $b): int => strcmp($a['username'], $b['username'])); diff --git a/src/core/model/Asset.ts b/src/core/model/Asset.ts index 69b1a7d5..bfc3ce8d 100644 --- a/src/core/model/Asset.ts +++ b/src/core/model/Asset.ts @@ -195,7 +195,7 @@ export interface TextureFrame extends PictureCut { } /** - * `file` names the image in `data/textures/` and is content-addressed (`{sha256}.{ext}`), so it + * `file` names the image in `data/textures/` and is content-addressed (`{folder}/{sha256}.{ext}`), so it * can be cached forever; empty means the upload failed or has not happened yet. `width`/`height` * are the size as uploaded, so frames can be sliced and validated before the image loads. */ @@ -295,7 +295,7 @@ export interface ShaderFunctionAsset extends AssetBase { /** * `file` names the typeface in `data/fonts/` the way a texture names its image: content-addressed - * `{sha256}.{ext}`, empty when the upload failed. A text object wears it through its `font` + * `{folder}/{sha256}.{ext}`, empty when the upload failed. A text object wears it through its `font` * property; the family the browser knows it under is derived from the file name, see `model/font`. */ export interface FontAsset extends AssetBase { @@ -305,7 +305,7 @@ export interface FontAsset extends AssetBase { /** * `file` names the sound in `data/sounds/` the way a font names its typeface: content-addressed - * `{sha256}.{ext}`, empty when the upload failed. Nothing wears a sound – a graph plays it with + * `{folder}/{sha256}.{ext}`, empty when the upload failed. Nothing wears a sound – a graph plays it with * `Play sound`, and `props` says how loud and whether it repeats (see `properties/sounds.ts`). */ export interface SoundAsset extends AssetBase { diff --git a/src/core/model/font.ts b/src/core/model/font.ts index 341040f1..b47784e1 100644 --- a/src/core/model/font.ts +++ b/src/core/model/font.ts @@ -1,8 +1,8 @@ import { findAsset, type Project } from './Project'; import { textOf } from './Properties'; +import { storedFileBase, storedFilePattern } from './storedFile'; -/** Mirrors `FontStore::isValidName` on the server, where the name is derived from the bytes. */ -const FILE_PATTERN = /^[a-f0-9]{64}\.(ttf|otf|woff|woff2)$/; +const FILE_PATTERN = storedFilePattern('ttf|otf|woff|woff2'); export function isFontFile(name: string): boolean { return FILE_PATTERN.test(name); @@ -11,10 +11,11 @@ export function isFontFile(name: string): boolean { /** * The CSS family a file is registered under. Derived from the content-addressed name, so the same * bytes are always one family and two uploads of one typeface cannot fight over a name – the name - * inside the font is never used for this, only for the asset's label. + * inside the font is never used for this, only for the asset's label. The folder is left out: the + * same bytes in two accounts' folders are the same face. */ export function fontFamilyOf(file: string): string { - return `gm-font-${file.slice(0, 16)}`; + return `gm-font-${storedFileBase(file).slice(0, 16)}`; } /** diff --git a/src/core/model/generated.ts b/src/core/model/generated.ts index f841d7f3..caa28dcd 100644 --- a/src/core/model/generated.ts +++ b/src/core/model/generated.ts @@ -4,7 +4,7 @@ * A picture is named by `TextureRef.file` everywhere: the sprite, the tile, the tilemap mesh, the * particle source and the picker all pass that string around. A generated picture therefore gets a * key of its own, `gen:@`, which no uploaded file can look like (those are - * `.`, see `isTextureFile`). Whoever draws it is asked for it under that key. + * `/.`, see `isTextureFile`). Whoever draws it is asked for it under that key. * * The revision is the other half of the contract: it changes whenever the drawing changes, so a * changed picture is a new key. Nothing has to ask "is this still valid" – the library rasterises diff --git a/src/core/model/sound.ts b/src/core/model/sound.ts index be733908..4a54c13d 100644 --- a/src/core/model/sound.ts +++ b/src/core/model/sound.ts @@ -1,7 +1,7 @@ import { findAsset, type Project } from './Project'; +import { storedFilePattern } from './storedFile'; -/** Mirrors `SoundStore::isValidName` on the server, where the name is derived from the bytes. */ -const FILE_PATTERN = /^[a-f0-9]{64}\.(mp3|ogg|wav|m4a)$/; +const FILE_PATTERN = storedFilePattern('mp3|ogg|wav|m4a'); export function isSoundFile(name: string): boolean { return FILE_PATTERN.test(name); diff --git a/src/core/model/storedFile.ts b/src/core/model/storedFile.ts new file mode 100644 index 00000000..09cb72e3 --- /dev/null +++ b/src/core/model/storedFile.ts @@ -0,0 +1,13 @@ +/** + * An uploaded image, font or sound is named `/.` (#344): the folder is the + * uploading account's, and a name without one is a file that belongs to nobody. Mirrors + * `HashedFileStore::isValidName` on the server, where the name is derived from the bytes. + */ +export function storedFilePattern(extensions: string): RegExp { + return new RegExp(`^(?:[a-f0-9]{12}/)?[a-f0-9]{64}\\.(${extensions})$`); +} + +/** The hash-and-extension part of a stored name, without its folder. */ +export function storedFileBase(file: string): string { + return file.slice(file.lastIndexOf('/') + 1); +} diff --git a/src/core/model/texture.ts b/src/core/model/texture.ts index 3252a465..eb732d4d 100644 --- a/src/core/model/texture.ts +++ b/src/core/model/texture.ts @@ -3,9 +3,9 @@ import { generatedKey, generatedPictureOf } from './generated'; import { findPixelFrame, pixelFrameRect } from './pixelart'; import { type PropertyBag, textOf, type Vec2 } from './Properties'; import { findAsset, type Project } from './Project'; +import { storedFilePattern } from './storedFile'; -/** Mirrors `TextureStore::isValidName` on the server, where the name is derived from the bytes. */ -const FILE_PATTERN = /^[a-f0-9]{64}\.(png|jpg|webp|gif)$/; +const FILE_PATTERN = storedFilePattern('png|jpg|webp|gif'); export function isTextureFile(name: string): boolean { return FILE_PATTERN.test(name); diff --git a/src/core/store/FileStore.ts b/src/core/store/FileStore.ts index 0b5894fc..c5f817db 100644 --- a/src/core/store/FileStore.ts +++ b/src/core/store/FileStore.ts @@ -1,6 +1,6 @@ /** * What the server answers for an upload: the content-addressed name it stored the bytes under - * (`{sha256}.{ext}`, see `isTextureFile`/`isFontFile`/`isSoundFile`) and the caption kept beside it. + * (`{folder}/{sha256}.{ext}`, see `storedFilePattern`) and the caption kept beside it. */ export interface UploadedFile { file: string; @@ -12,6 +12,10 @@ export interface UploadedFile { /** A file the server holds, whether any project references it or not. */ export interface StoredFile extends UploadedFile { updatedAt: string; + /** The account the file belongs to (#344): null for nobody, '' for an account that was deleted. */ + owner: string | null; + /** Whether the one asking may delete it – whether a project still uses it, only the delete finds out. */ + deletable: boolean; } /** @@ -19,9 +23,11 @@ export interface StoredFile extends UploadedFile { * so there is nothing else to send. `type` only fills the Content-Type header – the server trusts * the bytes, not the label – and uploading the same bytes twice yields the same file, the hash of * its content. `name` is the caption kept beside it; uploading the same bytes under a new name - * renames what the pickers show. Nothing here deletes a file: another account's project may wear it. + * renames what the pickers show. `delete` throws with the server's reason when the file is still + * used by a project of any account, or is not the asker's to delete. */ export interface FileStore { upload(bytes: Blob | ArrayBuffer, type: string, name?: string): Promise; list(): Promise; + delete(file: string): Promise; } diff --git a/src/core/store/FontStore.ts b/src/core/store/FontStore.ts index c2c4ac5e..bafb6f98 100644 --- a/src/core/store/FontStore.ts +++ b/src/core/store/FontStore.ts @@ -1,7 +1,7 @@ import type { FileStore, StoredFile, UploadedFile } from './FileStore'; /** - * Stored in `data/fonts/` as `{sha256}.{ttf|otf|woff|woff2}`. Unlike an image a font is never + * Stored in `data/fonts/` as `{folder}/{sha256}.{ttf|otf|woff|woff2}`. Unlike an image a font is never * re-encoded on the way, so what the server stores is what was picked. */ export type UploadedFont = UploadedFile; diff --git a/src/core/store/HttpFileStore.ts b/src/core/store/HttpFileStore.ts index 465359e3..d4ac072f 100644 --- a/src/core/store/HttpFileStore.ts +++ b/src/core/store/HttpFileStore.ts @@ -27,4 +27,12 @@ export class HttpFileStore< async list(): Promise { return apiRequest(this.endpoint, 'GET', this.route, {}); } + + /** The reason a refusal gives is meant for the user, so it is thrown as it is, without the route. */ + async delete(file: string): Promise { + await apiRequest(this.endpoint, 'DELETE', `${this.route}/${file}`, {}, (status, data) => { + const error = (data as { error?: unknown } | null)?.error; + return status >= 400 && typeof error === 'string' ? new Error(error) : undefined; + }); + } } diff --git a/src/core/store/SoundStore.ts b/src/core/store/SoundStore.ts index e4ebfc1d..530f06b7 100644 --- a/src/core/store/SoundStore.ts +++ b/src/core/store/SoundStore.ts @@ -1,7 +1,7 @@ import type { FileStore, StoredFile, UploadedFile } from './FileStore'; /** - * Stored in `data/sounds/` as `{sha256}.{mp3|ogg|wav|m4a}`, never re-encoded. How long a sound + * Stored in `data/sounds/` as `{folder}/{sha256}.{mp3|ogg|wav|m4a}`, never re-encoded. How long a sound * plays is nowhere in here, because only a decoder knows that. */ export type UploadedSound = UploadedFile; diff --git a/src/core/store/TextureStore.ts b/src/core/store/TextureStore.ts index 128bb789..9d475a06 100644 --- a/src/core/store/TextureStore.ts +++ b/src/core/store/TextureStore.ts @@ -1,7 +1,7 @@ import type { FileStore, StoredFile, UploadedFile } from './FileStore'; /** - * Stored in `data/textures/` as `{sha256}.{png|jpg|webp|gif}`. A picture may be re-encoded on the + * Stored in `data/textures/` as `{folder}/{sha256}.{png|jpg|webp|gif}`. A picture may be re-encoded on the * way, so `name` can be a `.png` name on a file stored as `.webp`. */ export interface UploadedTexture extends UploadedFile { diff --git a/src/editor/FilePicker.ts b/src/editor/FilePicker.ts index a4be137b..24681ef7 100644 --- a/src/editor/FilePicker.ts +++ b/src/editor/FilePicker.ts @@ -14,6 +14,7 @@ import type { StoredFile } from '../core/store/FileStore'; import { Button, drawControlBox, + type HStack, formatBytes, Label, Popup, @@ -46,10 +47,16 @@ export function withAssetNames( return stored.map((item) => ({ ...item, assetName: named.get(item.file) ?? null })); } -/** The line under the list for a picked file: its upload name and size, and who wears it. */ +/** Whose a file is (#344), as the line under the list says it. */ +export function ownerText(owner: string | null): string { + return owner === null ? 'no owner' : owner === '' ? 'owner deleted' : `owner ${owner}`; +} + +/** The line under the list for a picked file: its upload name and size, whose it is, and who wears it. */ function describeStored(item: PickerItem): string { const size = item.name ? `${item.name} – ${formatBytes(item.bytes)}` : formatBytes(item.bytes); - return item.assetName ? `${size} – already in the project as "${item.assetName}"` : size; + const owned = `${size} – ${ownerText(item.owner)}`; + return item.assetName ? `${owned} – already in the project as "${item.assetName}"` : owned; } export interface PickerRow { @@ -194,6 +201,9 @@ export abstract class RowList ex } } +/** What the Delete button says while it waits for the second press that really deletes. */ +const ARMED_LABEL = 'Delete for good'; + export interface PickerWords { title: string; loading: string; @@ -232,11 +242,16 @@ export abstract class FilePicker extends Popup { private readonly status: Label; private readonly detail: Label; private readonly use: Button; + private readonly remove: Button; + private readonly buttons: HStack; + /** The file the Delete button was pressed for once; the second press deletes it. */ + private armed: string | null = null; constructor( protected readonly list: PickerList, private readonly words: PickerWords, upload: () => void, + private readonly deleteFile: (file: string) => Promise, ) { super({ modal: true, padding: Theme.padding * 2, gap: Theme.gap }); this.content.add(new Label(words.title, { style: 'bodyBold' })); @@ -260,16 +275,39 @@ export abstract class FilePicker extends Popup { this.use = new Button(words.use, { variant: 'primary' }); this.use.onClick = () => this.confirm(); this.use.setEnabled(false); - this.content.add(dialogButtons(this.closeThen(), this.use, uploadButton)); + this.remove = new Button('Delete', { variant: 'danger' }); + this.remove.onClick = () => void this.pressDelete(); + this.remove.setEnabled(false); + this.buttons = dialogButtons(this.closeThen(), this.use, [uploadButton, this.remove]); + this.content.add(this.buttons); } - /** What the dialog offers, which of it the project already wears, and what is picked (test hook). */ - debugInfo(): { files: string[]; used: string[]; captions: string[]; selected: string | null } { + /** + * What the dialog offers, which of it the project already wears, what is picked, the line under + * the list and the Delete button with its centre (test hook). + */ + debugInfo(): { + files: string[]; + used: string[]; + captions: string[]; + selected: string | null; + detail: string; + deleteButton: { enabled: boolean; label: string; shown: string; x: number; y: number }; + } { + const at = this.remove.getGlobalPosition(); return { files: this.list.fileList, used: this.list.usedFiles, captions: this.list.captions, selected: this.list.selected?.file ?? null, + detail: this.detail.text, + deleteButton: { + enabled: this.remove.enabled, + label: this.remove.text, + shown: this.remove.shownText, + x: at.x + this.remove.w / 2, + y: at.y + this.remove.h / 2, + }, }; } @@ -308,11 +346,49 @@ export abstract class FilePicker extends Popup { private refreshSelection(): void { const item = this.list.selected; + this.armed = null; + this.setDeleteLabel('Delete'); this.use.setEnabled(item !== null && this.canUse(item)); + this.remove.setEnabled(item?.deletable ?? false); this.detail.setText(item ? this.describe(item) : (this.words.idle ?? '')); this.refreshLayout(); } + /** A button does not measure itself again on a new text; the row around it has to (see `Button.setText`). */ + private setDeleteLabel(label: string): void { + this.remove.setText(label); + this.buttons.layout(); + } + + /** + * Deleting a file cannot be taken back, so the first press only asks; the second deletes. The + * server refuses a file a project of any account still uses, and its reason is shown as it is. + */ + private async pressDelete(): Promise { + const item = this.list.selected; + if (!item?.deletable) return; + if (this.armed !== item.file) { + this.armed = item.file; + this.setDeleteLabel(ARMED_LABEL); + this.detail.setText(`Press again to delete ${item.name || `this ${this.words.noun}`} from the server.`); + this.refreshLayout(); + return; + } + this.remove.setEnabled(false); + try { + await this.deleteFile(item.file); + } catch (error) { + this.refreshSelection(); + this.detail.setText(`Not deleted: ${error instanceof Error ? error.message : String(error)}`); + this.refreshLayout(); + return; + } + if (this.list.destroyed) return; + await this.load(); + this.detail.setText(`Deleted ${item.name || `the ${this.words.noun}`}.`); + this.refreshLayout(); + } + private confirm(): void { const item = this.list.selected; if (!item || !this.canUse(item)) return; diff --git a/src/editor/FormDialog.ts b/src/editor/FormDialog.ts index ad9009b6..2da0642d 100644 --- a/src/editor/FormDialog.ts +++ b/src/editor/FormDialog.ts @@ -39,9 +39,15 @@ export abstract class FormDialog extends Popup { } /** The row a dialog ends on: Cancel and `confirm` on the right, `leading` – if any – on the left. */ -export function dialogButtons(onCancel: () => void, confirm: Button, leading?: Button): HStack { +export function dialogButtons( + onCancel: () => void, + confirm: Button, + leading?: Button | readonly Button[], +): HStack { const buttons = new HStack({ gap: Theme.gap, align: 'center' }); - if (leading) buttons.add(leading); + for (const button of leading === undefined ? [] : Array.isArray(leading) ? leading : [leading]) { + buttons.add(button); + } buttons.add(new Spacer(), 1); const cancel = new Button('Cancel'); cancel.onClick = onCancel; diff --git a/src/editor/font/FontPicker.ts b/src/editor/font/FontPicker.ts index 18c77653..ce2254c0 100644 --- a/src/editor/font/FontPicker.ts +++ b/src/editor/font/FontPicker.ts @@ -81,6 +81,7 @@ class FontPicker extends FilePicker { noun: 'font', }, () => pickAndImportFonts(deps), + (file) => deps.fonts.delete(file), ); } diff --git a/src/editor/sound/SoundPicker.ts b/src/editor/sound/SoundPicker.ts index e1cd7bf4..ca95465a 100644 --- a/src/editor/sound/SoundPicker.ts +++ b/src/editor/sound/SoundPicker.ts @@ -96,6 +96,7 @@ class SoundPicker extends FilePicker { noun: 'sound', }, () => pickAndImportSounds(deps), + (file) => deps.sounds.delete(file), ); } diff --git a/src/editor/texture/ImagePicker.ts b/src/editor/texture/ImagePicker.ts index c9db93e1..7c1a4723 100644 --- a/src/editor/texture/ImagePicker.ts +++ b/src/editor/texture/ImagePicker.ts @@ -111,6 +111,7 @@ class ImagePicker extends FilePicker { noun: 'image', }, () => pickAndImportImages(deps), + (file) => deps.textures.delete(file), ); } diff --git a/tests/core/fonts.test.ts b/tests/core/fonts.test.ts index a3f291a2..3996117b 100644 --- a/tests/core/fonts.test.ts +++ b/tests/core/fonts.test.ts @@ -40,6 +40,8 @@ describe('font files', () => { it('names one family per file, and never two for the same bytes', () => { expect(fontFamilyOf(FILE)).toBe(fontFamilyOf(FILE)); expect(fontFamilyOf(FILE)).not.toBe(fontFamilyOf(`${'b3'.repeat(32)}.ttf`)); + // The same bytes in two accounts' folders (#344) are one face, and a slash is no family name. + expect(fontFamilyOf(`0123456789ab/${FILE}`)).toBe(fontFamilyOf(FILE)); // A CSS family, so it has to survive being written into a font shorthand unquoted. expect(fontFamilyOf(FILE)).toMatch(/^[A-Za-z][\w-]*$/); }); diff --git a/tests/core/http-store.test.ts b/tests/core/http-store.test.ts index 823e8704..0fd85a54 100644 --- a/tests/core/http-store.test.ts +++ b/tests/core/http-store.test.ts @@ -105,6 +105,20 @@ describe('HttpFileStore', () => { ); }); + it('deletes a file of a folder under its route, the slash encoded', async () => { + const mock = stubFetch(null, { status: 204 }); + await new HttpFileStore('/api', 'sounds').delete(`0123456789ab/${FILE}`); + expect(mock.mock.calls[0]![0]).toBe(`/api?r=sounds%2F0123456789ab%2F${FILE}`); + expect(mock.mock.calls[0]![1]?.method).toBe('DELETE'); + }); + + it('hands on the reason a refused delete gives, as it is', async () => { + stubFetch(JSON.stringify({ error: 'Still used by “Garden”' }), { status: 409 }); + await expect(new HttpFileStore('/api', 'sounds').delete(FILE)).rejects.toThrow( + /^Still used by “Garden”$/, + ); + }); + it('lists', async () => { stubFetch(JSON.stringify([{ file: FILE, width: 8, height: 4, bytes: 12, updatedAt: 'now' }]), { status: 200, diff --git a/tests/core/texture.test.ts b/tests/core/texture.test.ts index 199637a2..a2753fab 100644 --- a/tests/core/texture.test.ts +++ b/tests/core/texture.test.ts @@ -33,6 +33,15 @@ describe('texture file names', () => { expect(isTextureFile(`${'ab'.repeat(32)}.jpg`)).toBe(true); }); + it('takes the folder of the account that uploaded it (#344)', () => { + expect(isTextureFile(`0123456789ab/${FILE}`)).toBe(true); + expect(isTextureFile(`0123456789a/${FILE}`)).toBe(false); + expect(isTextureFile(`0123456789AB/${FILE}`)).toBe(false); + expect(isTextureFile(`0123456789ab/0123456789ab/${FILE}`)).toBe(false); + expect(isTextureFile(`0123456789ab/../${FILE}`)).toBe(false); + expect(isTextureFile('0123456789ab/')).toBe(false); + }); + it('rejects anything a request must not be built from', () => { expect(isTextureFile('')).toBe(false); expect(isTextureFile('bricks.png')).toBe(false); diff --git a/tests/php/files.test.php b/tests/php/files.test.php new file mode 100644 index 00000000..bab02496 --- /dev/null +++ b/tests/php/files.test.php @@ -0,0 +1,148 @@ +put('ann', 'password1'); +$users->put('bob', 'password2'); +$sounds = new SoundStore("{$data}/sounds"); + +check('an account has no folder before its first upload', $users->folderOf('ann') === ''); +$ann = $users->folderOf('ann', true); +check('the first upload gives it one', preg_match('/^' . UserStore::FOLDER_PATTERN . '$/', $ann) === 1, $ann); +check('and keeps it', $users->folderOf('ann', true) === $ann && $users->folderOf('ann') === $ann); +$users->put('ann', 'another password'); +check('a new password keeps the folder', $users->folderOf('ann') === $ann); +$users->setRole('ann', 'admin'); +check('so does a new role', $users->folderOf('ann') === $ann); +$users->rename('ann', 'anna'); +check('and a new name', $users->folderOf('anna') === $ann); +$bob = $users->folderOf('bob', true); +check('another account gets another folder', $bob !== $ann); +check('the folders say whose they are', $users->folderOwners() === [$ann => 'anna', $bob => 'bob'], $users->folderOwners()); + +$mine = $sounds->store(wav('a'), 'jump.wav', $ann); +check('an upload lands in the folder and says so', $mine['file'] === $ann . '/' . hash('sha256', wav('a')) . '.wav', $mine); +check('the file is there', is_file("{$data}/sounds/{$mine['file']}")); +check('its name lies beside it', $mine['name'] === 'jump.wav'); +check('the same bytes again are the same file', $sounds->store(wav('a'), 'jump.wav', $ann)['file'] === $mine['file']); +$theirs = $sounds->store(wav('a'), 'hop.wav', $bob); +check('the same bytes from another account are a file of its own', $theirs['file'] === $bob . '/' . basename($mine['file']), $theirs); +check('and do not rename the first', $sounds->list($ann, false, [])[0]['name'] === 'jump.wav'); +// What was uploaded before #344 lies in the kind's own directory and belongs to nobody. +$loose = hash('sha256', wav('old')) . '.wav'; +file_put_contents("{$data}/sounds/{$loose}", wav('old')); + +check('a name with a folder is valid', SoundStore::isValidName($mine['file'])); +check('one without is too', SoundStore::isValidName($loose)); +foreach (["../{$loose}", "{$ann}/../{$loose}", "x/{$loose}", "{$ann}/{$ann}/{$loose}", "{$ann}/", strtoupper($ann) . "/{$loose}"] as $bad) { + check("{$bad} is not a stored name", !SoundStore::isValidName($bad)); +} +check('the folder of a name', HashedFileStore::folderOf($mine['file']) === $ann && HashedFileStore::folderOf($loose) === ''); + +$owners = $users->folderOwners(); +$files = static fn(array $list): array => array_column($list, 'file'); +$listed = $sounds->list($ann, false, $owners); +check('an account lists its own files and nobody\'s', $files($listed) == [$mine['file'], $loose] || $files($listed) == [$loose, $mine['file']], $files($listed)); +$byFile = array_column($listed, null, 'file'); +check('its own say whose they are and may go', $byFile[$mine['file']]['owner'] === 'anna' && $byFile[$mine['file']]['deletable'] === true, $byFile[$mine['file']]); +check('nobody\'s say so and may not', $byFile[$loose]['owner'] === null && $byFile[$loose]['deletable'] === false, $byFile[$loose]); +check('an account without a folder sees nobody\'s only', $files($sounds->list('', false, $owners)) === [$loose]); +$all = array_column($sounds->list('', true, $owners), null, 'file'); +check('an administrator lists every file', count($all) === 3, array_keys($all)); +check('and may delete nobody\'s too', $all[$loose]['deletable'] === true && $all[$loose]['owner'] === null); +$users->remove('bob'); +$orphans = array_column($sounds->list('', true, $users->folderOwners()), null, 'file'); +check('a deleted account\'s files stay, with no owner name', $orphans[$theirs['file']]['owner'] === '', $orphans[$theirs['file']]); + +// Which projects use a file: in the project, a snapshot or a release. +$project = static fn(string $id, string $owner, string $name, array $assets): array => [ + 'id' => $id, 'name' => $name, 'owner' => $owner, 'revision' => 1, 'project' => ['assets' => $assets], +]; +$write = static function (string $path, array $doc): void { + @mkdir(dirname($path), 0775, true); + file_put_contents($path, json_encode($doc, JSON_UNESCAPED_SLASHES)); +}; +$usage = new FileUsage($data); +check('a file no project names is not used', $usage->of($mine['file']) === []); +$write("{$data}/projects/p1.json", $project('p1', 'anna', 'Garden', [['type' => 'sound', 'file' => $mine['file']]])); +check('a project naming it uses it', $usage->of($mine['file']) === [['id' => 'p1', 'owner' => 'anna', 'name' => 'Garden']], $usage->of($mine['file'])); +check('the same bytes in another folder are another file', $usage->of($theirs['file']) === []); +check('a name without the folder is another file', $usage->of($loose) === []); +$write("{$data}/projects/p2.json", $project('p2', 'bob', 'Cave', [['type' => 'sound', 'file' => 'x']])); +$write("{$data}/snapshots/p2/s1.json", ['id' => 's1', 'project' => ['assets' => [['file' => $theirs['file']]]]]); +check('a snapshot counts for its project', array_column($usage->of($theirs['file']), 'id') === ['p2'], $usage->of($theirs['file'])); +$write("{$data}/releases/" . str_repeat('a', 32) . '.json', ['projectId' => 'p3', 'project' => ['nested' => [[$loose]]]]); +$write("{$data}/projects/p3.json", $project('p3', 'anna', 'Garden', [])); +check('so does a release, wherever in it the name is', array_column($usage->of($loose), 'id') === ['p3'], $usage->of($loose)); +check('a name that is only part of a string is no use', !FileUsage::uses(['x' => "{$loose} "], $loose)); +// Slashes escaped the way PHP writes them by default are found as well. +file_put_contents("{$data}/projects/p4.json", json_encode($project('p4', 'bob', 'Mine', [['file' => $mine['file']]]))); +check('a document with escaped slashes counts', count($usage->of($mine['file'])) === 2, $usage->of($mine['file'])); + +$uses = [ + ['id' => 'p1', 'owner' => 'anna', 'name' => 'Garden'], + ['id' => 'p3', 'owner' => 'anna', 'name' => 'Garden'], + ['id' => 'p4', 'owner' => 'bob', 'name' => 'Mine'], +]; +check( + 'the refusal names the asker\'s projects only', + FileUsage::refusal($uses, 'anna', false) === 'Still used by “Garden” (2 projects) and a project of another account, counting snapshots and releases', + FileUsage::refusal($uses, 'anna', false), +); +check( + 'an administrator learns whose the others are', + FileUsage::refusal($uses, 'anna', true) === 'Still used by “Garden” (2 projects) and “Mine” of bob, counting snapshots and releases', + FileUsage::refusal($uses, 'anna', true), +); +check( + 'somebody with none of them learns only how many', + FileUsage::refusal($uses, 'carl', false) === 'Still used by 3 projects of other accounts, counting snapshots and releases', + FileUsage::refusal($uses, 'carl', false), +); + +check('deleting a file removes it', $sounds->delete($mine['file']) && !is_file("{$data}/sounds/{$mine['file']}")); +check('and its name', !is_file("{$data}/sounds/{$mine['file']}.json")); +check('the same bytes in the other folder stay', $sounds->exists($theirs['file'])); +check('deleting it again says there was none', $sounds->delete($mine['file']) === false); +check('a path out of the store is no file to delete', $sounds->delete('../users.json') === false && is_file("{$data}/users.json")); + +exec('rm -rf ' . escapeshellarg($data)); + +echo $failed === 0 ? "ALL PASSED – {$passed} checks\n" : "{$failed} FAILED, {$passed} passed\n"; +exit($failed === 0 ? 0 : 1); diff --git a/tools/e2e/12-textures.mjs b/tools/e2e/12-textures.mjs index 57ba5fab..aaf3cd57 100644 --- a/tools/e2e/12-textures.mjs +++ b/tools/e2e/12-textures.mjs @@ -60,7 +60,7 @@ export async function textures(t) { const texture = imported[0] ?? {}; check( 'the file is named after its content hash', - /^[a-f0-9]{64}\.png$/.test(texture.file ?? ''), + /^[a-f0-9]{12}\/[a-f0-9]{64}\.png$/.test(texture.file ?? ''), String(texture.file), ); check( diff --git a/tools/e2e/30-font.mjs b/tools/e2e/30-font.mjs index d409a1d8..0e248af4 100644 --- a/tools/e2e/30-font.mjs +++ b/tools/e2e/30-font.mjs @@ -67,7 +67,7 @@ export async function font(t) { const mono = stored.find((item) => item.bytes === MONO_BYTES); check( 'the server holds the two seeded typefaces, named by their content', - Boolean(serif && mono) && /^[a-f0-9]{64}\.ttf$/.test(serif.file), + Boolean(serif && mono) && /^[a-f0-9]{12}\/[a-f0-9]{64}\.ttf$/.test(serif.file), JSON.stringify({ serif: serif?.file, mono: mono?.file, count: stored.length }), ); if (!serif || !mono) return; diff --git a/tools/e2e/34-sound.mjs b/tools/e2e/34-sound.mjs index ae221b30..5e7605ee 100644 --- a/tools/e2e/34-sound.mjs +++ b/tools/e2e/34-sound.mjs @@ -56,7 +56,7 @@ export async function sound(t) { const drone = stored.find((item) => item.bytes === DRONE_BYTES); check( 'the server holds the two seeded sounds, named by their content', - Boolean(blip && drone) && /^[a-f0-9]{64}\.wav$/.test(blip.file), + Boolean(blip && drone) && /^[a-f0-9]{12}\/[a-f0-9]{64}\.wav$/.test(blip.file), JSON.stringify({ blip: blip?.file, drone: drone?.file, count: stored.length }), ); if (!blip || !drone) return; @@ -65,6 +65,17 @@ export async function sound(t) { blip.name === 'blip.wav' && drone.name === 'drone.wav', JSON.stringify({ blip: blip.name, drone: drone.name }), ); + // A file of this run's own that no project uses, for the picker to delete (#344). Random bytes + // behind a WAV header: the store reads the signature only, and no earlier run has the same. + const spare = await evaluate(`(async () => { + const bytes = crypto.getRandomValues(new Uint8Array(48)); + bytes.set([...'RIFF'].map((c) => c.charCodeAt(0)), 0); + bytes.set([...'WAVE'].map((c) => c.charCodeAt(0)), 8); + const r = await fetch('/game-maker/api/index.php?r=sounds&name=spare.wav', { + method: 'POST', headers: { 'Content-Type': 'audio/wav' }, body: bytes, + }); + return (await r.json()).file; + })()`); // 34b. "+ → Sound" offers them; a sound is picked rather than created empty, like a typeface. await clickText('+'); @@ -83,6 +94,35 @@ export async function sound(t) { ); await shot('34-sound-picker'); + // 34b'. A file of one's own that no project uses goes from the picker (#344): the first press of + // Delete only asks, the second deletes it on the server. + const spareRow = await json(`__gm.soundPickerRow(${JSON.stringify(spare)})`); + await click(spareRow.x, spareRow.y); + const marked = await json('__gm.soundPicker()'); + check( + 'a file of one’s own says whose it is and may be deleted', + marked.selected === spare && marked.detail.includes('owner e2e') && marked.deleteButton.enabled, + JSON.stringify({ selected: marked.selected, spare, detail: marked.detail, button: marked.deleteButton }), + ); + await click(marked.deleteButton.x, marked.deleteButton.y); + const armed = await json('__gm.soundPicker()'); + await click(armed.deleteButton.x, armed.deleteButton.y); + const emptied = await waitFor( + () => json('__gm.soundPicker()'), + (p) => !p.files.includes(spare), + ); + const served = await evaluate(`fetch('/game-maker/data/sounds/${spare}').then((r) => r.status)`); + check( + 'the first press asks, the second deletes the file on the server', + armed.deleteButton.shown === 'Delete for good' && + armed.files.includes(spare) && + !emptied.files.includes(spare) && + emptied.files.includes(blip.file) && + emptied.detail === 'Deleted spare.wav.' && + served === 404, + JSON.stringify({ armed: armed.deleteButton, left: emptied.files.length, detail: emptied.detail, served }), + ); + // 34c. The ▶ at a row's left is a preview: it plays that file without choosing it. const playAt = await json(`__gm.soundPickerPlay(${JSON.stringify(drone.file)})`); await click(playAt.x, playAt.y); diff --git a/tools/e2e/65-song.mjs b/tools/e2e/65-song.mjs index 67158fa5..b550f8a6 100644 --- a/tools/e2e/65-song.mjs +++ b/tools/e2e/65-song.mjs @@ -653,7 +653,7 @@ export async function song(t) { takeItem.tick === BAR && takeAsset?.type === 'sound' && takeAsset.name === `${songName} – Take 1` && - /^[a-f0-9]{64}\.wav$/.test(takeAsset.file) && + /^[a-f0-9]{12}\/[a-f0-9]{64}\.wav$/.test(takeAsset.file) && takeSeconds > 1 && Math.abs(takeItem.length - Math.floor(takeSeconds * 960)) <= 1 && !(await json('__gm.song.recording()')) && diff --git a/tools/migrate-file-owners.php b/tools/migrate-file-owners.php new file mode 100644 index 00000000..3a8ab720 --- /dev/null +++ b/tools/migrate-file-owners.php @@ -0,0 +1,201 @@ + TextureStore::class, 'fonts' => FontStore::class, 'sounds' => SoundStore::class]; + +/** @var array $loose file without a folder => its kind */ +$loose = []; +foreach ($kinds as $kind => $class) { + foreach (glob("{$data}/{$kind}/*.*") ?: [] as $path) { + if ($class::isValidName(basename($path))) { + $loose[basename($path)] = $kind; + } + } +} + +/** Every string in `$value` that is a file without a folder. @return list */ +function looseIn(mixed $value, array $loose): array +{ + if (is_string($value)) { + return isset($loose[$value]) ? [$value] : []; + } + $found = []; + if (is_array($value)) { + foreach ($value as $item) { + array_push($found, ...looseIn($item, $loose)); + } + } + return $found; +} + +/** Swaps every string in `$value` that `$map` knows. */ +function rewrite(mixed $value, array $map): mixed +{ + if (is_string($value)) { + return $map[$value] ?? $value; + } + if (is_array($value)) { + foreach ($value as $key => $item) { + $value[$key] = rewrite($item, $map); + } + } + return $value; +} + +function readJson(string $path): ?array +{ + $raw = file_get_contents($path); + $doc = $raw === false ? null : json_decode($raw, true); + return is_array($doc) ? $doc : null; +} + +// The documents, each with the account it belongs to: a snapshot and a release are their project's. +$owners = []; +$docs = []; +foreach (glob("{$data}/projects/*.json") ?: [] as $path) { + $doc = readJson($path); + if ($doc === null) { + continue; + } + $owners[basename($path, '.json')] = is_string($doc['owner'] ?? null) ? $doc['owner'] : ''; + $docs[] = ['path' => $path, 'kind' => 'project', 'owner' => $owners[basename($path, '.json')]]; +} +foreach (glob("{$data}/snapshots/*/*.json") ?: [] as $path) { + $docs[] = ['path' => $path, 'kind' => 'snapshot', 'owner' => $owners[basename(dirname($path))] ?? '']; +} +foreach (glob("{$data}/releases/*.json") ?: [] as $path) { + $projectId = readJson($path)['projectId'] ?? ''; + $docs[] = ['path' => $path, 'kind' => 'release', 'owner' => is_string($projectId) ? ($owners[$projectId] ?? '') : '']; +} + +/** @var array> $usedBy file => accounts using it */ +$usedBy = []; +/** @var array $stuck files a document names that cannot be rewritten */ +$stuck = []; +foreach ($docs as $i => $doc) { + $files = array_unique(looseIn(readJson($doc['path'])['project'] ?? null, $loose)); + $docs[$i]['files'] = $files; + $account = $doc['owner'] !== '' && $users->exists($doc['owner']); + foreach ($files as $file) { + if ($account) { + $usedBy[$file][$doc['owner']] = true; + } else { + $stuck[$file] = true; + echo "kept: {$file} is named by {$doc['kind']} {$doc['path']} without an account\n"; + } + } +} + +// Copies into each user's folder, and what their documents' names become. +$maps = []; +$copies = 0; +foreach ($usedBy as $file => $accounts) { + $kind = $loose[$file]; + foreach (array_keys($accounts) as $account) { + $folder = $dry ? ($users->folderOf($account) ?: 'new-folder') : $users->folderOf($account, true); + $maps[$account][$file] = "{$folder}/{$file}"; + $copies++; + echo "copy: {$kind}/{$file} → {$kind}/{$folder}/ ({$account})\n"; + if ($dry) { + continue; + } + $dir = "{$data}/{$kind}/{$folder}"; + if (!is_dir($dir) && !mkdir($dir, 0775, true) && !is_dir($dir)) { + throw new RuntimeException("Cannot create {$dir}"); + } + if (!is_file("{$dir}/{$file}")) { + AtomicFile::write("{$dir}/{$file}", (string) file_get_contents("{$data}/{$kind}/{$file}"), AtomicFile::SERVED, 'stored file'); + } + OriginalName::write($dir, $file, OriginalName::read("{$data}/{$kind}", $file)); + } +} + +$rewritten = ['project' => 0, 'snapshot' => 0, 'release' => 0]; +foreach ($docs as $doc) { + $map = $maps[$doc['owner']] ?? []; + if ($doc['files'] === [] || $map === []) { + continue; + } + $rewritten[$doc['kind']]++; + if ($dry) { + continue; + } + $write = static function () use ($doc, $map): void { + $current = readJson($doc['path']); + if ($current === null) { + return; + } + $current['project'] = rewrite($current['project'], $map); + $flags = JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE; + if ($doc['kind'] === 'project') { + $current['revision'] = (int) $current['revision'] + 1; + $current['updatedAt'] = gmdate('c'); + } + if ($doc['kind'] !== 'release') { + $flags |= JSON_PRETTY_PRINT; + } + AtomicFile::write($doc['path'], (string) json_encode($current, $flags), AtomicFile::HIDDEN, "{$doc['kind']} file"); + }; + if ($doc['kind'] !== 'project') { + $write(); + continue; + } + // Under the save lock, as `Store` takes it: a save running at the same time waits, then fails + // on the revision instead of writing the old names back. + $lock = fopen($doc['path'] . '.lock', 'c'); + if ($lock === false || !flock($lock, LOCK_EX)) { + throw new RuntimeException("Cannot lock {$doc['path']}"); + } + try { + $write(); + } finally { + flock($lock, LOCK_UN); + fclose($lock); + } +} + +$moved = 0; +foreach (array_keys($usedBy) as $file) { + if (isset($stuck[$file])) { + continue; + } + $moved++; + if (!$dry) { + $dir = "{$data}/{$loose[$file]}"; + unlink("{$dir}/{$file}"); + OriginalName::delete($dir, $file); + } +} + +$unused = count($loose) - count($usedBy) - count(array_diff_key($stuck, $usedBy)); +echo ($dry ? '[dry run] ' : ''), + 'files used: ', count($usedBy) + count(array_diff_key($stuck, $usedBy)), + ", copies made: {$copies}, moved away: {$moved}, kept for documents without an account: ", count($stuck), + ", unused and nobody's: {$unused}\n", + "rewritten: {$rewritten['project']} projects, {$rewritten['snapshot']} snapshots, {$rewritten['release']} releases\n";