bjorn3 / browser_wasi_shim

A WASI shim for in the browser
Apache License 2.0
299 stars 40 forks source link

feature request: Implement both `path_remove_directory` and `path_unlink_file` #49

Closed HarikrishnanBalagopal closed 10 months ago

HarikrishnanBalagopal commented 10 months ago

Feature Request

We are implementing 2 of the syscalls that deal with removing files and empty directories.

https://github.com/bjorn3/browser_wasi_shim/blob/7349f7d32a8e6db7ecbd52df8d7373eb0a876130/src/fd.ts#L99-L101 https://github.com/bjorn3/browser_wasi_shim/blob/7349f7d32a8e6db7ecbd52df8d7373eb0a876130/src/fd.ts#L108-L110 https://wasix.org/docs/api-reference/wasi/path_unlink_file https://wasix.org/docs/api-reference/wasi/path_remove_directory

Status

We have implemented both and it seems to work for empty file and empty directory. The code is based on https://github.com/bjorn3/browser_wasi_shim/pull/42/files#diff-177a21b9f40c72ea679cf8658d14ed0d9702afcea694e92e0936f11ab182d2a3R331-R333 with some additional checks.

Challenges

bjorn3 commented 10 months ago

The WASI spec requires that we check file and directory permissions/rights before removing/unlinking but the rights base isn't stored anywhere?

Correct. Currently we entirely ignore permissions except at the location you mentioned. https://github.com/bjorn3/browser_wasi_shim/pull/42 has an implementation of this, but I need to look further into how exactly wasi's rights system works to check if the implementation is correct.

Fails on a directory that is not empty. In Golang with https://pkg.go.dev/os#RemoveAll it uses a recursive algorithm with unlinkat to unlink the files and remove the empty directories. This goes into an infinite loop.

Weird.

HarikrishnanBalagopal commented 10 months ago

https://github.com/bjorn3/browser_wasi_shim/pull/50/files#diff-177a21b9f40c72ea679cf8658d14ed0d9702afcea694e92e0936f11ab182d2a3R381 This is the code we have so far

bjorn3 commented 10 months ago

I will take a look tomorrow.