fix: use std::env::args_os instead of std::env::args (#1698)

Apparently `std::env::args()` will panic during iteration if any
argument to the process is not valid Unicode:

https://doc.rust-lang.org/std/env/fn.args.html

Let's avoid the risk and just go with `std::env::args_os()`.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/1698).
* #1705
* #1703
* #1702
* __->__ #1698
* #1697
This commit is contained in:
Michael Bolin
2025-07-28 08:52:18 -07:00
committed by GitHub
parent 9102255854
commit fcd197d596
4 changed files with 13 additions and 5 deletions

View File

@@ -30,7 +30,7 @@ where
Fut: Future<Output = anyhow::Result<()>>, Fut: Future<Output = anyhow::Result<()>>,
{ {
// Determine if we were invoked via the special alias. // Determine if we were invoked via the special alias.
let argv0 = std::env::args().next().unwrap_or_default(); let argv0 = std::env::args_os().next().unwrap_or_default();
let exe_name = Path::new(&argv0) let exe_name = Path::new(&argv0)
.file_name() .file_name()
.and_then(|s| s.to_str()) .and_then(|s| s.to_str())

View File

@@ -8,6 +8,7 @@
use std::collections::HashMap; use std::collections::HashMap;
use std::collections::HashSet; use std::collections::HashSet;
use std::ffi::OsString;
use std::time::Duration; use std::time::Duration;
use anyhow::Context; use anyhow::Context;
@@ -127,7 +128,12 @@ impl McpConnectionManager {
join_set.spawn(async move { join_set.spawn(async move {
let McpServerConfig { command, args, env } = cfg; let McpServerConfig { command, args, env } = cfg;
let client_res = McpClient::new_stdio_client(command, args, env).await; let client_res = McpClient::new_stdio_client(
command.into(),
args.into_iter().map(OsString::from).collect(),
env,
)
.await;
match client_res { match client_res {
Ok(client) => { Ok(client) => {
// Initialize the client. // Initialize the client.

View File

@@ -10,6 +10,7 @@
//! program. The utility connects, issues a `tools/list` request and prints the //! program. The utility connects, issues a `tools/list` request and prints the
//! server's response as pretty JSON. //! server's response as pretty JSON.
use std::ffi::OsString;
use std::time::Duration; use std::time::Duration;
use anyhow::Context; use anyhow::Context;
@@ -37,7 +38,7 @@ async fn main() -> Result<()> {
.try_init(); .try_init();
// Collect command-line arguments excluding the program name itself. // Collect command-line arguments excluding the program name itself.
let mut args: Vec<String> = std::env::args().skip(1).collect(); let mut args: Vec<OsString> = std::env::args_os().skip(1).collect();
if args.is_empty() || args[0] == "--help" || args[0] == "-h" { if args.is_empty() || args[0] == "--help" || args[0] == "-h" {
eprintln!("Usage: mcp-client <program> [args..]\n\nExample: mcp-client codex-mcp-server"); eprintln!("Usage: mcp-client <program> [args..]\n\nExample: mcp-client codex-mcp-server");

View File

@@ -12,6 +12,7 @@
//! issue requests and receive strongly-typed results. //! issue requests and receive strongly-typed results.
use std::collections::HashMap; use std::collections::HashMap;
use std::ffi::OsString;
use std::sync::Arc; use std::sync::Arc;
use std::sync::atomic::AtomicI64; use std::sync::atomic::AtomicI64;
use std::sync::atomic::Ordering; use std::sync::atomic::Ordering;
@@ -82,8 +83,8 @@ impl McpClient {
/// Caller is responsible for sending the `initialize` request. See /// Caller is responsible for sending the `initialize` request. See
/// [`initialize`](Self::initialize) for details. /// [`initialize`](Self::initialize) for details.
pub async fn new_stdio_client( pub async fn new_stdio_client(
program: String, program: OsString,
args: Vec<String>, args: Vec<OsString>,
env: Option<HashMap<String, String>>, env: Option<HashMap<String, String>>,
) -> std::io::Result<Self> { ) -> std::io::Result<Self> {
let mut child = Command::new(program) let mut child = Command::new(program)