From e3448144ae73b392954087124b5258b5462093a9 Mon Sep 17 00:00:00 2001 From: Austin Sullivan Date: Sun, 16 Jul 2023 21:13:27 -0400 Subject: [PATCH 1/7] revamp move --- index.bs | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 201 insertions(+) diff --git a/index.bs b/index.bs index 85afe86..acf576c 100644 --- a/index.bs +++ b/index.bs @@ -332,6 +332,9 @@ interface FileSystemHandle { readonly attribute USVString name; Promise isSameEntry(FileSystemHandle other); + Promise move(USVString newEntryName); + Promise move(FileSystemDirectoryHandle destinationDirectory); + Promise move(FileSystemDirectoryHandle destinationDirectory, USVString newEntryName); }; @@ -416,6 +419,204 @@ The isSameEntry(|other|) method steps are +### The {{FileSystemHandle/move(destinationDirectory, newEntryName)|move()}} method ### {#api-filesystemhandle-move} + +
+ : await |handle| . {{FileSystemHandle/move(newEntryName)|move}}({ {{USVString}}: |newEntryName|}) + :: Renames the [=file system entry=] [=locate an entry|locatable=] by + |handle|'s [=FileSystemHandle/locator=] to |newEntryName|. + + : await |handle| . {{FileSystemHandle/move(destinationDirectory)|move}}({ {{FileSystemDirectoryHandle}}: |destinationDirectory|}) + :: Moves the [=file system entry=] [=locate an entry|locatable=] by + |handle|'s [=FileSystemHandle/locator=] to the [=directory entry=] + [=locate an entry|locatable=] by |destinationDirectory|'s + [=FileSystemHandle/locator=], while keeping its existing name. + + : await |handle| . {{FileSystemHandle/move(destinationDirectory, newEntryName)|move}}({ {{FileSystemDirectoryHandle}}: |destinationDirectory|, {{USVString}}: |newEntryName|}) + :: Moves the [=file system entry=] [=locate an entry|locatable=] by + |handle|'s [=FileSystemHandle/locator=] to the [=directory entry=] + [=locate an entry|locatable=] by |destinationDirectory|'s + [=FileSystemHandle/locator=], as well as renaming to |newEntryName|. + + If the [=file system locator/root|roots=] of the respective + [=FileSystemHandle/locator|locators=] of |destinationDirectory| and + |handle| are not the same, this operation may abort or fail non-atomically. +
+ +
+The move({{FileSystemDirectoryHandle}}: |destinationDirectory|) +method steps are to [=FileSystemHandle/move=] |handle| +given |handle|'s {{FileSystemHandle/name}} and |destinationDirectory|. + +
+ +
+The move({{USVString}}: |newEntryName|) +method steps are to [=FileSystemHandle/move=] |handle| +given |newEntryName|. + +
+ +
+The move({{FileSystemDirectoryHandle}}: |destinationDirectory|, {{USVString}}: |newEntryName|) +method steps are to [=FileSystemHandle/move=] |handle| +given |newEntryName| and |destinationDirectory|. + +
+ +
+ +To move a {{FileSystemHandle}} |handle| given +a {{USVString}} |newEntryName| and +an optional {{FileSystemDirectoryHandle}} |destinationDirectory|: + +1. Let |result| be [=a new promise=]. +1. Let |locator| be |handle|'s [=FileSystemHandle/locator=]. +1. Let |global| be |handle|'s [=relevant global object=]. +1. Let |isInABucketFileSystem| be true if + |handle| [=FileSystemHandle/is in a bucket file system=]; + otherwise false. +1. [=Enqueue the following steps=] to the [=file system queue=]: + 1. If |newEntryName| is not a [=valid file name=], + [=queue a storage task=] with |global| to [=/reject=] |result| + with a {{TypeError}} and abort these steps. + + 1. Let |entry| be the result of [=locating an entry=] given |locator|. + 1. Let |accessResult| be the result of running |entry|'s + [=file system entry/request access=] given "`readwrite`". + 1. If |accessResult|'s [=file system access result/permission state=] + is not "{{PermissionState/granted}}", [=queue a storage task=] with + |global| to [=/reject=] |result| with a {{DOMException}} of + |accessResult|'s [=file system access result/error name=] and + abort these steps. + + 1. If |destinationDirectory| was given: + 1. Let |destinationDirectoryLocator| be + |destinationDirectory|'s [=FileSystemHandle/locator=]. + 1. Let |destinationDirectoryEntry| be the result of [=locating an entry=] + given |destinationDirectoryLocator|. + + 1. Let |destinationDirectoryAccessResult| be the result of running + |destinationDirectoryEntry|'s + [=file system entry/request access=] given "`readwrite`". + 1. If |destinationDirectoryAccessResult|'s + [=file system access result/permission state=] + is not "{{PermissionState/granted}}", [=queue a storage task=] with + |global| to [=/reject=] |result| with a {{DOMException}} of + |accessResult|'s [=file system access result/error name=] and + abort these steps. + + 1. If |destinationDirectoryLocator|'s [=file system locator/root=] is not + |locator|'s [=file system locator/root=], [=queue a storage task=] with + |global| to [=/reject=] |result| with an + "{{NotSupportedError}}" {{DOMException}} and abort these steps. + + Issue(114): Decide which moves across file systems, if any, should be + supported, or if this should be left up to individual user-agent + implementations. + + 1. Let |destinationPath| be the result of [=list/clone|cloning=] + |destinationDirectoryLocator|'s [=file system locator/path=] and + [=list/append|appending=] |newEntryName|. + 1. Otherwise: + 1. Let |destinationPath| be the result of [=list/clone|cloning=] + |locator|'s [=file system locator/path=], [=list/remove|removing=] + the last [=list/item=], and [=list/append|appending=] |newEntryName|. + + 1. Let |destinationLocator| be a [=/file system locator=] whose + [=file system locator/kind=] is |locator|'s [=file system locator/kind=], + [=file system locator/root=] is |locator|'s [=file system locator/root=], and + [=file system locator/path=] is |destinationPath|. + 1. Let |destinationEntry| be the result of [=locating an entry=] + given |destinationLocator|. + + 1. If |destinationDirectory| was not given: + 1. Let |destinationAccessResult| be the result of running + |destinationEntry|'s [=file system entry/request access=] + given "`readwrite`". + + Issue(101): Make sure this still makes sense once access check algorithms + are no longer associated with an entry. + 1. Otherwise: + 1. Let |destinationAccessResult| be |destinationDirectoryAccessResult|. + + 1. Let |isTheRootOfABucketFileSystem| be true if + |isInABucketFileSystem| is true and + |entry|'s [=file system entry/parent=] is `null`; + otherwise false. + 1. If |isTheRootOfABucketFileSystem| is true, + [=queue a storage task=] with |global| to [=/reject=] |result| with a + "{{InvalidModificationError}}" {{DOMException}} and abort these steps. + + Issue: Do not allow |entry| to be moved if it is a + sensitive directory + + 1. If |destinationAccessResult|'s + [=file system access result/permission state=] is not + "{{PermissionState/granted}}": + 1. If |destinationEntry| is not `null`, [=queue a storage task=] with + |global| to [=/reject=] |result| with a {{DOMException}} of + |destinationAccessResult|'s [=file system access result/error name=] and + abort these steps + + Note: This prevents overwriting an existing file or directory which the + site does not have access to. + + 1. Let |settingsGlobal| be |handle|'s [=relevant settings object=]'s + [=environment settings object/global object=]. + 1. If |settingsGlobal| does not have [=transient activation=], + [=queue a storage task=] with |global| to [=/reject=] |result| + with a "{{NotAllowedError}}" {{DOMException}}. + + 1. If |entry| is `null`, [=queue a storage task=] with |global| to [=/reject=] + |result| with a "{{NotFoundError}}" {{DOMException}} and abort these steps. + 1. If |destinationDirectory| was given and |destinationDirectoryEntry| + is `null`, [=queue a storage task=] with |global| to [=/reject=] |result| + with a "{{NotFoundError}}" {{DOMException}} and abort these steps. + + 1. Let |lockResult| be the result of [=file entry/lock/take|taking a lock=] + with "`exclusive`" on |entry|. + + Issue(137): Support locking directory entries. + + 1. If |destinationEntry| is not `null`: + 1. Let |destinationLockResult| be the result of + [=file entry/lock/take|taking a lock=] with "`exclusive`" + on |destinationEntry|. + 1. Otherwise: + 1. Let |destinationLockResult| be "`not taken`". + + Issue: Consider whether it should be possible to lock a + [=/file system entry=] which does not (yet) exist. + + 1. [=Queue a storage task=] with |global| to run these steps: + 1. If |lockResult| is "`failure`", [=/reject=] |result| with a + "{{NoModificationAllowedError}}" {{DOMException}} and abort these steps. + 1. If |destinationLockResult| is "`failure`", [=/reject=] |result| with a + "{{NoModificationAllowedError}}" {{DOMException}} and abort these steps. + + 1. If |destinationDirectory| was given: + 1. If |entry|'s [=file system entry/parent=] is not `null`, + [=set/remove=] |entry| from |entry|'s [=file system entry/parent=]'s + [=directory entry/children=]. + 1. [=set/Append=] |entry| to |destinationDirectoryEntry|'s + [=directory entry/children=]. + + 1. If |entry| is a [=file entry=] and |isInABucketFileSystem| is false, + run [=implementation-defined=] malware scans and safe browsing checks. + If these checks fail, [=/reject=] |result| with an + "{{AbortError}}" {{DOMException}} and abort these steps. + + 1. Attempt to move |entry| to |destinationEntry| in the underlying file + system. If that throws an exception, [=/reject=] |result| with that + exception and abort these steps. + + 1. [=/Resolve=] |result| with `undefined`. + +1. Return |result|. + +
+ ## The {{FileSystemFileHandle}} interface ## {#api-filesystemfilehandle} From 5794159c6b1c6deae601fe716063e0c6b4227a73 Mon Sep 17 00:00:00 2001 From: Austin Sullivan <asully@chromium.org> Date: Sun, 16 Jul 2023 21:19:07 -0400 Subject: [PATCH 2/7] release lock after move --- index.bs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/index.bs b/index.bs index acf576c..3dcc185 100644 --- a/index.bs +++ b/index.bs @@ -611,7 +611,10 @@ an optional {{FileSystemDirectoryHandle}} |destinationDirectory|: system. If that throws an exception, [=/reject=] |result| with that exception and abort these steps. - 1. [=/Resolve=] |result| with `undefined`. + 1. [=Enqueue the following steps=] to the [=file system queue=]: + 1. [=file entry/lock/release|Release the lock=] on |entry|. + 1. [=Queue a storage task=] with |global| to + [=/resolve=] |result| with `undefined`. 1. Return |result|. From e42817f356b32ad0a54ccc6feda05818a99d8db5 Mon Sep 17 00:00:00 2001 From: Austin Sullivan <asully@chromium.org> Date: Sun, 16 Jul 2023 21:25:11 -0400 Subject: [PATCH 3/7] release lock on destination, too --- index.bs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/index.bs b/index.bs index 3dcc185..168ec2c 100644 --- a/index.bs +++ b/index.bs @@ -611,8 +611,12 @@ an optional {{FileSystemDirectoryHandle}} |destinationDirectory|: system. If that throws an exception, [=/reject=] |result| with that exception and abort these steps. + Note: In some cases, moving a file or directory can fail non-atomically. + 1. [=Enqueue the following steps=] to the [=file system queue=]: 1. [=file entry/lock/release|Release the lock=] on |entry|. + 1. If |destinationLockResult| is "`success`", + [=file entry/lock/release|release the lock=] on |destinationEntry|. 1. [=Queue a storage task=] with |global| to [=/resolve=] |result| with `undefined`. From 9dcfea02319365d19feaaa6d3f450d6c079785db Mon Sep 17 00:00:00 2001 From: Austin Sullivan <asully@chromium.org> Date: Sun, 16 Jul 2023 22:30:19 -0400 Subject: [PATCH 4/7] expand notes + set locator to dest --- index.bs | 51 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/index.bs b/index.bs index 168ec2c..2bb1c5a 100644 --- a/index.bs +++ b/index.bs @@ -549,7 +549,7 @@ an optional {{FileSystemDirectoryHandle}} |destinationDirectory|: "{{InvalidModificationError}}" {{DOMException}} and abort these steps. Issue: Do not allow |entry| to be moved if it is a - <a href=https://wicg.github.io/file-system-access/#wellknowndirectory-too-sensitive-or-dangerous>sensitive directory</a> + <a href=https://wicg.github.io/file-system-access/#enumdef-wellknowndirectory>well-known directory</a>. 1. If |destinationAccessResult|'s [=file system access result/permission state=] is not @@ -560,7 +560,12 @@ an optional {{FileSystemDirectoryHandle}} |destinationDirectory|: abort these steps Note: This prevents overwriting an existing file or directory which the - site does not have access to. + website does not have access to. To prevent overwriting a file which + the site can access, use of the + <a href=https://www.w3.org/TR/web-locks/>Web Locks API</a> is encouraged. + + Issue(46): Make it easier to generate a uniquely-identifying string for + a {{FileSystemHandle}}. 1. Let |settingsGlobal| be |handle|'s [=relevant settings object=]'s [=environment settings object/global object=]. @@ -595,23 +600,51 @@ an optional {{FileSystemDirectoryHandle}} |destinationDirectory|: 1. If |destinationLockResult| is "`failure`", [=/reject=] |result| with a "{{NoModificationAllowedError}}" {{DOMException}} and abort these steps. - 1. If |destinationDirectory| was given: - 1. If |entry|'s [=file system entry/parent=] is not `null`, - [=set/remove=] |entry| from |entry|'s [=file system entry/parent=]'s - [=directory entry/children=]. - 1. [=set/Append=] |entry| to |destinationDirectoryEntry|'s - [=directory entry/children=]. - 1. If |entry| is a [=file entry=] and |isInABucketFileSystem| is false, run [=implementation-defined=] malware scans and safe browsing checks. If these checks fail, [=/reject=] |result| with an "{{AbortError}}" {{DOMException}} and abort these steps. + 1. Let |sourceQueryAccess| be |entry|'s [=file system entry/query access=]. + 1. Let |sourceRequestAccess| be |entry|'s [=file system entry/request access=]. + + 1. If |destinationDirectory| was given: + 1. [=set/Append=] |entry| to |destinationDirectoryEntry|'s + [=directory entry/children=]. + 1. Attempt to move |entry| to |destinationEntry| in the underlying file system. If that throws an exception, [=/reject=] |result| with that exception and abort these steps. Note: In some cases, moving a file or directory can fail non-atomically. + A file move may result in a partially-written |destinationEntry|. + A directory move may result in only some files or sub-folders being + copied to |destinationEntry|, and some containing files may themselves + fail to be moved atomically. + In both cases, |entry| will only be removed from the underlying file + system if all contents were successfully moved to |destinationEntry|. + + 1. If |destinationDirectory| was given: + 1. If |entry|'s [=file system entry/parent=] is not `null`, + [=set/remove=] |entry| from |entry|'s [=file system entry/parent=]'s + [=directory entry/children=]. + + 1. Set |handle|'s [=FileSystemHandle/locator=] to |destinationLocator|. + + Note: This does not update other {{FileSystemHandle}}s with the same + [=FileSystemHandle/locator=], nor does it update any {{FileSystemHandle}} + which [=locate an entry|located to=] any of |handle|'s + [=directory entry/children=] if |handle| is a [=directory entry=]. + Each of these handles will presumably no longer [=locate an entry=], + unless a new entry is created at the respective location. + + 1. Set |destinationEntry|'s [=file system entry/query access=] to + |sourceQueryAccess|. + 1. Set |destinationEntry|'s [=file system entry/request access=] to + |sourceRequestAccess|. + + Issue(101): Make sure this still makes sense once access check + algorithms are no longer associated with an entry. 1. [=Enqueue the following steps=] to the [=file system queue=]: 1. [=file entry/lock/release|Release the lock=] on |entry|. From a96ae77d9deab380a9825c169f3950a2dc9ded77 Mon Sep 17 00:00:00 2001 From: Austin Sullivan <asully@chromium.org> Date: Sun, 16 Jul 2023 22:55:51 -0400 Subject: [PATCH 5/7] handle self-move --- index.bs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/index.bs b/index.bs index 2bb1c5a..1f3cbba 100644 --- a/index.bs +++ b/index.bs @@ -527,10 +527,18 @@ an optional {{FileSystemDirectoryHandle}} |destinationDirectory|: [=file system locator/kind=] is |locator|'s [=file system locator/kind=], [=file system locator/root=] is |locator|'s [=file system locator/root=], and [=file system locator/path=] is |destinationPath|. + 1. If the result of [=file system locator/resolving=] |destinationLocator| + relative to |locator| is not null and is not [=list/is empty|empty=], + [=queue a storage task=] with |global| to [=/reject=] |result| with a + "{{InvalidModificationError}}" {{DOMException}} and abort these steps. + + Note: This prevents moving a directory within itself. + 1. Let |destinationEntry| be the result of [=locating an entry=] given |destinationLocator|. - 1. If |destinationDirectory| was not given: + 1. If |destinationDirectory| was not given and + |destinationLocator| is not [=the same entry as=] |locator|: 1. Let |destinationAccessResult| be the result of running |destinationEntry|'s [=file system entry/request access=] given "`readwrite`". @@ -600,6 +608,12 @@ an optional {{FileSystemDirectoryHandle}} |destinationDirectory|: 1. If |destinationLockResult| is "`failure`", [=/reject=] |result| with a "{{NoModificationAllowedError}}" {{DOMException}} and abort these steps. + 1. If |destinationLocator| is [=the same entry as=] |locator|, + [=enqueue the following steps=] to the [=file system queue=]: + 1. [=file entry/lock/release|Release the lock=] on |entry|. + 1. [=Queue a storage task=] with |global| to + [=/resolve=] |result| with `undefined`. + 1. If |entry| is a [=file entry=] and |isInABucketFileSystem| is false, run [=implementation-defined=] malware scans and safe browsing checks. If these checks fail, [=/reject=] |result| with an From 5713e8bf29b8b3e96b9cfd51867ca445f82925eb Mon Sep 17 00:00:00 2001 From: Austin Sullivan <asully@chromium.org> Date: Sun, 16 Jul 2023 23:20:22 -0400 Subject: [PATCH 6/7] exit early during self-move --- index.bs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.bs b/index.bs index 1f3cbba..613860e 100644 --- a/index.bs +++ b/index.bs @@ -612,7 +612,7 @@ an optional {{FileSystemDirectoryHandle}} |destinationDirectory|: [=enqueue the following steps=] to the [=file system queue=]: 1. [=file entry/lock/release|Release the lock=] on |entry|. 1. [=Queue a storage task=] with |global| to - [=/resolve=] |result| with `undefined`. + [=/resolve=] |result| with `undefined` and abort these steps. 1. If |entry| is a [=file entry=] and |isInABucketFileSystem| is false, run [=implementation-defined=] malware scans and safe browsing checks. From f04431393e38b60e0e8420bffca23a9c58ae6d48 Mon Sep 17 00:00:00 2001 From: Austin Sullivan <asully@chromium.org> Date: Mon, 17 Jul 2023 23:40:01 -0400 Subject: [PATCH 7/7] fix bs error --- index.bs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/index.bs b/index.bs index 613860e..fc02a6c 100644 --- a/index.bs +++ b/index.bs @@ -445,21 +445,21 @@ The <dfn method for=FileSystemHandle>isSameEntry(|other|)</dfn> method steps are <div algorithm> The <dfn method for=FileSystemHandle>move({{FileSystemDirectoryHandle}}: |destinationDirectory|)</dfn> -method steps are to [=FileSystemHandle/move=] |handle| -given |handle|'s {{FileSystemHandle/name}} and |destinationDirectory|. +method steps are to [=FileSystemHandle/move=] [=this=] +given [=this=]'s {{FileSystemHandle/name}} and |destinationDirectory|. </div> <div algorithm> The <dfn method for=FileSystemHandle>move({{USVString}}: |newEntryName|)</dfn> -method steps are to [=FileSystemHandle/move=] |handle| +method steps are to [=FileSystemHandle/move=] [=this=] given |newEntryName|. </div> <div algorithm> The <dfn method for=FileSystemHandle>move({{FileSystemDirectoryHandle}}: |destinationDirectory|, {{USVString}}: |newEntryName|)</dfn> -method steps are to [=FileSystemHandle/move=] |handle| +method steps are to [=FileSystemHandle/move=] [=this=] given |newEntryName| and |destinationDirectory|. </div>