An overlapping move — a destination inside the source, or a source inside the destination — is rejected by every implementation, but nothing holds them to it and they do not agree on how. NodeVFS rejects with a raw Node Error rather than a VFSError, so a caller cannot classify it and a protocol server maps it to a generic failure instead of the status the same request gets over MemoryVFS.
Surfaced while reviewing #184. Not a regression from it; it predates that work.
The coverage gap
packages/vfs/src/conformance/conformance.ts pins overlapping copy in all three directions:
There is no equivalent for move, in any direction. The move contract (packages/vfs/src/index.ts:91-95) covers a missing source and replace-rather-than-merge, and says nothing about overlap, so the suite is not omitting a documented rule — the rule was never written down.
This matters more for move than the symmetry suggests. moveFallback is copy-then-delete, so an unguarded overlapping move is the #176 heap exhaustion with a delete of the source at the end.
The bug it hides
vfsErr in packages/vfs/src/node/node.ts:66-77 maps ENOENT, EEXIST, ENOTDIR, EISDIR, ENOTEMPTY, EACCES and EPERM. POSIX rename(2) returns EINVAL for a directory moved into itself, which is not in that list, so it falls through to throw e:
MemoryVFS rejects the same call with precondition-failed (its explicit pathsOverlap check, added in 6c66acb88 for #176), and copyFallback rejects with precondition-failed too. So the identical WebDAV MOVE answers 412 over one backend and a generic error over the other, because the raw Error fails the instanceof VFSError test in the server's handler.
This is the same shape as the NodeVFS inverted-range ERR_OUT_OF_RANGE that #175 turned up: an errno reaching a caller unmapped.
Requested direction
Document overlap on the move contract, matching what copy already promises — reject with precondition-failed, in both directions, without modifying either path.
Add the three conformance cases for move, mirroring the copy ones.
Map EINVAL in vfsErr, or guard overlap in NodeVFS.move explicitly before calling fsp.rename. The explicit guard is probably better: EINVAL covers more than overlap, so mapping it wholesale to precondition-failed would mislabel other failures.
Step 2 is what makes step 3 fail, so they want doing together.
Worth checking the other implementations against the new cases while there — FsaVFS, and the SFTP, FTP, SMB2 and WebDAV clients, whose answer comes from a remote server and may not be precondition-failed either.
Related
#176 — the same overlap defect on MemoryVFS.copy, closed. This is the move half.
#137 — copy/move semantics; if existing replaces overwrite, the overlap rule should be stated in the same pass.
#186 — rename will need the same rule, and inherits it if it is on the contract rather than on one method.
## Summary
An overlapping `move` — a destination inside the source, or a source inside the destination — is rejected by every implementation, but nothing holds them to it and they do not agree on how. `NodeVFS` rejects with a raw Node `Error` rather than a `VFSError`, so a caller cannot classify it and a protocol server maps it to a generic failure instead of the status the same request gets over `MemoryVFS`.
Surfaced while reviewing #184. Not a regression from it; it predates that work.
## The coverage gap
`packages/vfs/src/conformance/conformance.ts` pins overlapping `copy` in all three directions:
- `copy(tree, tree, { overwrite: true })` → `precondition-failed`
- `copy(tree, tree/inner)` → `precondition-failed`
- `copy(tree/inner, tree, { overwrite: true })` → `precondition-failed`
There is no equivalent for `move`, in any direction. The `move` contract (`packages/vfs/src/index.ts:91-95`) covers a missing source and replace-rather-than-merge, and says nothing about overlap, so the suite is not omitting a documented rule — the rule was never written down.
This matters more for `move` than the symmetry suggests. `moveFallback` is copy-then-delete, so an unguarded overlapping move is the #176 heap exhaustion with a `delete` of the source at the end.
## The bug it hides
`vfsErr` in `packages/vfs/src/node/node.ts:66-77` maps `ENOENT`, `EEXIST`, `ENOTDIR`, `EISDIR`, `ENOTEMPTY`, `EACCES` and `EPERM`. POSIX `rename(2)` returns `EINVAL` for a directory moved into itself, which is not in that list, so it falls through to `throw e`:
```
$ node -e '<NodeVFS over a temp dir>; await v.mkdir("/tree"); await v.mkdir("/tree/inner"); await v.move("/tree", "/tree/inner/deep")'
rejected. isVFSError: false | code: EINVAL | name: Error
```
`MemoryVFS` rejects the same call with `precondition-failed` (its explicit `pathsOverlap` check, added in `6c66acb88` for #176), and `copyFallback` rejects with `precondition-failed` too. So the identical WebDAV MOVE answers 412 over one backend and a generic error over the other, because the raw `Error` fails the `instanceof VFSError` test in the server's handler.
This is the same shape as the `NodeVFS` inverted-range `ERR_OUT_OF_RANGE` that #175 turned up: an errno reaching a caller unmapped.
## Requested direction
1. Document overlap on the `move` contract, matching what `copy` already promises — reject with `precondition-failed`, in both directions, without modifying either path.
2. Add the three conformance cases for `move`, mirroring the `copy` ones.
3. Map `EINVAL` in `vfsErr`, or guard overlap in `NodeVFS.move` explicitly before calling `fsp.rename`. The explicit guard is probably better: `EINVAL` covers more than overlap, so mapping it wholesale to `precondition-failed` would mislabel other failures.
Step 2 is what makes step 3 fail, so they want doing together.
Worth checking the other implementations against the new cases while there — `FsaVFS`, and the SFTP, FTP, SMB2 and WebDAV clients, whose answer comes from a remote server and may not be `precondition-failed` either.
## Related
- #176 — the same overlap defect on `MemoryVFS.copy`, closed. This is the `move` half.
- #137 — copy/move semantics; if `existing` replaces `overwrite`, the overlap rule should be stated in the same pass.
- #184 — where this was found.
- #186 — `rename` will need the same rule, and inherits it if it is on the contract rather than on one method.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
An overlapping
move— a destination inside the source, or a source inside the destination — is rejected by every implementation, but nothing holds them to it and they do not agree on how.NodeVFSrejects with a raw NodeErrorrather than aVFSError, so a caller cannot classify it and a protocol server maps it to a generic failure instead of the status the same request gets overMemoryVFS.Surfaced while reviewing #184. Not a regression from it; it predates that work.
The coverage gap
packages/vfs/src/conformance/conformance.tspins overlappingcopyin all three directions:copy(tree, tree, { overwrite: true })→precondition-failedcopy(tree, tree/inner)→precondition-failedcopy(tree/inner, tree, { overwrite: true })→precondition-failedThere is no equivalent for
move, in any direction. Themovecontract (packages/vfs/src/index.ts:91-95) covers a missing source and replace-rather-than-merge, and says nothing about overlap, so the suite is not omitting a documented rule — the rule was never written down.This matters more for
movethan the symmetry suggests.moveFallbackis copy-then-delete, so an unguarded overlapping move is the #176 heap exhaustion with adeleteof the source at the end.The bug it hides
vfsErrinpackages/vfs/src/node/node.ts:66-77mapsENOENT,EEXIST,ENOTDIR,EISDIR,ENOTEMPTY,EACCESandEPERM. POSIXrename(2)returnsEINVALfor a directory moved into itself, which is not in that list, so it falls through tothrow e:MemoryVFSrejects the same call withprecondition-failed(its explicitpathsOverlapcheck, added in6c66acb88for #176), andcopyFallbackrejects withprecondition-failedtoo. So the identical WebDAV MOVE answers 412 over one backend and a generic error over the other, because the rawErrorfails theinstanceof VFSErrortest in the server's handler.This is the same shape as the
NodeVFSinverted-rangeERR_OUT_OF_RANGEthat #175 turned up: an errno reaching a caller unmapped.Requested direction
movecontract, matching whatcopyalready promises — reject withprecondition-failed, in both directions, without modifying either path.move, mirroring thecopyones.EINVALinvfsErr, or guard overlap inNodeVFS.moveexplicitly before callingfsp.rename. The explicit guard is probably better:EINVALcovers more than overlap, so mapping it wholesale toprecondition-failedwould mislabel other failures.Step 2 is what makes step 3 fail, so they want doing together.
Worth checking the other implementations against the new cases while there —
FsaVFS, and the SFTP, FTP, SMB2 and WebDAV clients, whose answer comes from a remote server and may not beprecondition-failedeither.Related
MemoryVFS.copy, closed. This is themovehalf.existingreplacesoverwrite, the overlap rule should be stated in the same pass.renamewill need the same rule, and inherits it if it is on the contract rather than on one method.