Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wasi: fix symlink-related issues with path handling #1648

Merged
merged 2 commits into from
Aug 23, 2023
Merged

Conversation

evacchi
Copy link
Contributor

@evacchi evacchi commented Aug 22, 2023

  • Fixes a bug in handling the buffer when the write goes over the given limit. ERANGE should be returned.
  • Removes dependency from the syscall package in fs.go, experimentalsys.O_WRONLY should be used instead (portability)

@evacchi
Copy link
Contributor Author

evacchi commented Aug 22, 2023

  • symlink_filestat

    ==> wasi_snapshot_preview1.path_filestat_get(fd=5,flags=,path=symlink)
    <== (filestat={filetype=SYMBOLIC_LINK,size=4,mtim=1692707577959674593},errno=ESUCCESS)
    thread 'main' panicked at 'assertion failed: `(left == right)`
      left: `1692707577959674593`,
     right: `1692707577959674393`: symlink mtim should change', src/bin/symlink_filestat.rs:46:5
    
    
        // Check that symlink mtim motification worked
        let modified_sym_stat = wasi::path_filestat_get(dir_fd, 0, "symlink")
            .expect("reading file stats after path_filestat_set_times");
        assert_eq!(
            modified_sym_stat.mtim, sym_new_mtim,
            "symlink mtim should change"
        );
    

utimens actually supports a NOFOLLOW flag, that we were using originally; the rationale for not using utimens(flag) is described in #1588

however, it seems like the workaround is actually resolving the link.

On Linux, the proprietary flag O_PATH seems to do what we want:
https://man7.org/linux/man-pages/man2/open.2.html

Obtain a file descriptor that can be used for two
purposes: to indicate a location in the filesystem tree
and to perform operations that act purely at the file
descriptor level. The file itself is not opened, and
other file operations (e.g., read(2), write(2), fchmod(2),
fchown(2), fgetxattr(2), ioctl(2), mmap(2)) fail with the
error EBADF.

...but this is obviously non-portable.

Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
@evacchi evacchi changed the title wasi: fix symlink-related path handling wasi: fix symlink-related issues with path handling Aug 23, 2023
@evacchi
Copy link
Contributor Author

evacchi commented Aug 23, 2023

repurposing this PR to only fix the actual issues, dropping the edge cases for path handling and resolution in symlinks for now.

@evacchi evacchi marked this pull request as ready for review August 23, 2023 15:07
@mathetake mathetake merged commit a5b30a5 into main Aug 23, 2023
54 checks passed
@mathetake mathetake deleted the fix-wasi-tests branch August 23, 2023 22:25
@mathetake
Copy link
Member

good call!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants