Skip to content
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions system/lib/wasmfs/backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ class Backend {
virtual std::shared_ptr<Directory> createDirectory(mode_t mode) = 0;
virtual std::shared_ptr<Symlink> createSymlink(std::string target) = 0;

// Returns true if this backend requires WasmFS to traverse and resolve
// paths (e.g. because it is mounted under a prefix). Returns false if
// paths can be passed through directly to the backend.
virtual bool requiresPathTraversal() { return true; }

virtual ~Backend() = default;
};

Expand Down
12 changes: 12 additions & 0 deletions system/lib/wasmfs/backends/node_backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ class NodeState {
int fd = -1;

public:
// Base path in the host filesystem for this backend.
// An empty string means "no base path": paths are used as-is without
// prefixing.
std::string path;

NodeState(std::string path) : path(path) {}
Expand Down Expand Up @@ -180,6 +183,11 @@ class NodeDirectory : public Directory {

private:
std::string getChildPath(const std::string& name) {
// If state.path is empty, this backend represents the real root and paths
// should be passed through unchanged.
if (state.path.empty()) {
return name;
}
Comment thread
tlively marked this conversation as resolved.
return state.path + '/' + name;
}

Expand Down Expand Up @@ -296,6 +304,10 @@ class NodeBackend : public Backend {
std::shared_ptr<Symlink> createSymlink(std::string target) override {
WASMFS_UNREACHABLE("TODO: implement NodeBackend::createSymlink");
}

virtual bool requiresPathTraversal() override {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about calling this usesRawPaths? (And flipping the result, of course.) I think that's slightly easier to understand than requiresPathTraversal, though it's not perfect.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think usesRawPaths it's too implementation-specific. How about requiresPathResolution? See commit e21bc63.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good to me.

return !mountPath.empty();
}
};

// TODO: symlink
Expand Down
6 changes: 5 additions & 1 deletion system/lib/wasmfs/backends/noderawfs_root.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,9 @@
#include "emscripten/wasmfs.h"

backend_t wasmfs_create_root_dir(void) {
return wasmfs_create_node_backend(".");
// Use an empty string as the backend "path" to indicate that this backend
// is mounted at the real filesystem root. In this mode, paths are passed
// through verbatim (no "./" prefix or path rewriting), which is required
// for correct handling of absolute paths under NODERAWFS.
return wasmfs_create_node_backend("");
}
15 changes: 12 additions & 3 deletions system/lib/wasmfs/paths.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,21 @@ ParsedParent doParseParent(std::string_view path,
size_t& recursions) {
// Empty paths never exist.
if (path.empty()) {
return {-ENOENT};
return -ENOENT;
}

auto root = wasmFS.getRootDirectory();

// If the root backend does not require path traversal, it can operate on the
// full path directly. In that case, skip WasmFS path parsing and pass the
// path through unchanged.
if (!root->getBackend()->requiresPathTraversal()) {
return {std::make_pair(std::move(curr), path)};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to cause WasmFS to model the underlying file system as completely flat, so that there is a single mount directory directly containing every file ever opened, no matter how deep that file is in the underlying directory structure?

If so, is that going to cause problems with reading directory entries?

Copy link
Copy Markdown
Collaborator Author

@kleisauke kleisauke Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, this is going to break in such situations as it implicitly assumes that the entire file system behaves like the root backend.

Commit e21bc63 changes this to curr->getBackend() instead, which allows:

$ tree / -L 2 --dirsfirst
/ (MEMFS)
├── dev
│   ├── null -> SpecialFiles::getNull()
│   ├── random -> SpecialFiles::getRandom()
│   ├── stderr -> SpecialFiles::getStderr()
│   ├── stdin -> SpecialFiles::getStdin()
│   ├── stdout -> SpecialFiles::getStdout()
│   └── urandom -> SpecialFiles::getURandom()
├── tmp
└── host (NODERAWFS)

(i.e. the Node filesystem is mounted under /host, while the wasmfs_create_root_dir() hook is not overriden)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... actually, that's not a great example, since it already worked without issues (AFAIK). I suspect it would have failed previously with the following layout:

$ tree / -L 3 --dirsfirst
/ (NODERAWFS)
└── memory (MEMFS)
    ├── dev
    │   ├── null -> SpecialFiles::getNull()
    │   ├── random -> SpecialFiles::getRandom()
    │   ├── stderr -> SpecialFiles::getStderr()
    │   ├── stdin -> SpecialFiles::getStdin()
    │   ├── stdout -> SpecialFiles::getStdout()
    │   └── urandom -> SpecialFiles::getURandom()
    └── tmp

So, the wasmfs_create_root_dir() hook is overridden via -sNODERAWFS, while MEMFS is mounted at /memory. Though, /memory/dev and /memory/tmp aren't present, but that's an issue for PR #26607.

I suppose that we're missing some test coverage for this.

}

// Handle absolute paths.
if (path.front() == '/') {
curr = wasmFS.getRootDirectory();
curr = root;
path.remove_prefix(1);
}

Expand All @@ -84,7 +93,7 @@ ParsedParent doParseParent(std::string_view path,
// contain a child segment for us to return. The root is its own parent, so we
// can handle this by returning (root, ".").
if (path.empty()) {
return {std::make_pair(std::move(curr), std::string_view("."))};
return {std::make_pair(std::move(curr), ".")};
}

while (true) {
Expand Down
7 changes: 5 additions & 2 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -6151,9 +6151,12 @@ def test_unistd_unlink(self):

# Several differences/bugs on non-linux including https://github.com/nodejs/node/issues/18014
# TODO: NODERAWFS in WasmFS
if '-DNODERAWFS' in self.cflags and os.geteuid() == 0:
if '-DNODERAWFS' in self.cflags:
# 0 if root user
self.cflags += ['-DSKIP_ACCESS_TESTS']
if os.geteuid() == 0:
self.cflags += ['-DSKIP_ACCESS_TESTS']
if self.get_setting('WASMFS'):
self.skipTest('https://github.com/emscripten-core/emscripten/issues/18112')

self.do_runf('unistd/unlink.c', 'success')

Expand Down
17 changes: 12 additions & 5 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -9291,12 +9291,19 @@ def test_noderawfs_disables_embedding(self):
self.assert_fail(base + ['--preload-file', 'somefile'], expected)
self.assert_fail(base + ['--embed-file', 'somefile'], expected)

@crossplatform
@also_with_wasmfs
def test_noderawfs_access_abspath(self):
create_file('foo', 'bar')
create_file('access.c', r'''
#include <stdio.h>
#include <assert.h>
#include <unistd.h>
int main(int argc, char** argv) {
return access(argv[1], F_OK);
printf("testing access to %s\n", argv[1]);
int rtn = access(argv[1], F_OK);
assert(rtn == 0);
return 0;
}
''')
self.do_runf('access.c', cflags=['-sNODERAWFS'], args=[os.path.abspath('foo')])
Expand Down Expand Up @@ -13256,6 +13263,8 @@ def test_unistd_chown(self):

@wasmfs_all_backends
def test_wasmfs_getdents(self):
if self.get_setting('NODERAWFS'):
self.skipTest('test expectations assumes /dev is virtualized')
# Run only in WASMFS for now.
self.set_setting('FORCE_FILESYSTEM')
self.do_run_in_out_file_test('wasmfs/wasmfs_getdents.c')
Expand Down Expand Up @@ -13844,15 +13853,13 @@ def test_fs_icase(self):
@crossplatform
@with_all_fs
def test_std_filesystem(self):
if self.get_setting('NODERAWFS') and self.get_setting('WASMFS'):
self.skipTest('https://github.com/emscripten-core/emscripten/issues/24830')
if (WINDOWS or MACOS) and self.get_setting('NODERAWFS') and self.get_setting('WASMFS'):
self.skipTest('fails with ENOTEMPTY (Directory not empty) during fs::remove_all')
self.do_other_test('test_std_filesystem.cpp')

@crossplatform
@with_all_fs
def test_std_filesystem_tempdir(self):
if self.get_setting('NODERAWFS') and self.get_setting('WASMFS'):
self.skipTest('https://github.com/emscripten-core/emscripten/issues/24830')
self.do_other_test('test_std_filesystem_tempdir.cpp', cflags=['-g'])

def test_strict_js_closure(self):
Expand Down
Loading