Conversation
c14cf1c to
0af3741
Compare
c2b3a59 to
38270df
Compare
There is a bug in calculateAt(): if the directory node in the file system is
renamed, calculateAt() will look up the path field and get a stale path. The
regression test indicates the problem:
1. create a/file.txt and b/subdir/file.txt
2. set dirfd = open("b/subdir")
3. rename b/subdir to b/subdir_moved
4. add a symlink from b/subdir to a
5. unlinkat(dirfd, "file.txt")
In the last step it should remove b/subdir_moved/file.txt and not remove
a/file.txt. To fix we use `FS.getPath(dirstream.node)` to calculate the path,
rather than using the `dirstream.path` field.
In NODERAWFS, this doesn't work, so I left it broken.
38270df to
1ceefcd
Compare
1ceefcd to
f8b0eba
Compare
| } | ||
| set path(val) { | ||
| this._path = val; | ||
| } |
There was a problem hiding this comment.
Is this needed? i.e. can we make this a getter only?
There was a problem hiding this comment.
Well sockfs, pipefs, and noderawfs all use it.
| FS[key] = _wrapNodeStreamFunc(value, FS[key]); | ||
| }`, | ||
| } | ||
| `, |
| unlink("unlinkat-dir/file"); | ||
| rmdir("unlinkat-dir"); | ||
| rmdir("unlinkat-dir-new"); | ||
| #endif |
There was a problem hiding this comment.
Should we make this into its own test_unlinkat.c? If it can fail independently of unlink then maybe it makes sense?
|
|
||
| closedir(d); | ||
|
|
||
| #ifndef NODERAWFS // TODO: Fix NODERAWFS unlinkat() too. |
There was a problem hiding this comment.
Do you know why NODERAWFS fails here?
There was a problem hiding this comment.
Yeah but it's a bit of a tricky problem. We only have a directory descriptor and the original path that we opened the directory descriptor at. If the directory or any of its parents moved, the path changed but there is no way to find that out. Node doesn't expose unlinkat or any of the *at family operations.
There was a problem hiding this comment.
Hmm.. that is unfortunate.
This branch has not been deployed
2 participants
There is a bug in calculateAt(): if the directory node in the file system is renamed, calculateAt() will look up the path field and get a stale path. The regression test indicates the problem:
In the last step it should remove b/subdir_moved/file.txt and not remove a/file.txt. To fix we use
FS.getPath(dirstream.node)to calculate the path, rather than using thedirstream.pathfield.In NODERAWFS, this doesn't work, so I left it broken.