Repository navigation
fix: Stop assuming POSIX paths and permissions on a host without them - #3
Merged
Merged
Conversation
writeAtomically makes its temporary file with a POSIX permissions attribute. A host that keeps no permissions of that kind refuses the attribute and makes no file at all, so every write fails on such a host: the attribute view is absent there, which targetPermissions reads as no permissions to take. The permissions are read where the host keeps them now, and the attribute is passed only then. A file that is replaced still takes the permissions of the one it replaces, and the write is still atomic.
The floor denies /dev on every host. On a host without a device tree that path is \dev, which names no root of its own, so no check can canonicalize it and the floor cannot be built: AgentFloor is made of those roots, so the agent plugin is not created at all and everything that starts the runtime fails on such a host. The device tree is a root where there is one, so it is denied where the host separates paths with a slash and nowhere else.
Seven tests of this module passed on Linux and failed on Windows, and none of them was about the code under test. Each one wrote an expectation in the syntax of one platform while the value comes from the host: - The XDG cases fed the variables /xdg/data, which Windows does not call an absolute path, so the code fell back to its own directories. They now name directories in the syntax of the host that resolves them, which is what the case is about: the precedence of the variables, not the separator. - The cases that compare whole paths run against the systems whose syntax the host shares. A POSIX path is not rendered the same way on Windows, and no assertion can change that. - A refused path reports a wording each host has its own for, and Windows names the offending character itself: Illegal char <NUL>., with a real null between the brackets. The expected reason is read from the host now instead of being written out. - --help, --version and the usage on a bad argument are compared after their line endings are made the same, because the host prints \r\n. - The named data directory of the --help case was written with doubled backslashes, which the program never prints. The formatting of the KSP assertions from the commit before this one came along, since spotlessCheck wants the call that did not fit on a line broken up. Verified: :app:test passes, 45 tests, and :build-ksp-plugin:test passes, 118 tests, both on Windows, with their spotlessCheck; the assertions the two commits touch were passing on Linux before and are unchanged in what they check there.
Two tests wrote the data directory into the expected report as a slash separated string while the code renders it with Path.toString, so they passed on Linux and failed on Windows, where the same path prints with backslashes. They now build the expectation from the same Path the probe holds, which is what the assertion is about; the separator stops mattering. Verified: :build-ksp-plugin:test passes, 118 tests, on Windows. The suite also passes on Linux, where the old expectation happened to be right.
Yos-X
changed the title
The master branch moved to org.unitedsoftware.alexandrite and rewrote AtomicWrites, so the device tree guard is applied inside the new standardRoots and the permission handling of the platform commit is superseded by writeNewFile, which keeps no permission of that kind where the host has none. The MainTest usage text follows master, and its data directory stays the one this host names.
The floor decides what lies under what by comparing path keys, and a key was the path as the host spells it. A host that separates paths with a backslash therefore had no key under any other: isAtOrUnder compares with a slash, so every protected place was open on Windows, and credentials, the configuration roots and the installation root all read as allowed. A key names elements, not spellings, so the separators of such a host are folded to a slash while the key is made, and CanonicalPath.entryKey goes through nameKey as well so that a carve-out is compared the same way. On a POSIX host a backslash stays a character of a name. FloorWindowsTest checks a deny root on such a host, in another case and with forward slashes.
command.exec searched only the POSIX system directories and passed only the POSIX variables to a program. On a Windows host that made every program not found: the environment spells PATH as Path, which a case-sensitive lookup missed; the search list held /usr/bin and no Windows directory; and a program named without its extension, such as git, cmd or curl, was never tried as git.exe or cmd, never as git.cmd. A child process got no PATH at all and a fallback of the POSIX directories, so a program that did run could not find its own programs. - systemDirectories() keeps the Windows installation directories that the host names, and searchPath() reads PATH as the host does, quoted entries included. - resolveProgram() tries the extension that Windows runs a file by, and takes only .exe, .com, .bat and .cmd as a program. - childEnvironment() compares variable names as the host does and passes the variables a Windows program needs, such as SystemRoot, TEMP and USERPROFILE; a missing PATH falls back to the system directories. - baseName() and namesDirectory() replace the slash assumptions of the planner, the wrappers, the chains and the option tables. - an address, a URL or a user@host:path, and a word that names no path of the host, are no paths: an argument such as https://... or *.apk no longer comes back as blocked_floor, while a word that looks like a path and that the host cannot name is still refused. ProgramsWindowsTest and ExecWindowsTest cover the resolution, the variables of a child, the addresses and the run of cmd, powershell, curl and the programs that this machine installs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four places in three modules assume a POSIX host. On Windows two of them keep the runtime from working at all, and two fail tests of this repository. Each is a commit of its own.
standardRootsdeniesPath.of("/dev")on every host, and a host without a device tree reads that path as\dev, which names no root of its own. Canonicalizing it is refused:AgentFlooris built from those roots, so the agent plugin is not created at all, and everything that starts the runtime fails before a test runs:265 of the 658 tests of
:libraries:agent:testfail that way in this checkout, and the message names the floor rather than the platform. The device tree is denied where the host separates paths with/, and nowhere else.writeAtomicallymakes its temporary file with a POSIX permissions attribute, which such a host refuses:The attribute view is absent there, and
targetPermissionsreads that absence as no permissions to take and falls back to owner-only, so the attribute is passed and making the file throws.fs.write,fs.editand the persona tools then fail whatever they write. The permissions are read where the host keeps them, and the attribute is passed only then: a file that is replaced still takes the permissions of the one it replaced, and the write stays atomic.The tests of
appandbuild-ksp-pluginassert POSIX spellings, POSIX line endings, the wording a POSIX host gives for a path it refuses, and the text of one host's separator. Each assertion is taken from the host under test:Path.of, the host's line endings, the reason that host gives, and a comparison of paths as paths.The four commits were checked on Windows.
:libraries:internal:test --tests '*AtomicWrites*'passes 6 tests, where 1 failed without the write fix.:libraries:agent:testloses the 265 failures that could not start the runtime, and they return when the device-tree commit is left out.:app:test --tests '*LocationsTest*' --tests '*MainTest*'goes from 9 failures to 1, which is a control socket that a host without sockets keeps none of.:build-ksp-plugin:testpasses 115 tests, where 2 failed.spotlessCheckpasses for each module touched.macOS was not run, and the assertions speak of it as well. The Linux run of CI passes these four places already, so nothing changes there.