From 4b958f8defacc805e53a8a872ae5fb5385f56170 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Fri, 9 Sep 2022 14:18:40 -0700 Subject: [PATCH] FSA: Reject moves with empty names move("") rejects with a TypeError, while move(dir, "") succeeds (by ignoring the second arg). Make this consistent by always rejecting if there's an invalid name, as specified in https://wicg.github.io/file-system-access/#valid-file-name See https://github.com/whatwg/fs/pull/10#issuecomment-1236232639 Bug: 1327741 Change-Id: Ifd8457df05aad7f75007ff5eece6237a09098a94 --- .../FileSystemDirectoryHandle-move.js | 60 ++----------------- fs/script-tests/FileSystemFileHandle-move.js | 37 ++---------- 2 files changed, 10 insertions(+), 87 deletions(-) diff --git a/file-system-access/script-tests/FileSystemDirectoryHandle-move.js b/file-system-access/script-tests/FileSystemDirectoryHandle-move.js index 856c1f3293208e..4d1820ace4ca9d 100644 --- a/file-system-access/script-tests/FileSystemDirectoryHandle-move.js +++ b/file-system-access/script-tests/FileSystemDirectoryHandle-move.js @@ -50,15 +50,14 @@ directory_test(async (t, root) => { const dir_dest = await root.getDirectoryHandle('dir-dest', {create: true}); const dir_in_dir = await dir_src.getDirectoryHandle('dir-in-dir', {create: true}); - await dir_in_dir.move(dir_dest, ""); + await dir_in_dir.move(dir_dest, ''); + + await promise_rejects_js(t, TypeError, dir_in_dir.move(dir_dest, '')); assert_array_equals( - await getSortedDirectoryEntries(root), ['dir-dest/', 'dir-src/']); - assert_array_equals(await getSortedDirectoryEntries(dir_src), []); - assert_array_equals( - await getSortedDirectoryEntries(dir_dest), ['dir-in-dir/']); - assert_array_equals(await getSortedDirectoryEntries(dir_in_dir), []); -}, 'move(dir, "") to move an empty directory to a new directory'); + await getSortedDirectoryEntries(dir_src), ['dir-before/']); + assert_array_equals(await getSortedDirectoryEntries(dir_dest), []); +}, 'move(dir, "") to move a directory to a new directory fails'); directory_test(async (t, root) => { const dir_src = await root.getDirectoryHandle('dir-src', {create: true}); @@ -75,26 +74,6 @@ directory_test(async (t, root) => { assert_array_equals(await getSortedDirectoryEntries(dir_in_dir), []); }, 'move(dir, name) to move an empty directory to a new directory'); -directory_test(async (t, root) => { - const dir_src = await root.getDirectoryHandle('dir-src', {create: true}); - const dir_dest = await root.getDirectoryHandle('dir-dest', {create: true}); - const dir_in_dir = - await dir_src.getDirectoryHandle('dir-in-dir', {create: true}); - const file = - await createFileWithContents(t, 'file-in-dir', 'abc', dir_in_dir); - await dir_in_dir.move(dir_dest, ""); - - assert_array_equals( - await getSortedDirectoryEntries(root), ['dir-dest/', 'dir-src/']); - assert_array_equals(await getSortedDirectoryEntries(dir_src), []); - assert_array_equals( - await getSortedDirectoryEntries(dir_dest), ['dir-in-dir/']); - assert_array_equals( - await getSortedDirectoryEntries(dir_in_dir), ['file-in-dir']); - // `file` should be invalidated after moving directories. - await promise_rejects_dom(t, 'NotFoundError', getFileContents(file)); -}, 'move(dir, "") to move a non-empty directory to a new directory'); - directory_test(async (t, root) => { const dir_src = await root.getDirectoryHandle('dir-src', {create: true}); const dir_dest = await root.getDirectoryHandle('dir-dest', {create: true}); @@ -142,33 +121,6 @@ directory_test(async (t, root) => { assert_equals(await getFileContents(handle), 'foo'); }, 'move(dir) can be called multiple times'); -directory_test(async (t, root) => { - const dir1 = await root.getDirectoryHandle('dir1', {create: true}); - const dir2 = await root.getDirectoryHandle('dir2', {create: true}); - const handle = await createFileWithContents(t, 'file', 'foo', root); - - await handle.move(dir1, ""); - assert_array_equals( - await getSortedDirectoryEntries(root), ['dir1/', 'dir2/']); - assert_array_equals(await getSortedDirectoryEntries(dir1), ['file']); - assert_array_equals(await getSortedDirectoryEntries(dir2), []); - assert_equals(await getFileContents(handle), 'foo'); - - await handle.move(dir2, ""); - assert_array_equals( - await getSortedDirectoryEntries(root), ['dir1/', 'dir2/']); - assert_array_equals(await getSortedDirectoryEntries(dir1), []); - assert_array_equals(await getSortedDirectoryEntries(dir2), ['file']); - assert_equals(await getFileContents(handle), 'foo'); - - await handle.move(root, ""); - assert_array_equals( - await getSortedDirectoryEntries(root), ['dir1/', 'dir2/', 'file']); - assert_array_equals(await getSortedDirectoryEntries(dir1), []); - assert_array_equals(await getSortedDirectoryEntries(dir2), []); - assert_equals(await getFileContents(handle), 'foo'); -}, 'move(dir, "") can be called multiple times'); - directory_test(async (t, root) => { const dir1 = await root.getDirectoryHandle('dir1', {create: true}); const dir2 = await root.getDirectoryHandle('dir2', {create: true}); diff --git a/fs/script-tests/FileSystemFileHandle-move.js b/fs/script-tests/FileSystemFileHandle-move.js index a3be9f49afd85d..cba8e4608b5d67 100644 --- a/fs/script-tests/FileSystemFileHandle-move.js +++ b/fs/script-tests/FileSystemFileHandle-move.js @@ -129,15 +129,13 @@ directory_test(async (t, root) => { const dir_src = await root.getDirectoryHandle('dir-src', {create: true}); const dir_dest = await root.getDirectoryHandle('dir-dest', {create: true}); const file = await createFileWithContents(t, 'file', 'abc', dir_src); - await file.move(dir_dest, ''); + await promise_rejects_js(t, TypeError, file.move(dir_dest, '')); - assert_array_equals( - await getSortedDirectoryEntries(root), ['dir-dest/', 'dir-src/']); - assert_array_equals(await getSortedDirectoryEntries(dir_src), []); - assert_array_equals(await getSortedDirectoryEntries(dir_dest), ['file']); + assert_array_equals(await getSortedDirectoryEntries(dir_src), ['file']); assert_equals(await getFileContents(file), 'abc'); assert_equals(await getFileSize(file), 3); -}, 'move(dir, "") to move a file to a new directory'); + assert_array_equals(await getSortedDirectoryEntries(dir_dest), []); +}, 'move(dir, "") to move a file to a new directory fails'); directory_test(async (t, root) => { const dir_src = await root.getDirectoryHandle('dir-src', {create: true}); @@ -182,33 +180,6 @@ directory_test(async (t, root) => { assert_equals(await getFileContents(handle), 'foo'); }, 'move(dir) can be called multiple times'); -directory_test(async (t, root) => { - const dir1 = await root.getDirectoryHandle('dir1', {create: true}); - const dir2 = await root.getDirectoryHandle('dir2', {create: true}); - const handle = await createFileWithContents(t, 'file', 'foo', root); - - await handle.move(dir1, ""); - assert_array_equals( - await getSortedDirectoryEntries(root), ['dir1/', 'dir2/']); - assert_array_equals(await getSortedDirectoryEntries(dir1), ['file']); - assert_array_equals(await getSortedDirectoryEntries(dir2), []); - assert_equals(await getFileContents(handle), 'foo'); - - await handle.move(dir2, ""); - assert_array_equals( - await getSortedDirectoryEntries(root), ['dir1/', 'dir2/']); - assert_array_equals(await getSortedDirectoryEntries(dir1), []); - assert_array_equals(await getSortedDirectoryEntries(dir2), ['file']); - assert_equals(await getFileContents(handle), 'foo'); - - await handle.move(root, ""); - assert_array_equals( - await getSortedDirectoryEntries(root), ['dir1/', 'dir2/', 'file']); - assert_array_equals(await getSortedDirectoryEntries(dir1), []); - assert_array_equals(await getSortedDirectoryEntries(dir2), []); - assert_equals(await getFileContents(handle), 'foo'); -}, 'move(dir, "") can be called multiple times'); - directory_test(async (t, root) => { const dir1 = await root.getDirectoryHandle('dir1', {create: true}); const dir2 = await root.getDirectoryHandle('dir2', {create: true});