Files
netris-nestri/apps/nesdoctor/src/report.rs
Wanjohi 786267f30a fix(nesdoctor): report storage properly, and stop being blind on Windows
A submission from a team machine with four drives and 22 TiB reported
`disk=8880`, and the field was not wrong so much as meaningless: it was the
free space on the single largest mount, with no capacity anywhere and no total.
A content store is sized against capacity.

Storage now reports four things, because they answer different questions and
one number could not:

  diskfree   total free across every real filesystem
  disksize   total capacity
  diskmax    the largest single filesystem, which is the real ceiling for any
             one store -- a dataset cannot be spread across drives
  disks      how many there are

The ambiguous `disk` key is gone rather than silently redefined, so old rows
stay readable as what they were. `Get-PSDrive` reports Free *and* Used and we
were reading only Free, hence no capacity on Windows at all.

Pseudo-filesystems are now excluded by *type* rather than by mount path. Path
filtering missed `/tmp` on a tmpfs, whose free space is RAM -- so 7 GiB of
memory was being added to a storage total, which is exactly the sort of number
a capacity plan gets built on.

## The real finding, which was not about disks

"We are working blind on Windows" is correct, and both Windows bugs this tool
has had prove it: a virtual display adapter reported as the GPU, and a URL
truncated at its first `&`. Both were in code that only runs on Windows, both
were found by a person reading the results channel, and neither could have been
found here -- the development machine is Linux and `xdg-open` never sees a
shell.

Two things about that, and the first is the one that generalises.

`OPENERS` is now a const with a test asserting the property that actually
matters: **never hand a URL to anything that will re-parse it.** No `cmd`, no
`sh`, no `powershell`, no `start` builtin, and no argument that looks like it
wants the URL interpolated into it. Unlike the bug, that is checkable on every
platform in a millisecond. Verified by reintroducing `cmd /C start "" <url>`
and confirming the test fails with the right message, then reverting.

And CI already runs a real Windows machine and a real macOS one -- we simply
were not looking at them. Each smoke-tested target now prints its full report
and JSON into a collapsed log group. Deliberately not `set -e`: this step is
for looking, and a probe that misbehaves on a runner must not fail a release.
It turns "working blind" into "looking at it once per release", which would
have shown the Parsec adapter problem the first time a Windows binary was ever
built.

Version to 0.2.2.
2026-09-02 16:10:05 +03:00

775 lines
28 KiB
Rust
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
//! The verdict, the printed report, and the one line somebody pastes.
//!
//! Two audiences and they want different things. The person running this wants
//! to know whether their machine is any good and what to fix. We want the
//! distribution. The summary line is the only thing that crosses over, and it
//! is built to be legible to both: a human can read it, and it parses.
//!
//! # What is not in the line
//!
//! No IP address, no hostname, no username, no game titles, no file paths, no
//! machine identifier of any kind. A size *band* rather than a size, and an
//! hour histogram rather than timestamps. The full JSON — which does contain
//! titles and paths — stays on the local disk, and the person is told where.
//!
//! That is not politeness. A line that people are comfortable pasting in public
//! is a line that gets pasted, and one that quietly carries their hostname gets
//! screenshotted once and then never again.
use serde::Serialize;
use crate::ask::Answers;
use crate::hostreq::{HostReport, State};
use crate::net::NetReport;
use crate::steam::{self, SteamReport};
use crate::sys::SysInfo;
pub const VERSION: &str = env!("CARGO_PKG_VERSION");
#[derive(Debug, Serialize)]
pub struct Full<'a> {
pub nesdoctor: &'static str,
pub sys: &'a SysInfo,
pub display: &'a crate::display::DisplayReport,
pub host: &'a HostReport,
pub net: &'a NetReport,
pub steam: &'a SteamReport,
pub answers: &'a Answers,
pub verdict: Verdict,
pub region_hint: Option<String>,
}
/// What this machine is, in one word, plus why.
#[derive(Debug, Serialize, Clone, Copy, PartialEq, Eq)]
#[serde(rename_all = "kebab-case")]
pub enum Verdict {
/// Passes every blocking host check, and the uplink is good enough.
HostReady,
/// Hardware and software are fine; the network is the problem.
HostBlockedByNetwork,
/// Every check passes, the uplink is clean — and the machine is a long way
/// from the rest of the internet, so it can only usefully serve players
/// near it. Which is the most interesting result this tool produces.
HostReadyLocalOnly,
/// Could host with setup work — nothing missing that cannot be installed.
HostFixable,
/// Cannot host. Usually the GPU vendor or the OS.
ClientOnly,
Unknown,
}
impl Verdict {
pub fn tag(self) -> &'static str {
match self {
Verdict::HostReady => "HOST-READY",
Verdict::HostReadyLocalOnly => "HOST-READY-LOCAL",
Verdict::HostBlockedByNetwork => "HOST-NET",
Verdict::HostFixable => "HOST-FIXABLE",
Verdict::ClientOnly => "CLIENT",
Verdict::Unknown => "UNKNOWN",
}
}
}
/// The uplink thresholds a host has to clear.
///
/// Both matter and the second matters more: 1080p60 needs 1015 Mbps, and
/// almost every fibre peer clears that, but added latency under load is what
/// actually disqualifies a machine. See `net::grade`.
const MIN_UP_MBPS: f64 = 15.0;
const MAX_BLOAT_MS: f64 = 30.0;
/// Idle round trip to the nearest anycast edge, above which this machine can
/// only serve players close to it.
///
/// This is a *floor* on what any player will see, not a measure of the machine.
/// The network allowance is ~40 ms in total, so a host already spending 178 ms
/// to reach Cloudflare's nearest point of presence cannot serve anyone who is
/// not on roughly its own local networks — and no upgrade changes that, because
/// it is distance.
///
/// Measured on the development connection 2026-09-02: 178 ms idle, served from
/// Johannesburg. That machine passes every other check and still cannot host
/// for a European player — which is the coverage argument seen from the other
/// end: the places with no nearby edge are the places a local host is the only
/// option anyone has.
const FAR_RTT_MS: f64 = 60.0;
pub fn verdict(sys: &SysInfo, host: &HostReport, net: &NetReport) -> Verdict {
// A machine that cannot host is a client, and that is a complete answer —
// not a failure. Most respondents will land here and the wording matters.
if !host.could_host {
// Distinguish "wrong hardware" from "missing setup": an AMD or Intel
// card with a render node and KVM is a fixable machine.
let fixable = sys.os == "linux"
&& host
.checks
.iter()
.filter(|c| c.blocking && c.state == State::Fail)
.all(|c| c.id == "vkvideo");
return if fixable {
Verdict::HostFixable
} else {
Verdict::ClientOnly
};
}
if host.unknowns > 0 {
return Verdict::Unknown;
}
match (net.upstream_mbps, net.bloat_ms) {
(Some(up), Some(bloat)) => {
if up < MIN_UP_MBPS || bloat > MAX_BLOAT_MS {
Verdict::HostBlockedByNetwork
// Deliberately the *median* and not the floor. Bloat asks "how much
// queueing is added", so its baseline is the best the path can do.
// This asks "what will a player actually see", so it takes the
// typical case -- and on a link whose idle latency is bimodal
// between 56 ms and 180 ms, the floor would call it near when half
// of all connections are not.
} else if net
.idle_rtt_p50_ms
.or(net.idle_rtt_ms)
.is_some_and(|r| r > FAR_RTT_MS)
{
Verdict::HostReadyLocalOnly
} else {
Verdict::HostReady
}
}
_ => Verdict::Unknown,
}
}
/// The line to paste. Pipe-separated fields, `k=v` inside, stable key order.
/// Both renderers take the assembled report rather than seven arguments: the
/// set of things they need is exactly [`Full`], and keeping them in step with
/// it is the point.
pub fn summary_line(f_: &Full) -> String {
let (sys, host, net, steam, answers, verdict, region) = (
f_.sys,
f_.host,
f_.net,
f_.steam,
f_.answers,
f_.verdict,
&f_.region_hint,
);
let mut f: Vec<String> = Vec::new();
f.push(format!("nesdoctor {VERSION}"));
f.push(format!("{}/{}", sys.os, sys.arch));
let gpu = sys
.gpus
.iter()
.find(|g| g.render_node.is_some())
.or_else(|| sys.gpus.first());
f.push(format!(
"gpu={}",
gpu.map(|g| g.name.as_str()).unwrap_or("unknown")
));
if let Some(r) = sys.ram_gib {
f.push(format!("cpu={}t ram={r:.0}G", sys.cpu_threads));
}
if sys.os == "linux" {
let st = |id: &str| {
host.checks
.iter()
.find(|c| c.id == id)
.map(|c| match c.state {
State::Pass => "y",
State::Fail => "n",
State::Unknown => "?",
})
.unwrap_or("-")
};
f.push(format!(
"kvm={} venc={} zfs={} boxfs={} io={}",
st("kvm"),
st("vkvideo"),
st("content-store"),
st("box-store"),
st("cgroup-io")
));
}
match (net.upstream_mbps, net.bloat_ms, net.grade) {
(Some(up), Some(b), Some(g)) => f.push(format!(
"up={up:.0}Mbps rtt={}/{}ms bloat=+{b:.0}ms grade={g}",
net.idle_rtt_ms.map_or("?".into(), |r| format!("{r:.0}")),
net.idle_rtt_p50_ms
.map_or("?".into(), |r| format!("{r:.0}"))
)),
_ => f.push("net=unmeasured".into()),
}
// Total free, and the largest single filesystem, because one dataset
// cannot span drives.
let free_total: f64 = sys.disks.iter().map(|d| d.free_gib).sum();
if free_total > 0.0 {
let max = sys.disks.first().map_or(0.0, |d| d.free_gib);
f.push(if sys.disks.len() > 1 {
format!(
"disk={free_total:.0}G free/{max:.0}G largest x{}",
sys.disks.len()
)
} else {
format!("disk={free_total:.0}G")
});
}
if let (Some(h), Some(days)) = (sys.powered_hours_per_day, sys.powered_span_days) {
f.push(format!("powered={h:.0}h/d over {days:.0}d"));
}
if let Some(r) = region {
f.push(format!("edge={r}"));
}
if steam.found && steam.titles > 0 {
f.push(format!(
"steam={} titles/{}",
steam.titles,
steam::size_band(steam.bytes_on_disk)
));
if let Some((s, e)) = steam.peak_window {
f.push(format!("plays={s:02}-{e:02}h n={}", steam.launch_samples));
}
}
let a = |o: &Option<String>| o.clone().unwrap_or_else(|| "-".into());
f.push(format!(
"role={} share={} pays={}",
a(&answers.role),
a(&answers.share_for),
a(&answers.pays_today)
));
if let Some(o) = &answers.other_linux {
f.push(format!("otherlinux={o}"));
}
f.push(verdict.tag().to_string());
f.join(" | ")
}
// ------------------------------------------------------------------ output ---
pub fn print_checks(host: &HostReport) {
println!("\n\x1b[1mCan this machine run a Nestri box?\x1b[0m");
for c in &host.checks {
let (mark, colour) = match c.state {
State::Pass => ("ok ", "32"),
State::Fail => ("no ", "31"),
State::Unknown => ("? ", "33"),
};
println!(" \x1b[{colour}m{mark}\x1b[0m {}", c.what);
if !c.detail.is_empty() {
for line in wrap(&c.detail, 68) {
println!(" \x1b[2m{line}\x1b[0m");
}
}
}
}
pub fn print_verdict(v: Verdict, net: &NetReport) {
println!();
let (colour, headline, body) = match v {
Verdict::HostReady => (
"32",
"This machine could host.",
"Every hard requirement passes and the uplink is good enough. That is rarer \
than it sounds — most machines fail on the encode extension or on queueing.",
),
Verdict::HostBlockedByNetwork => (
"33",
"Good machine, the network is in the way.",
"The hardware and the software are fine. See the uplink figures above — if the \
problem is added latency rather than throughput, it is a router setting and not \
a line you need to upgrade.",
),
Verdict::HostReadyLocalOnly => (
"32",
"This machine could host — for players near you.",
"Every requirement passes and your uplink queues cleanly. But the idle round trip to the nearest major network is already most of the latency budget, and that is distance rather than a fault: no upgrade shortens it. So this machine is a good host for people on your side of the world and cannot be one for anybody else. If you are somewhere without a cloud gaming edge, that is not a consolation prize — it is the only way anyone there gets a playable stream.",
),
Verdict::HostFixable => (
"33",
"This machine could host, with some setup.",
"Nothing here is a hardware limit — what is missing can be installed.",
),
Verdict::ClientOnly => (
"36",
"This is a client, not a host.",
"Which is a complete answer and not a failure: most machines are clients, and \
the thing you would actually use Nestri for works fine from here.",
),
Verdict::Unknown => (
"33",
"Inconclusive.",
"One or more checks could not be run rather than failing. The report says which; \
an unknown is not a no.",
),
};
println!("\x1b[1;{colour}m{headline}\x1b[0m");
for line in wrap(body, 72) {
println!("\x1b[2m{line}\x1b[0m");
}
if !net.note.is_empty() {
println!();
for line in wrap(&net.note, 72) {
println!("\x1b[33m{line}\x1b[0m");
}
}
}
/// Wrap on whitespace. Twelve lines rather than a dependency, per `Cargo.toml`.
fn wrap(s: &str, width: usize) -> Vec<String> {
let mut out = vec![String::new()];
for word in s.split_whitespace() {
let cur = out.last_mut().unwrap();
if !cur.is_empty() && cur.chars().count() + 1 + word.chars().count() > width {
out.push(word.to_string());
} else {
if !cur.is_empty() {
cur.push(' ');
}
cur.push_str(word);
}
}
out.retain(|l| !l.is_empty());
out
}
/// Put the summary line on the clipboard, and say which tool did it.
///
/// Selecting a long line out of a terminal is fiddly and it is the last step
/// before we learn anything, so it should not be work. Every one of these ships
/// with the desktop it belongs to; where none is present we simply say so and
/// the line is still on screen.
pub fn to_clipboard(line: &str) -> Option<&'static str> {
use std::io::Write;
use std::process::{Command, Stdio};
const TOOLS: [(&str, &[&str]); 5] = [
("wl-copy", &[]), // Wayland
("xclip", &["-selection", "clipboard"]), // X11
("xsel", &["--clipboard", "--input"]), // X11, the other one
("pbcopy", &[]), // macOS
("clip", &[]), // Windows
];
for (tool, args) in TOOLS {
let Ok(mut child) = Command::new(tool)
.args(args)
.stdin(Stdio::piped())
.stdout(Stdio::null())
.stderr(Stdio::null())
.spawn()
else {
continue;
};
let wrote = child
.stdin
.as_mut()
.is_some_and(|s| s.write_all(line.as_bytes()).is_ok());
// Wait either way, so a failed tool is not left running.
let ok = child.wait().map(|s| s.success()).unwrap_or(false);
if wrote && ok {
return Some(tool);
}
}
None
}
// ------------------------------------------------------------------ submit ---
/// Percent-encode everything that is not unreserved. Small enough to write.
fn enc(s: &str) -> String {
let mut out = String::with_capacity(s.len());
for b in s.bytes() {
match b {
b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => {
out.push(b as char)
}
_ => out.push_str(&format!("%{b:02X}")),
}
}
out
}
/// The URL that submits this run.
///
/// Query parameters rather than an opaque blob, deliberately. A base64 payload
/// would be shorter and would let us send more, and it would also mean the
/// person clicking cannot read what they are sending — which is the one thing
/// this program has going for it. Readable parameters are self-documenting, and
/// the length is nowhere near a browser limit.
///
/// Carries more than the clipboard line does, because it is not something
/// anyone has to eyeball in a chat window: every check individually, the full
/// latency triple, the 24-hour launch histogram, and the five largest titles.
pub fn submit_url(base: &str, f_: &Full) -> String {
let (sys, host, net, steam, answers, verdict, region) = (
f_.sys,
f_.host,
f_.net,
f_.steam,
f_.answers,
f_.verdict,
&f_.region_hint,
);
let mut q: Vec<String> = Vec::new();
let mut put = |k: &str, v: String| q.push(format!("{k}={}", enc(&v)));
put("v", VERSION.to_string());
put("os", format!("{}/{}", sys.os, sys.arch));
if let Some(rel) = &sys.release {
put("rel", rel.clone());
}
if let Some(g) = sys
.gpus
.iter()
.find(|g| g.render_node.is_some())
.or_else(|| sys.gpus.first())
{
put("gpu", g.name.clone());
}
if sys.gpus.len() > 1 {
put("gpus", sys.gpus.len().to_string());
// Every adapter, not just the count. The first Windows submission
// reported `gpus=2` with a virtual adapter as the primary, which told
// us something had been lost but not what.
put(
"gpulist",
sys.gpus
.iter()
.map(|g| g.name.as_str())
.collect::<Vec<_>>()
.join("~"),
);
}
put("cpu", sys.cpu_threads.to_string());
if let Some(m) = &sys.cpu_model {
put("cpumodel", m.clone());
}
if let Some(r) = sys.ram_gib {
put("ram", format!("{r:.0}"));
}
// Every check, individually — the aggregate verdict hides which single
// requirement stops people, which is the thing worth knowing.
for c in &host.checks {
// Prefixed: the `gpu` check id would otherwise overwrite the GPU model
// parameter, and last-writer-wins in a query string is a silent loss.
put(
&format!("ck_{}", c.id),
match c.state {
State::Pass => "y",
State::Fail => "n",
State::Unknown => "?",
}
.to_string(),
);
}
if let Some(u) = net.upstream_mbps {
put("up", format!("{u:.1}"));
}
if let Some(v) = net.idle_rtt_ms {
put("rtt", format!("{v:.0}"));
}
if let Some(v) = net.idle_rtt_p50_ms {
put("rttidle50", format!("{v:.0}"));
}
if let Some(v) = net.loaded_rtt_ms {
put("rttload", format!("{v:.0}"));
}
if let Some(v) = net.loaded_rtt_p95_ms {
put("rttp95", format!("{v:.0}"));
}
if let Some(v) = net.bloat_ms {
put("bloat", format!("{v:.0}"));
}
if let Some(g) = net.grade {
put("grade", g.to_string());
}
if let Some(r) = region {
put("edge", r.clone());
}
// Four fields rather than one, because `disk=8880` was ambiguous and wrong
// for its purpose. A submission from a four-drive, 22 TiB machine reported
// the free space on its single largest mount and nothing else.
//
// Both the totals and the largest single mount matter, and they answer
// different questions: a content store is sized against total capacity,
// but one dataset cannot be spread across drives, so the largest single
// filesystem is the real ceiling for any one store. The old ambiguous
// `disk` key is gone rather than silently redefined.
let free_total: f64 = sys.disks.iter().map(|d| d.free_gib).sum();
let size_total: f64 = sys.disks.iter().filter_map(|d| d.size_gib).sum();
if free_total > 0.0 {
put("diskfree", format!("{free_total:.0}"));
}
if size_total > 0.0 {
put("disksize", format!("{size_total:.0}"));
}
if let Some(d) = sys.disks.first() {
put("diskmax", format!("{:.0}", d.free_gib));
if let Some(fs) = &d.fs {
put("diskfs", fs.clone());
}
}
put("disks", sys.disks.len().to_string());
if let (Some(h), Some(s)) = (sys.powered_hours_per_day, sys.powered_span_days) {
put("powered", format!("{h:.1}"));
put("span", format!("{s:.0}"));
}
if steam.found {
put("titles", steam.titles.to_string());
put("gib", format!("{:.0}", steam::gib(steam.bytes_on_disk)));
if steam.launch_samples > 0 {
put(
"hours",
steam
.launch_hours
.iter()
.map(u32::to_string)
.collect::<Vec<_>>()
.join(","),
);
put("n", steam.launch_samples.to_string());
put("profiles", steam.profiles.to_string());
if steam.launches_uninstalled > 0 {
put("ngone", steam.launches_uninstalled.to_string());
}
}
if let Some((a, b)) = steam.peak_window {
put("peak", format!("{a}-{b}"));
}
if !steam.largest.is_empty() {
// Whether the title distribution has a head decides whether a depot
// cache is worth building at all, and it cannot be seen from counts.
put(
"top",
steam
.largest
.iter()
.map(|(n, _)| n.as_str())
.collect::<Vec<_>>()
.join("~"),
);
}
}
for (k, v) in [
("want", &answers.want),
("role", &answers.role),
("share", &answers.share_for),
("pays", &answers.pays_today),
("otherlinux", &answers.other_linux),
] {
if let Some(v) = v {
put(k, v.clone());
}
}
// Display and decode. These are what decide the colour and codec choices
// on the wire -- until now every one of them was made against the single
// panel in one room.
let d = f_.display;
if let Some(s) = &d.session {
put("session", s.clone());
}
if let Some(c) = &d.compositor {
put("wm", c.clone());
}
if d.xwayland {
put("xwayland", "1".into());
}
if let Some(o) = d.outputs.first() {
if let (Some(w), Some(h)) = (o.width, o.height) {
put("mode", format!("{w}x{h}"));
}
if let Some(r) = o.refresh_hz {
put("hz", format!("{r:.0}"));
}
if let Some(b) = o.bit_depth {
put("bpc", b.to_string());
}
if !o.eotf.is_empty() {
put("eotf", o.eotf.join(","));
}
if !o.bt2020.is_empty() {
put("bt2020", o.bt2020.join(","));
}
put(
"chroma",
match (o.ycbcr420, o.ycbcr444) {
(true, true) => "420+444",
(true, false) => "420",
(false, true) => "444",
(false, false) => "-",
}
.into(),
);
}
if d.outputs.len() > 1 {
put("outputs", d.outputs.len().to_string());
}
if !d.decode.vulkan.is_empty() {
put("vkdec", d.decode.vulkan.join(","));
}
if !d.decode.vaapi.is_empty() {
put("vadec", d.decode.vaapi.join(","));
}
if let Some(e) = &answers.email {
put("email", e.clone());
}
put("verdict", verdict.tag().to_string());
format!("{}/?{}", base.trim_end_matches('/'), q.join("&"))
}
/// Plain English list of what the submit URL contains, printed before it opens.
///
/// The URL is readable, but it is also 800 characters long and nobody reads
/// 800 characters. This is the honest summary of it.
pub fn submit_contents(steam: &SteamReport, answers: &Answers) -> Vec<&'static str> {
let mut v = vec![
"this machine's OS, CPU, RAM and GPU model",
"your display's resolution, refresh rate, colour depth and HDR support",
"which host requirements passed and which did not",
"the network figures you just saw",
"free disk space, and how long this machine tends to stay on",
];
if steam.found && steam.titles > 0 {
v.push("how many games are installed, their total size, and your five largest");
if steam.launch_samples > 0 {
v.push("the hour-of-day histogram above — hours, never dates");
}
}
if answers.want.is_some()
|| answers.role.is_some()
|| answers.share_for.is_some()
|| answers.pays_today.is_some()
{
v.push("your answers to the questions");
}
// Reworded when the email field landed. The old line said "no username, no
// identifiers", which stopped being true, and leaving it up would have been
// the dishonest option.
if answers.email.is_some() {
v.push("the email address you just typed — the only identifying thing here");
}
v.push("no hostname, no IP address, no file paths, and nothing about your account");
v
}
/// Every program we will hand a URL to, in order.
///
/// A `const` rather than a local, so the tests at the bottom of this file can
/// assert the property that actually matters about this list.
const OPENERS: [(&str, &[&str]); 4] = [
("xdg-open", &[]),
("open", &[]), // macOS
("rundll32", &["url.dll,FileProtocolHandler"]), // Windows
("wslview", &[]), // WSL, where xdg-open is often absent
];
/// Hand a URL to whatever the desktop uses to open links.
pub fn open_in_browser(url: &str) -> bool {
use std::process::{Command, Stdio};
// NEVER route this through `cmd`.
//
// The Windows arm used to be `cmd /C start "" <url>` and it destroyed every
// Windows submission we received. `cmd.exe` re-parses its own command line
// and treats `&` as a command separator; Rust's `Command` quotes arguments
// for the MSVC C runtime convention, which `cmd` does not honour. So a URL
// is cut at its first `&` -- which in ours falls immediately after `v=` --
// and the browser opened `https://doctor.nestri.io/?v=0.2.0` carrying
// nothing else at all.
//
// Silently, too: the worker saw a version, accepted it, and thanked the
// person for a submission that contained one field. Two arrived like that
// before anyone noticed.
//
// `rundll32 url.dll,FileProtocolHandler` hands the URL to the shell's
// protocol handler without any command interpreter in the path, so nothing
// re-parses it. `explorer.exe` also works and returns a non-zero exit
// status even on success, which would make the caller think it failed.
for (cmd, args) in OPENERS {
if Command::new(cmd)
.args(args)
.arg(url)
.stdout(Stdio::null())
.stderr(Stdio::null())
.status()
.map(|s| s.success())
.unwrap_or(false)
{
return true;
}
}
false
}
#[cfg(test)]
mod tests {
use super::OPENERS;
/// The regression test for the worst bug this program has had.
///
/// `cmd /C start "" <url>` destroyed every Windows submission for a day.
/// `cmd.exe` re-parses its own command line and treats `&` as a command
/// separator, so the URL was cut at its first one — immediately after
/// `v=` — and the browser opened a link carrying a version number and
/// nothing else, behind a thank-you page.
///
/// It was found by a person reading the results channel, because the
/// developer machine is Linux and `xdg-open` never sees a shell. The
/// property that prevents the whole class is **never hand a URL to
/// anything that will re-parse it** — and unlike the bug, that is
/// checkable on every platform, in a millisecond, forever.
#[test]
fn no_opener_goes_through_a_command_interpreter() {
const INTERPRETERS: [&str; 8] = [
"cmd",
"cmd.exe",
"sh",
"bash",
"zsh",
"powershell",
"powershell.exe",
"pwsh",
];
for (cmd, args) in OPENERS {
assert!(
!INTERPRETERS.contains(&cmd),
"{cmd} re-parses its arguments; a URL containing & will be truncated"
);
// `start` exists only as a cmd builtin, so seeing it means a shell
// is involved even when the program name looks innocent.
assert!(
!args.contains(&"start"),
"{cmd} {args:?} looks like a shell invocation"
);
}
}
/// The URL is always passed as its own argument, never interpolated into
/// one — the other half of the same property.
#[test]
fn openers_take_fixed_arguments_only() {
for (_, args) in OPENERS {
for a in args {
assert!(
!a.contains("{}") && !a.contains('&') && !a.contains('?'),
"argument {a:?} looks like it wants the URL interpolated into it"
);
}
}
}
}