fix: Handle complex Exec keys in desktop entries (#930)

Closes: #922, #924, #925, #928

My original patch, #891, mishandled Exec keys. The ordering of arguments
matters for the keys, such as for Flatpak apps.
This commit is contained in:
Joshua Megnauth 2025-04-10 20:19:07 -04:00 • committed by GitHub
parent d6f1efbf67
commit 5033b9dfed
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -27,14 +27,17 @@ pub fn exec_to_command(
) -> Option<Vec<process::Command>> { ) -> Option<Vec<process::Command>> {
let args_vec = shlex::split(exec)?; let args_vec = shlex::split(exec)?;
let program = args_vec.first()?; let program = args_vec.first()?;
// Skip program to make indexing easier
let args_vec = &args_vec[1..];
// Base Command instance(s) // Base Command instance(s)
// 1. We may need to launch multiple of the same process. // 1. We may need to launch multiple of the same process.
// 2. Each of those processes will need to be passed args from exec. // 2. Each of those processes will need to be passed args from exec.
// 3. Each of those args may appear in any order. // 3. Each of those args may appear in any order.
// 4. Arg order should be preserved.
// //
// So, we'll go through exec in two passes. The first pass handles paths (%f etc) while the // So, we'll go through exec in two passes. The first pass handles paths (%f etc) and args up
// second passes extra, non-% args to each processes. // to the field code followed by the second which passes extra, non-% args to each processes.
// //
// While it'd be marginally faster to process everything in one pass, that's problematic: // While it'd be marginally faster to process everything in one pass, that's problematic:
// 1. path_opt may need to be cloned because it may be moved on each iteration (borrowck // 1. path_opt may need to be cloned because it may be moved on each iteration (borrowck
@ -42,11 +45,15 @@ pub fn exec_to_command(
// 2. We have to keep track of which modifier (%f etc) we've used/seen already // 2. We have to keep track of which modifier (%f etc) we've used/seen already
// 3. We have to keep track of which processes received non-modifier args which gets messy fast // 3. We have to keep track of which processes received non-modifier args which gets messy fast
// 4. `exec` is likely small so looping over it twice is not a big deal // 4. `exec` is likely small so looping over it twice is not a big deal
let args_handler = args_vec let field_code_pos = args_vec
.iter() .iter()
.find(|arg| EXEC_HANDLERS.contains(&arg.as_str())); .position(|arg| EXEC_HANDLERS.contains(&arg.as_str()));
let args_handler = field_code_pos.and_then(|i| args_vec.get(i));
// msrv // msrv
// .inspect(|handler| log::trace!("Found paths handler: {handler} for exec: {exec}")); // .inspect(|handler| log::trace!("Found paths handler: {handler} for exec: {exec}"));
// Number of args before the field code.
// This won't be an off by one err below because take is not zero indexed.
let field_code_pos = field_code_pos.unwrap_or_default();
let mut processes = match args_handler.map(|s| s.as_str()) { let mut processes = match args_handler.map(|s| s.as_str()) {
Some("%f") => { Some("%f") => {
let mut processes = Vec::with_capacity(path_opt.len()); let mut processes = Vec::with_capacity(path_opt.len());
@ -59,7 +66,13 @@ pub fn exec_to_command(
// Passing multiple paths to %f should open an instance per path // Passing multiple paths to %f should open an instance per path
let mut process = process::Command::new(program); let mut process = process::Command::new(program);
process.arg(path); process.args(
args_vec
.iter()
.map(AsRef::as_ref)
.take(field_code_pos)
.chain(std::iter::once(path)),
);
processes.push(process); processes.push(process);
} }
@ -77,7 +90,13 @@ pub fn exec_to_command(
// Launch one instance with all args // Launch one instance with all args
let mut process = process::Command::new(program); let mut process = process::Command::new(program);
process.args(path_opt); process.args(
args_vec
.iter()
.map(OsStr::new)
.take(field_code_pos)
.chain(path_opt.iter().map(AsRef::as_ref)),
);
vec![process] vec![process]
} }
@ -85,29 +104,42 @@ pub fn exec_to_command(
.iter() .iter()
.map(|path| { .map(|path| {
let mut process = process::Command::new(program); let mut process = process::Command::new(program);
process.arg(path); process.args(
args_vec
.iter()
.map(OsStr::new)
.take(field_code_pos)
.chain(std::iter::once(path.as_ref())),
);
process process
}) })
.collect(), .collect(),
Some("%U") => { Some("%U") => {
let mut process = process::Command::new(program); let mut process = process::Command::new(program);
process.args(path_opt); process.args(
args_vec
.iter()
.map(OsStr::new)
.take(field_code_pos)
.chain(path_opt.iter().map(AsRef::as_ref)),
);
vec![process] vec![process]
} }
Some(invalid) => unreachable!("All valid variants were checked; got: {invalid}"), Some(invalid) => unreachable!("All valid variants were checked; got: {invalid}"),
None => vec![process::Command::new(program)], None => vec![process::Command::new(program)],
}; };
// Pass 2: Add every argument that's not % to each process // Pass 2: Add remaining arguments that are not % to each process
for arg in args_vec.into_iter().skip(1) { for arg in args_vec.iter().skip(field_code_pos) {
match arg.as_str() { match arg.as_str() {
unsupported // Consume path field codes or fail on codes we don't handle yet
if arg.starts_with('%') field_code if arg.starts_with('%') => {
&& !EXEC_HANDLERS.contains(&unsupported) if !EXEC_HANDLERS.contains(&field_code)
&& !DEPRECATED_HANDLERS.contains(&unsupported) => && !DEPRECATED_HANDLERS.contains(&field_code)
{ {
log::warn!("unsupported Exec code {:?} in {:?}", unsupported, exec); log::warn!("unsupported Exec code {:?} in {:?}", field_code, exec);
return None; return None;
}
} }
arg => { arg => {
for process in &mut processes { for process in &mut processes {
@ -116,6 +148,16 @@ pub fn exec_to_command(
} }
} }
} }
#[cfg(debug_assertions)]
for command in &processes {
log::debug!(
"Parsed program {} with args: {:?}",
command.get_program().to_string_lossy(),
command.get_args()
);
}
Some(processes) Some(processes)
} }