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

Need more tests for W/Z suffixed std.fs functions #18080

Open
squeek502 opened this issue Nov 22, 2023 · 2 comments
Open

Need more tests for W/Z suffixed std.fs functions #18080

squeek502 opened this issue Nov 22, 2023 · 2 comments
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@squeek502
Copy link
Collaborator

squeek502 commented Nov 22, 2023

In light of #16736 being closed, this:

Note also that pretty much all Absolute functions have poor test coverage to the point that some will give a compile error if anyone tried to actually use them (e.g. on Windows anything that tries to call the no-longer-existing os.windows.cStrToWin32PrefixedFileW function [these functions have never worked since their introduction in #5879 which ended up removing cStrToWin32PrefixedFileW before it was merged but left some calls of it around])

needs to be addressed by actually fixing and improving the test coverage of the Z/W suffixed functions.

Relevant test file:

https://github.com/ziglang/zig/blob/master/lib/std/fs/test.zig

@squeek502 squeek502 added contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library. labels Nov 22, 2023
@Vexu Vexu added this to the 0.13.0 milestone Nov 23, 2023
@matu3ba
Copy link
Contributor

matu3ba commented Dec 2, 2023

@squeek502 Can you be more specific how those tests are insufficient? Except deleteTreeAbsolute each fn is used at least once in https://github.com/ziglang/zig/blob/master/lib/std/fs/test.zig as of bf5ab54.

  • Do you mean that it does not test all file path encodings (ntdll networkpaths and regular ones on Windows) ?
  • Can you tldr; what the suggested strategy was regarding the absolute functions to keep things portable? Unfortunately I don't know a wrapup and the comments do not mention in what ways absolute paths can or can not be used or relied upon.
  • What should code do, which relies on some absolute paths of the platform for security guarantees to prevent attack vectors ?

Summary of current state:

general

  • accessAbsolute: test "accessAbsolute" (used in test ". and .. in absolute functions")
  • renameAbsolute: test "renameAbsolute" (used in test ". and .. in absolute functions")

identically named

  • openDirAbsolute: test "openDirAbsolute"
  • openFileAbsolute (used in test ". and .. in absolute functions")
  • deleteFileAbsolute (used in test "file operations on directories", test "open file with exclusive nonblocking lock twice (absolute paths)")
  • deleteDirAbsolute (used in test "directory operations on files", test ". and .. in absolute functions")
  • deleteTreeAbsolute: not tested

weirdly differently named

  • createFileAbsolute (used in test "file operations on directories", test "open file with exclusive nonblocking lock twice (absolute paths)")
  • makeDirAbsolute (used in test "directory operations on files", test ". and .. in absolute functions")

differently named

  • copyFileAbsolute (used in test ". and .. in absolute functions")
  • updateFileAbsolute (used in test ". and .. in absolute functions")
  • readLinkAbsolute: test "readLinkAbsolute"
  • symlinkAbsolute (used in test "readLinkAbsolute")

@squeek502
Copy link
Collaborator Author

squeek502 commented Dec 2, 2023

Sorry, I had things a bit mixed up.

The real issue is the W/Z suffixed versions of the Absolute functions. For example, readLinkAbsoluteZ and symLinkAbsoluteZ are compile errors on Windows currently. This lack of testing of the W/Z suffixed versions probably also applies to the Dir functions as well.

@squeek502 squeek502 changed the title Need more tests for Absolute suffixed std.fs functions Need more tests for W/Z suffixed std.fs functions Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

3 participants