From 5bd422eb114804780f2e7a25e9cfc7bda7af943c Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Mon, 25 Nov 2024 20:20:02 +0100 Subject: [PATCH 01/10] When opening broken symlink with O_CREAT, create file at target Instead of raising an EEXIST error --- src/library_fs.js | 24 +++++++++++---- test/test_core.py | 3 ++ test/unistd/write_broken_link.c | 49 +++++++++++++++++++++++++++++++ test/unistd/write_broken_link.out | 6 ++++ 4 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 test/unistd/write_broken_link.c create mode 100644 test/unistd/write_broken_link.out diff --git a/src/library_fs.js b/src/library_fs.js index 886efa61f4c45..c0cc58d2f7f75 100644 --- a/src/library_fs.js +++ b/src/library_fs.js @@ -199,8 +199,14 @@ FS.staticInit(); break; } - current = FS.lookupNode(current, parts[i]); current_path = PATH.join2(current_path, parts[i]); + try { + current = FS.lookupNode(current, parts[i]); + } catch (e) { + if ((e?.errno === {{{ cDefs.ENOENT }}}) && islast && opts.handleBrokenLink) { + return { path: current_path }; + } + } // jump to the mount's root node if this is a mountpoint if (FS.isMountpoint(current)) { @@ -213,11 +219,17 @@ FS.staticInit(); // setting opts.follow = true will override this behavior. if (!islast || opts.follow) { var count = 0; - while (FS.isLink(current.mode)) { - var link = FS.readlink(current_path); + while (current && FS.isLink(current.mode)) { + if (!current.node_ops.readlink) { + throw new FS.ErrnoError({{{ cDefs.ENOSYS }}}); + } + var link = current.node_ops.readlink(current); current_path = PATH_FS.resolve(PATH.dirname(current_path), link); - var lookup = FS.lookupPath(current_path, { recurse_count: opts.recurse_count + 1 }); + var lookup = FS.lookupPath(current_path, { + recurse_count: opts.recurse_count + 1, + handleBrokenLink: islast && opts.handleBrokenLink + }); current = lookup.node; if (count++ > 40) { // limit max consecutive symlinks to 40 (SYMLOOP_MAX). @@ -1027,9 +1039,11 @@ FS.staticInit(); path = PATH.normalize(path); try { var lookup = FS.lookupPath(path, { - follow: !(flags & {{{ cDefs.O_NOFOLLOW }}}) + follow: !(flags & {{{ cDefs.O_NOFOLLOW }}}), + handleBrokenLink: true }); node = lookup.node; + path = lookup.path; } catch (e) { // ignore } diff --git a/test/test_core.py b/test/test_core.py index f0da30a4af1f6..b0c2b49bbecf8 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -6059,6 +6059,9 @@ def test_unistd_links(self, args, nodefs): self.do_run_in_out_file_test('unistd/links.c', emcc_args=args) + def test_unistd_write_broken_link(self): + self.do_run_in_out_file_test('unistd/write_broken_link.c') + @no_windows('Skipping NODEFS test, since it would require administrative privileges.') @requires_node def test_unistd_symlink_on_nodefs(self): diff --git a/test/unistd/write_broken_link.c b/test/unistd/write_broken_link.c new file mode 100644 index 0000000000000..6f74ba2b3512b --- /dev/null +++ b/test/unistd/write_broken_link.c @@ -0,0 +1,49 @@ +#include +#include +#include +#include +#include +#include + + +char* join_path(const char* path1, const char* path2) { + int len1 = strlen(path1); + int len2 = strlen(path2); + char* result = malloc(len1 + len2 + 2); + memcpy(result, path1, len1); + result[len1] = '/'; + memcpy(result + len1 + 1, path2, len2); + return result; +} + +int main() { + char template[] = "/tmp/tmpdir.XXXXXX"; + char *tmpdir = mkdtemp(template); + char* p1 = join_path(tmpdir, "test"); + char* p2 = join_path(tmpdir, "test2"); + + int res = symlink(p2, p1); + printf("link result: %d\n", res); + int src_fd = open(p1, O_CREAT | O_WRONLY, 0777); + printf("source_fd: %d, errno: %d %s\n", src_fd, errno, strerror(errno)); + write(src_fd, "abc", 3); + close(src_fd); + { + int target_fd = open(p2, O_RDONLY); + printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno)); + char buf[10]; + read(target_fd, buf, 10); + printf("buf: '%s'\n", buf); + close(target_fd); + } + { + int target_fd = open(p1, O_RDONLY); + printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno)); + char buf[10]; + read(target_fd, buf, 10); + printf("buf: '%s'\n", buf); + close(target_fd); + } + free(p1); + free(p2); +} diff --git a/test/unistd/write_broken_link.out b/test/unistd/write_broken_link.out new file mode 100644 index 0000000000000..6f5a9e3ea3183 --- /dev/null +++ b/test/unistd/write_broken_link.out @@ -0,0 +1,6 @@ +link result: 0 +source_fd: 3, errno: 0 No error information +target_fd: 3, errno: 0 No error information +buf: 'abc' +target_fd: 3, errno: 0 No error information +buf: 'abc' From 751652c2b733102c8d904b23b381553bc8cbd074 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 26 Nov 2024 10:28:46 +0100 Subject: [PATCH 02/10] Update test --- test/unistd/write_broken_link.c | 63 +++++++++++++-------------------- 1 file changed, 24 insertions(+), 39 deletions(-) diff --git a/test/unistd/write_broken_link.c b/test/unistd/write_broken_link.c index 6f74ba2b3512b..4eb7401bcb50f 100644 --- a/test/unistd/write_broken_link.c +++ b/test/unistd/write_broken_link.c @@ -1,4 +1,3 @@ -#include #include #include #include @@ -6,44 +5,30 @@ #include -char* join_path(const char* path1, const char* path2) { - int len1 = strlen(path1); - int len2 = strlen(path2); - char* result = malloc(len1 + len2 + 2); - memcpy(result, path1, len1); - result[len1] = '/'; - memcpy(result + len1 + 1, path2, len2); - return result; -} - int main() { - char template[] = "/tmp/tmpdir.XXXXXX"; - char *tmpdir = mkdtemp(template); - char* p1 = join_path(tmpdir, "test"); - char* p2 = join_path(tmpdir, "test2"); + char* p1 = "link_source"; + char* p2 = "link_target"; - int res = symlink(p2, p1); - printf("link result: %d\n", res); - int src_fd = open(p1, O_CREAT | O_WRONLY, 0777); - printf("source_fd: %d, errno: %d %s\n", src_fd, errno, strerror(errno)); - write(src_fd, "abc", 3); - close(src_fd); - { - int target_fd = open(p2, O_RDONLY); - printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno)); - char buf[10]; - read(target_fd, buf, 10); - printf("buf: '%s'\n", buf); - close(target_fd); - } - { - int target_fd = open(p1, O_RDONLY); - printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno)); - char buf[10]; - read(target_fd, buf, 10); - printf("buf: '%s'\n", buf); - close(target_fd); - } - free(p1); - free(p2); + int res = symlink(p2, p1); + printf("link result: %d\n", res); + int src_fd = open(p1, O_CREAT | O_WRONLY, 0777); + printf("source_fd: %d, errno: %d %s\n", src_fd, errno, strerror(errno)); + write(src_fd, "abc", 3); + close(src_fd); + { + int target_fd = open(p2, O_RDONLY); + printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno)); + char buf[10]; + read(target_fd, buf, 10); + printf("buf: '%s'\n", buf); + close(target_fd); + } + { + int target_fd = open(p1, O_RDONLY); + printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno)); + char buf[10]; + read(target_fd, buf, 10); + printf("buf: '%s'\n", buf); + close(target_fd); + } } From 96e318f865c545193eab018fef3efe471ce84340 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 26 Nov 2024 11:27:41 +0100 Subject: [PATCH 03/10] Add missing throw --- src/library_fs.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/library_fs.js b/src/library_fs.js index c0cc58d2f7f75..53f2ffe66f213 100644 --- a/src/library_fs.js +++ b/src/library_fs.js @@ -206,6 +206,7 @@ FS.staticInit(); if ((e?.errno === {{{ cDefs.ENOENT }}}) && islast && opts.handleBrokenLink) { return { path: current_path }; } + throw e; } // jump to the mount's root node if this is a mountpoint From 9beea7e2c5b7006a4364a9accea6235ef70fc41b Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 26 Nov 2024 11:45:13 +0100 Subject: [PATCH 04/10] Remove try/catch block which is no longer needed --- src/library_fs.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/library_fs.js b/src/library_fs.js index 53f2ffe66f213..b1ffe98b399f9 100644 --- a/src/library_fs.js +++ b/src/library_fs.js @@ -1038,16 +1038,12 @@ FS.staticInit(); node = path; } else { path = PATH.normalize(path); - try { - var lookup = FS.lookupPath(path, { - follow: !(flags & {{{ cDefs.O_NOFOLLOW }}}), - handleBrokenLink: true - }); - node = lookup.node; - path = lookup.path; - } catch (e) { - // ignore - } + var lookup = FS.lookupPath(path, { + follow: !(flags & {{{ cDefs.O_NOFOLLOW }}}), + handleBrokenLink: true + }); + node = lookup.node; + path = lookup.path; } // perhaps we need to create the node var created = false; From 1f5a17e0863df9db1d23fc75bc9bc2d1b0b4e5d1 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 26 Nov 2024 11:47:46 +0100 Subject: [PATCH 05/10] Rename option and add comment --- src/library_fs.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/library_fs.js b/src/library_fs.js index b1ffe98b399f9..611e8a2a18a97 100644 --- a/src/library_fs.js +++ b/src/library_fs.js @@ -203,7 +203,10 @@ FS.staticInit(); try { current = FS.lookupNode(current, parts[i]); } catch (e) { - if ((e?.errno === {{{ cDefs.ENOENT }}}) && islast && opts.handleBrokenLink) { + // if noent_okay is true, suppress a ENOENT in the last component + // and return an object with an undefined node. This is needed for + // resolving symlinks in the path when creating a file. + if ((e?.errno === {{{ cDefs.ENOENT }}}) && islast && opts.noent_okay) { return { path: current_path }; } throw e; @@ -1038,9 +1041,12 @@ FS.staticInit(); node = path; } else { path = PATH.normalize(path); + // noent_okay makes it so that if the final component of the path + // doesn't exist, lookupPath returns `node: undefined`. `path` will be + // updated to point to the target of all symlinks. var lookup = FS.lookupPath(path, { follow: !(flags & {{{ cDefs.O_NOFOLLOW }}}), - handleBrokenLink: true + noent_okay: true }); node = lookup.node; path = lookup.path; From bd1849b3365d9377cb5d19e89286e6120e71fffd Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 26 Nov 2024 13:55:27 +0100 Subject: [PATCH 06/10] Fix --- src/library_fs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library_fs.js b/src/library_fs.js index 611e8a2a18a97..8fd368a32706a 100644 --- a/src/library_fs.js +++ b/src/library_fs.js @@ -232,7 +232,7 @@ FS.staticInit(); var lookup = FS.lookupPath(current_path, { recurse_count: opts.recurse_count + 1, - handleBrokenLink: islast && opts.handleBrokenLink + noent_okay: islast && opts.noent_okay }); current = lookup.node; From 39428ce2d141d156cad884e1a51edda5fcadd62d Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 26 Nov 2024 22:54:57 +0100 Subject: [PATCH 07/10] Add also_with_noderawfs to test --- test/test_core.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_core.py b/test/test_core.py index b0c2b49bbecf8..9d7d8329678ee 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -6059,6 +6059,7 @@ def test_unistd_links(self, args, nodefs): self.do_run_in_out_file_test('unistd/links.c', emcc_args=args) + @also_with_noderawfs def test_unistd_write_broken_link(self): self.do_run_in_out_file_test('unistd/write_broken_link.c') From 7cd06038255f9dafc18c0da24f1ef34dcf57f13f Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 28 Nov 2024 14:13:28 +0100 Subject: [PATCH 08/10] Fix test_unistd_open --- test/wasmfs/wasmfs_open.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/wasmfs/wasmfs_open.c b/test/wasmfs/wasmfs_open.c index eb8454f1ccd0a..ac95cbd066746 100644 --- a/test/wasmfs/wasmfs_open.c +++ b/test/wasmfs/wasmfs_open.c @@ -89,11 +89,7 @@ int main() { int fd5 = open("/dev/stdout/foo", O_RDWR); printf("Errno: %s\n", strerror(errno)); // Both errors are valid, but in WasmFS, ENOTDIR is returned first. -#ifdef WASMFS assert(errno == ENOTDIR); -#else - assert(errno == ENOENT); -#endif errno = 0; // Attempt to open and write to the root directory. From e244345e31e615a714a99cd335ec106e9e12a823 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 4 Dec 2024 22:26:50 +0100 Subject: [PATCH 09/10] Rename test files --- test/test_core.py | 2 +- .../{write_broken_link.c => test_unistd_write_broken_link.c} | 0 ...{write_broken_link.out => test_unistd_write_broken_link.out} | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename test/unistd/{write_broken_link.c => test_unistd_write_broken_link.c} (100%) rename test/unistd/{write_broken_link.out => test_unistd_write_broken_link.out} (100%) diff --git a/test/test_core.py b/test/test_core.py index 353ecf692c32e..9e855cfcddc62 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -6097,7 +6097,7 @@ def test_unistd_links(self, args, nodefs): @also_with_noderawfs def test_unistd_write_broken_link(self): - self.do_run_in_out_file_test('unistd/write_broken_link.c') + self.do_run_in_out_file_test('unistd/test_unistd_write_broken_link.c') @no_windows('Skipping NODEFS test, since it would require administrative privileges.') @requires_node diff --git a/test/unistd/write_broken_link.c b/test/unistd/test_unistd_write_broken_link.c similarity index 100% rename from test/unistd/write_broken_link.c rename to test/unistd/test_unistd_write_broken_link.c diff --git a/test/unistd/write_broken_link.out b/test/unistd/test_unistd_write_broken_link.out similarity index 100% rename from test/unistd/write_broken_link.out rename to test/unistd/test_unistd_write_broken_link.out From 6304e9c53c790fc3aa11c68c5ac10abb5ba18ff8 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 4 Dec 2024 22:27:48 +0100 Subject: [PATCH 10/10] Inline strings --- test/unistd/test_unistd_write_broken_link.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/unistd/test_unistd_write_broken_link.c b/test/unistd/test_unistd_write_broken_link.c index 4eb7401bcb50f..1c119a6e45a7e 100644 --- a/test/unistd/test_unistd_write_broken_link.c +++ b/test/unistd/test_unistd_write_broken_link.c @@ -6,17 +6,14 @@ int main() { - char* p1 = "link_source"; - char* p2 = "link_target"; - - int res = symlink(p2, p1); + int res = symlink("link_target", "link_source"); printf("link result: %d\n", res); - int src_fd = open(p1, O_CREAT | O_WRONLY, 0777); + int src_fd = open("link_source", O_CREAT | O_WRONLY, 0777); printf("source_fd: %d, errno: %d %s\n", src_fd, errno, strerror(errno)); write(src_fd, "abc", 3); close(src_fd); { - int target_fd = open(p2, O_RDONLY); + int target_fd = open("link_target", O_RDONLY); printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno)); char buf[10]; read(target_fd, buf, 10); @@ -24,7 +21,7 @@ int main() { close(target_fd); } { - int target_fd = open(p1, O_RDONLY); + int target_fd = open("link_source", O_RDONLY); printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno)); char buf[10]; read(target_fd, buf, 10);