From c028edb335f3035a3f32c5d459d587ea723da24c Mon Sep 17 00:00:00 2001
From: Hood Chatham <roberthoodchatham@gmail.com>
Date: Thu, 5 Dec 2024 19:46:08 +0100
Subject: [PATCH] Fix renaming a file on top of an existing file in memfs and
 nodefs (#23074)

---
 src/library_memfs.js                 | 16 ++++----
 src/library_nodefs.js                |  3 ++
 test/fs/test_fs_rename_on_existing.c | 55 ++++++++++++++++++++++++++++
 test/test_core.py                    | 10 +++++
 4 files changed, 76 insertions(+), 8 deletions(-)
 create mode 100644 test/fs/test_fs_rename_on_existing.c

diff --git a/src/library_memfs.js b/src/library_memfs.js
index 35597c99056b5..61fe2ab8b8db6 100644
--- a/src/library_memfs.js
+++ b/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];
diff --git a/src/library_nodefs.js b/src/library_nodefs.js
index 846a930c15c48..5570bcc68f5c8 100644
--- a/src/library_nodefs.js
+++ b/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;
       },
diff --git a/test/fs/test_fs_rename_on_existing.c b/test/fs/test_fs_rename_on_existing.c
new file mode 100644
index 0000000000000..8c32bc3ed1b49
--- /dev/null
+++ b/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");
+}
diff --git a/test/test_core.py b/test/test_core.py
index 25e032ec69a79..521edb501411a 100644
--- a/test/test_core.py
+++ b/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')