From a7eeb14de5f6df39254b111a6c440d768af888e4 Mon Sep 17 00:00:00 2001 From: Wanjohi Date: Wed, 2 Sep 2026 15:50:47 +0300 Subject: [PATCH] fix(nesdoctor): cmd re-parsed the submit URL and destroyed every Windows result Two submissions arrived carrying `v=0.2.0` and nothing else. Flagged from the channel, not caught by us. The Windows arm of `open_in_browser` was `cmd /C start "" `. `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 the URL was 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 whatsoever. Reproduced exactly with the same mechanism in a POSIX shell: `sh -c 'echo '` unquoted prints precisely that prefix. Every Windows user who pressed Enter lost their entire report, and lost it silently -- the page returned 200 and thanked them. Windows is most of this audience, so most of the data we would ever have collected was going to disappear this way. Now `rundll32 url.dll,FileProtocolHandler`, which hands the URL to the shell's protocol handler with no command interpreter anywhere in the path, so nothing re-parses it. `explorer.exe` also opens URLs and was rejected: it returns a non-zero exit status even on success, which would make the caller believe it had failed and fall through. The relay now also refuses to thank anyone for a version-only arrival, since an older binary keeps producing them and a URL pasted into a shell unquoted does the same thing. Version to 0.2.1. --- Cargo.lock | 2 +- apps/nesdoctor/Cargo.toml | 2 +- apps/nesdoctor/src/report.rs | 25 ++++++++++++++++++++++--- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a65c0717..203ef211 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2462,7 +2462,7 @@ dependencies = [ [[package]] name = "nesdoctor" -version = "0.2.0" +version = "0.2.1" dependencies = [ "anyhow", "clap", diff --git a/apps/nesdoctor/Cargo.toml b/apps/nesdoctor/Cargo.toml index 8764ab34..98e0986d 100644 --- a/apps/nesdoctor/Cargo.toml +++ b/apps/nesdoctor/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nesdoctor" -version = "0.2.0" +version = "0.2.1" edition.workspace = true license.workspace = true repository.workspace = true diff --git a/apps/nesdoctor/src/report.rs b/apps/nesdoctor/src/report.rs index a5cc23fd..9daba80e 100644 --- a/apps/nesdoctor/src/report.rs +++ b/apps/nesdoctor/src/report.rs @@ -641,11 +641,30 @@ pub fn submit_contents(steam: &SteamReport, answers: &Answers) -> Vec<&'static s /// 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 "" ` 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. let attempts: [(&str, &[&str]); 4] = [ ("xdg-open", &[]), - ("open", &[]), // macOS - ("cmd", &["/C", "start", ""]), // Windows - ("wslview", &[]), // WSL, where xdg-open is often absent + ("open", &[]), // macOS + ("rundll32", &["url.dll,FileProtocolHandler"]), // Windows + ("wslview", &[]), // WSL, where xdg-open is often absent ]; for (cmd, args) in attempts { if Command::new(cmd)