Fix renaming a file on top of an existing file in memfs and nodefs (#23074)

Author: hoodmane

src/library_memfs.js

@@ -191,18 +191,18 @@ addToLibrary({
         return MEMFS.createNode(parent, name, mode, dev);
       },
       rename(old_node, new_dir, new_name) {
-        // if we're overwriting a directory at new_name, make sure it's empty.
-        if (FS.isDir(old_node.mode)) {
-          var new_node;
-          try {
-            new_node = FS.lookupNode(new_dir, new_name);
-          } catch (e) {
-          }
-          if (new_node) {
+        var new_node;
+        try {
+          new_node = FS.lookupNode(new_dir, new_name);
+        } catch (e) {}
+        if (new_node) {
+          if (FS.isDir(old_node.mode)) {
+            // if we're overwriting a directory at new_name, make sure it's empty.
             for (var i in new_node.contents) {
               throw new FS.ErrnoError({{{ cDefs.ENOTEMPTY }}});
             }
           }
+          FS.hashRemoveNode(new_node);
         }
         // do the internal rewiring
         delete old_node.parent.contents[old_node.name];

src/library_nodefs.js

@@ -197,6 +197,9 @@ addToLibrary({
       rename(oldNode, newDir, newName) {
         var oldPath = NODEFS.realPath(oldNode);
         var newPath = PATH.join2(NODEFS.realPath(newDir), newName);
+        try {
+          FS.unlink(newPath);
+        } catch(e) {}
         NODEFS.tryFSOperation(() => fs.renameSync(oldPath, newPath));
         oldNode.name = newName;
       },

test/fs/test_fs_rename_on_existing.c

@@ -0,0 +1,55 @@
+#include <stdio.h>
+#include <unistd.h>
+#include <assert.h>
+#include <fcntl.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <assert.h>
+#include <errno.h>
+#include <string.h>
+
+
+#if defined(__EMSCRIPTEN__)
+#include <emscripten.h>
+#endif
+
+void makedir(const char *dir) {
+  int rtn = mkdir(dir, 0777);
+  assert(rtn == 0);
+}
+
+void changedir(const char *dir) {
+  int rtn = chdir(dir);
+  assert(rtn == 0);
+}
+
+static void create_file(const char *path, const char *buffer) {
+  printf("creating: %s\n", path);
+  int fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0666);
+  printf("error: %s\n", strerror(errno));
+  assert(fd >= 0);
+
+  int err = write(fd, buffer, sizeof(char) * strlen(buffer));
+  assert(err ==  (sizeof(char) * strlen(buffer)));
+
+  close(fd);
+}
+
+
+void setup() {
+#if defined(__EMSCRIPTEN__) && defined(NODEFS)
+  makedir("working");
+  EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working'));
+  changedir("working");
+#endif
+}
+
+int main() {
+    setup();
+    create_file("a", "abc");
+    create_file("b", "xyz");
+    assert(rename("a", "b") == 0);
+    assert(unlink("b") == 0);
+    create_file("b", "xyz");
+    printf("success\n");
+}

test/test_core.py

@@ -5935,6 +5935,16 @@ def test_fs_64bit(self):
       self.set_setting('FORCE_FILESYSTEM')
     self.do_runf('fs/test_64bit.c', 'success')
 
+  @parameterized({
+    '': ([],),
+    'nodefs': (['-DNODEFS', '-lnodefs.js'],),
+    'noderawfs': (['-sNODERAWFS'],)
+  })
+  def test_fs_rename_on_existing(self, args):
+    if self.get_setting('WASMFS'):
+      self.set_setting('FORCE_FILESYSTEM')
+    self.do_runf('fs/test_fs_rename_on_existing.c', 'success', emcc_args=args)
+
   def test_sigalrm(self):
     self.do_runf('test_sigalrm.c', 'Received alarm!')
     self.set_setting('EXIT_RUNTIME')