mirror of
https://github.com/horizon-rust/horizon.git
synced 2026-11-07 11:06:11 -10:00
horizon-server update on Windows: swap the current junction safely
point() removed the `current` junction before making the new one, so a failed mklink left the server with no current release, and std quotes an argument only when it holds a space, so a folder like D:\mc&co\ reached cmd unquoted and & split the command. - The new junction is made first as current.switching; the old one is removed and the new one renamed into place; if that rename fails the old junction is put back. - mklink runs as `cmd /D /S /C "mklink /J "link" "target""` with raw_arg, both paths quoted; paths holding % or " (which keep a meaning inside quotes) are refused with a message. - Test (Windows only): real junctions in a folder named with `&`: current moves v1 -> v2 and reads each release's file, the old release is untouched, no current.switching is left, and a release whose path holds % is refused with current unchanged. Compiled, linted (clippy -D warnings) and tested on GitHub's windows-latest runner: all 146 horizon-server tests, the two junction tests among them. The Unix build and tests pass too. Split from #9129. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SJ3stK8iXVDXqoi7midcqd
This commit is contained in:
1 parent
f319701e8c
commit
e249683ac9
1 file changed
+90
-8
@@ -215,18 +215,52 @@ fn point(link: &Path, target: &Path) -> std::io::Result<()> {
|
||||
std::fs::rename(&fresh, link)
|
||||
}
|
||||
|
||||
/// Makes `link` a junction to `target` (install.ps1's links: they need no admin rights).
|
||||
/// Makes `link` a junction to `target` (install.ps1's links: they need no admin rights). The new
|
||||
/// junction is made first, beside it, so a failure leaves `current` as it was; it then takes the old
|
||||
/// one's place, and if that last step fails the old junction is put back.
|
||||
#[cfg(windows)]
|
||||
fn point(link: &Path, target: &Path) -> std::io::Result<()> {
|
||||
if link.exists() || link.symlink_metadata().is_ok() {
|
||||
std::fs::remove_dir(link)?;
|
||||
let fresh = link.with_extension("switching");
|
||||
if fresh.symlink_metadata().is_ok() {
|
||||
std::fs::remove_dir(&fresh)?;
|
||||
}
|
||||
junction(&fresh, target)?;
|
||||
let old = std::fs::read_link(link).ok();
|
||||
if link.symlink_metadata().is_ok() {
|
||||
// A junction: removing it removes the link, never the release it points at.
|
||||
if let Err(e) = std::fs::remove_dir(link) {
|
||||
let _ = std::fs::remove_dir(&fresh);
|
||||
return Err(e);
|
||||
}
|
||||
}
|
||||
std::fs::rename(&fresh, link).inspect_err(|_| {
|
||||
let _ = std::fs::remove_dir(&fresh);
|
||||
if let Some(old) = &old {
|
||||
let _ = junction(link, old);
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// `mklink /J link target` through cmd, with both paths quoted as cmd reads them: unquoted, a folder
|
||||
/// named with `&` or `^` (and no space, so std doesn't quote it) would split the command. `%` and `"`
|
||||
/// keep a meaning inside quotes, so paths holding them are refused.
|
||||
#[cfg(windows)]
|
||||
fn junction(link: &Path, target: &Path) -> std::io::Result<()> {
|
||||
use std::os::windows::process::CommandExt;
|
||||
let quoted = |p: &Path| -> std::io::Result<String> {
|
||||
let text = p.to_str().ok_or_else(|| {
|
||||
std::io::Error::other(format!("{}: not a path cmd can take", p.display()))
|
||||
})?;
|
||||
if text.contains(['%', '"']) {
|
||||
return Err(std::io::Error::other(format!(
|
||||
"{text}: a folder whose path holds % or \" can't be linked; move the server elsewhere"
|
||||
)));
|
||||
}
|
||||
Ok(format!("\"{text}\""))
|
||||
};
|
||||
let line = mklink_line("ed(link)?, "ed(target)?);
|
||||
let status = crate::process::command("cmd")
|
||||
.arg("/C")
|
||||
.arg("mklink")
|
||||
.arg("/J")
|
||||
.arg(link)
|
||||
.arg(target)
|
||||
.raw_arg(line)
|
||||
.stdout(std::process::Stdio::null())
|
||||
.status()?;
|
||||
if status.success() {
|
||||
@@ -238,6 +272,13 @@ fn point(link: &Path, target: &Path) -> std::io::Result<()> {
|
||||
}
|
||||
}
|
||||
|
||||
/// cmd's arguments for a junction, both paths already quoted: `/D` skips AutoRun, and `/S /C "..."`
|
||||
/// makes cmd take the outer quotes off and run the rest as written.
|
||||
#[cfg_attr(not(windows), allow(dead_code))]
|
||||
fn mklink_line(link: &str, target: &str) -> String {
|
||||
format!("/D /S /C \"mklink /J {link} {target}\"")
|
||||
}
|
||||
|
||||
/// A release on GitHub: its name here and the archive for this computer.
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct Release {
|
||||
@@ -908,6 +949,47 @@ mod tests {
|
||||
Install { root }
|
||||
}
|
||||
|
||||
/// Real junctions, in a folder whose name cmd would split on unquoted (`&`, no space so std doesn't
|
||||
/// quote it): `current` moves from one release to the other and nothing else changes.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn junctions_switch_in_a_folder_cmd_would_split() {
|
||||
let root = std::env::temp_dir().join(format!(
|
||||
"horizon-server-junction-mc&co-{}",
|
||||
std::process::id()
|
||||
));
|
||||
let _ = std::fs::remove_dir_all(&root);
|
||||
for tag in ["v1", "v2", "v%3"] {
|
||||
let release = root.join("releases").join(tag);
|
||||
std::fs::create_dir_all(&release).unwrap();
|
||||
std::fs::write(release.join(exe("horizon-server")), tag).unwrap();
|
||||
}
|
||||
let install = Install { root: root.clone() };
|
||||
let running = root.join("current").join(exe("horizon-server"));
|
||||
install.switch("v1").unwrap();
|
||||
assert_eq!(std::fs::read_to_string(&running).unwrap(), "v1");
|
||||
install.switch("v2").unwrap();
|
||||
assert_eq!(std::fs::read_to_string(&running).unwrap(), "v2");
|
||||
assert!(root.join("current.switching").symlink_metadata().is_err());
|
||||
// Replacing the junction never touched the release it pointed at.
|
||||
let v1 = root.join("releases").join("v1").join(exe("horizon-server"));
|
||||
assert_eq!(std::fs::read_to_string(v1).unwrap(), "v1");
|
||||
// `%` keeps a meaning inside cmd's quotes: refused, and `current` stays where it was.
|
||||
let refused = install.switch("v%3").unwrap_err();
|
||||
assert!(refused.contains('%'), "{refused}");
|
||||
assert_eq!(std::fs::read_to_string(&running).unwrap(), "v2");
|
||||
assert!(root.join("current.switching").symlink_metadata().is_err());
|
||||
std::fs::remove_dir_all(&root).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_junctions_paths_are_quoted_for_cmd() {
|
||||
assert_eq!(
|
||||
mklink_line(r#""D:\mc&co\current""#, r#""D:\mc&co\releases\v0.2.95""#),
|
||||
r#"/D /S /C "mklink /J "D:\mc&co\current" "D:\mc&co\releases\v0.2.95"""#
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tags_compare_by_number() {
|
||||
assert!(newer("v2026.10.19", "v2026.10.18.27"));
|
||||
|
||||
Reference in new issue
Block a user