Merge pull request #1775 from pop-os/field-codes

fix: improve desktop entry field code handling
This commit is contained in:
Jeremy Soller 2026-04-30 07:49:37 -06:00 committed by GitHub
commit 506f86eff1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 184 additions and 160 deletions

1
Cargo.lock generated
View file

@ -1375,6 +1375,7 @@ version = "1.0.11"
dependencies = [
"anyhow",
"atomic_float",
"bstr",
"bzip2",
"compio",
"cosmic-client-toolkit",

View file

@ -65,6 +65,7 @@ tracing-subscriber = { version = "0.3.23", features = ["env-filter"] }
thiserror = "2.0.18"
atomic_float = "1.1.0"
num_enum = "0.7.6"
bstr = "1.12.1"
# Completion-based IO runtime to enable io_uring / IOCP file IO support.
[dependencies.compio]

View file

@ -884,31 +884,39 @@ impl App {
#[cfg(feature = "desktop")]
fn launch_desktop_entries(paths: &[impl AsRef<Path>]) {
use cosmic::desktop::fde::DesktopEntry;
let locales = cosmic::desktop::fde::get_languages_from_env();
for path in paths.iter().map(AsRef::as_ref) {
match DesktopEntry::from_path::<&str>(path, None) {
Ok(entry) => match entry.exec() {
Some(exec) => match mime_app::exec_to_command(exec, &[] as &[&str; 0]) {
Some(commands) => {
let cwd_opt = entry.desktop_entry("Path");
Some(exec) => {
match mime_app::exec_to_command(
exec,
entry.name(&locales).as_deref().unwrap_or_default(),
Some(path),
&[] as &[&str; 0],
) {
Some(commands) => {
let cwd_opt = entry.desktop_entry("Path");
for mut command in commands {
if let Some(cwd) = cwd_opt {
command.current_dir(cwd);
}
for mut command in commands {
if let Some(cwd) = cwd_opt {
command.current_dir(cwd);
}
if let Err(err) = spawn_detached(&mut command) {
log::warn!("failed to execute {}: {}", path.display(), err);
if let Err(err) = spawn_detached(&mut command) {
log::warn!("failed to execute {}: {}", path.display(), err);
}
}
}
None => {
log::warn!(
"failed to parse {}: invalid Desktop Entry/Exec",
path.display()
);
}
}
None => {
log::warn!(
"failed to parse {}: invalid Desktop Entry/Exec",
path.display()
);
}
},
}
None => {
log::warn!(
"failed to parse {}: missing Desktop Entry/Exec",

View file

@ -47,7 +47,7 @@ impl ContextActionPreset {
}
for step in &self.steps {
let Some(commands) = mime_app::exec_to_command(step, paths) else {
let Some(commands) = mime_app::exec_to_command(step, &self.name, None, paths) else {
log::warn!(
"failed to parse context action {:?}: invalid Exec {:?}",
self.name,

View file

@ -1,6 +1,7 @@
// Copyright 2023 System76 <info@system76.com>
// SPDX-License-Identifier: GPL-3.0-only
use bstr::{BString, ByteSlice, ByteVec};
#[cfg(feature = "desktop")]
use cosmic::desktop;
use cosmic::widget;
@ -8,145 +9,110 @@ pub use mime_guess::Mime;
use rustc_hash::FxHashMap;
use std::cmp::Ordering;
use std::ffi::OsStr;
use std::os::unix::ffi::OsStrExt;
use std::path::{Path, PathBuf};
use std::time::Instant;
use std::{fs, io, process};
// Supported exec key field codes
const EXEC_HANDLERS: [&str; 4] = ["%f", "%F", "%u", "%U"];
// Deprecated field codes. The spec advises to ignore these handlers.
const DEPRECATED_HANDLERS: [&str; 6] = ["%d", "%D", "%n", "%N", "%v", "%m"];
pub fn exec_to_command(
exec: &str,
entry_name: &str,
entry_path: Option<&Path>,
path_opt: &[impl AsRef<OsStr>],
) -> Option<Vec<process::Command>> {
let args_vec = shlex::split(exec)?;
let program = args_vec.first()?;
// Skip program to make indexing easier
let args_vec = &args_vec[1..];
let arguments = shlex::split(exec)?;
// Base Command instance(s)
// 1. We may need to launch multiple of the same process.
// 2. Each of those processes will need to be passed args from exec.
// 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) and args up
// 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:
// 1. path_opt may need to be cloned because it may be moved on each iteration (borrowck
// doesn't know we'll only use it once)
// 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
// 4. `exec` is likely small so looping over it twice is not a big deal
let field_code_pos = args_vec
if arguments.is_empty() {
tracing::error!("command does not contain any arguments");
return None;
}
let mut commands = Vec::new();
let paths = path_opt
.iter()
.position(|arg| EXEC_HANDLERS.contains(&arg.as_str()));
let args_handler = field_code_pos.and_then(|i| args_vec.get(i));
// msrv
// .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(String::as_str) {
Some("%f") => {
let mut processes = Vec::with_capacity(path_opt.len());
.map(AsRef::as_ref)
.map(Some)
// Add a single `None` if no path was given.
.chain(std::iter::repeat_n(
None,
if path_opt.is_empty() { 1 } else { 0 },
));
for path in path_opt.iter().map(AsRef::as_ref) {
// TODO: %f and %F need to handle non-file URLs (see spec)
if from_file_or_dir(path).is_none() {
log::warn!("Desktop file expects a file path instead of a URL: {path:?}");
for path in paths {
let mut batch_process = false;
let mut args = Vec::with_capacity(arguments.len());
let mut field_code_used = false;
for argument in arguments.iter().skip(1) {
let mut new_argument = BString::new(Vec::with_capacity(argument.capacity()));
let mut chars = argument.chars();
while let Some(char) = chars.next() {
// https://specifications.freedesktop.org/desktop-entry/latest/exec-variables.html
if char == '%' {
match chars.next() {
Some('%') => new_argument.push_char(char),
Some('c') => new_argument.push_str(entry_name),
Some('k') => {
if let Some(path) = entry_path {
new_argument.push_str(path.as_os_str().as_bytes());
}
}
// %f and %u behave the same in a file manager.
Some('f' | 'u')
if let Some(path) = path
&& !field_code_used =>
{
// TODO: files on remote file systems should be copied to a temporary local file.
batch_process = true;
field_code_used = true;
new_argument.push_str(path.as_bytes());
}
// %F and %U behave the same in a file manager.
Some('F') | Some('U') if !field_code_used && new_argument.is_empty() => {
field_code_used = true;
for path in path_opt.iter().map(AsRef::as_ref) {
args.push(BString::new(path.as_bytes().to_owned()));
}
}
_ => (),
}
} else {
new_argument.push_char(char);
}
// Passing multiple paths to %f should open an instance per path
let mut process = process::Command::new(program);
process.args(
args_vec
.iter()
.map(AsRef::as_ref)
.take(field_code_pos)
.chain(std::iter::once(path)),
);
processes.push(process);
}
processes
}
Some("%F") => {
// TODO: %f and %F need to handle non-file URLs (see spec)
for invalid in path_opt
.iter()
.map(AsRef::as_ref)
.filter(|&path| from_file_or_dir(path).is_none())
{
log::warn!("Desktop file expects a file path instead of a URL: {invalid:?}");
if !new_argument.is_empty() {
args.push(new_argument);
}
// Launch one instance with all args
let mut process = process::Command::new(program);
process.args(
args_vec
.iter()
.map(OsStr::new)
.take(field_code_pos)
.chain(path_opt.iter().map(AsRef::as_ref)),
);
vec![process]
}
Some("%u") => path_opt
.iter()
.map(|path| {
let mut process = process::Command::new(program);
process.args(
args_vec
.iter()
.map(OsStr::new)
.take(field_code_pos)
.chain(std::iter::once(path.as_ref())),
);
process
})
.collect(),
Some("%U") => {
let mut process = process::Command::new(program);
process.args(
args_vec
.iter()
.map(OsStr::new)
.take(field_code_pos)
.chain(path_opt.iter().map(AsRef::as_ref)),
);
vec![process]
}
Some(invalid) => unreachable!("All valid variants were checked; got: {invalid}"),
None => vec![process::Command::new(program)],
};
// Pass 2: Add remaining arguments that are not % to each process
for arg in args_vec.iter().skip(field_code_pos) {
match arg.as_str() {
// Consume path field codes or fail on codes we don't handle yet
field_code if arg.starts_with('%') => {
if !EXEC_HANDLERS.contains(&field_code)
&& !DEPRECATED_HANDLERS.contains(&field_code)
{
log::warn!("unsupported Exec code {field_code:?} in {exec:?}");
let mut command = process::Command::new(&arguments[0]);
for arg in args {
match arg.to_os_str() {
Ok(arg) => {
command.arg(arg);
}
Err(_) => {
tracing::error!("invalid string encoding in command");
return None;
}
}
arg => {
for process in &mut processes {
process.arg(arg);
}
}
}
commands.push(command);
if !batch_process {
break;
}
}
#[cfg(debug_assertions)]
for command in &processes {
for command in &commands {
log::debug!(
"Parsed program {} with args: {:?}",
command.get_program().to_string_lossy(),
@ -154,13 +120,7 @@ pub fn exec_to_command(
);
}
Some(processes)
}
fn from_file_or_dir(path: impl AsRef<Path>) -> Option<url::Url> {
url::Url::from_file_path(&path)
.ok()
.or_else(|| url::Url::from_directory_path(&path).ok())
Some(commands)
}
#[derive(Clone, Debug)]
@ -176,7 +136,12 @@ pub struct MimeApp {
impl MimeApp {
//TODO: move to libcosmic, support multiple files
pub fn command<O: AsRef<OsStr>>(&self, path_opt: &[O]) -> Option<Vec<process::Command>> {
exec_to_command(self.exec.as_deref()?, path_opt)
exec_to_command(
self.exec.as_deref()?,
&self.name,
self.path.as_deref(),
path_opt,
)
}
}
@ -472,11 +437,43 @@ impl Default for MimeAppCache {
mod tests {
use super::exec_to_command;
#[test]
fn keys_within_words() {
let exec = "/usr/bin/foo --option=%f";
let paths = ["file1"];
let commands = exec_to_command(exec, "keys_within_words", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
assert_eq!("/usr/bin/foo", command.get_program().to_str().unwrap());
assert_eq!(
"--option=file1",
command.get_args().next().unwrap().to_str().unwrap()
);
}
#[test]
fn no_path_f_field_code() {
let exec = "/usr/bin/foo %f";
let paths: [&str; 0] = [];
let commands = exec_to_command(exec, "no_path_f_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
assert_eq!("/usr/bin/foo", command.get_program().to_str().unwrap());
assert_eq!(0, command.get_args().len());
}
#[test]
fn one_path_f_field_code() {
let exec = "/usr/bin/foo %f";
let paths = ["file1"];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let commands = exec_to_command(exec, "one_path_f_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
@ -491,31 +488,40 @@ mod tests {
#[test]
#[allow(non_snake_case)]
fn one_path_F_field_code() {
let exec = "/usr/bin/bar %F";
let paths = ["cat"];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let exec = "/usr/bin/cosmic-term -w %F";
let paths = ["/home/user"];
let commands = exec_to_command(exec, "one_path_F_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
let mut args = command.get_args();
assert_eq!("/usr/bin/bar", command.get_program().to_str().unwrap());
assert_eq!("cat", command.get_args().next().unwrap().to_str().unwrap());
assert_eq!(
"/usr/bin/cosmic-term",
command.get_program().to_str().unwrap()
);
assert_eq!("-w", args.next().unwrap().to_str().unwrap());
assert_eq!(paths[0], args.next().unwrap().to_str().unwrap());
}
#[test]
fn one_path_u_field_code() {
let exec = "/usr/bin/foobar %u";
let paths = ["/home/josh/krumpli"];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let exec = "/usr/bin/cosmic-term -w %u";
let paths = ["/home/user"];
let commands = exec_to_command(exec, "one_path_u_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
let mut args = command.get_args();
assert_eq!("/usr/bin/foobar", command.get_program().to_str().unwrap());
assert_eq!(
*paths.first().unwrap(),
command.get_args().next().unwrap().to_str().unwrap()
"/usr/bin/cosmic-term",
command.get_program().to_str().unwrap()
);
assert_eq!("-w", args.next().unwrap().to_str().unwrap());
assert_eq!(paths[0], args.next().unwrap().to_str().unwrap());
}
#[test]
@ -523,7 +529,8 @@ mod tests {
fn one_path_U_field_code() {
let exec = "/usr/bin/rmrfbye %U";
let paths = ["/"];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let commands = exec_to_command(exec, "one_path_U_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
@ -539,7 +546,8 @@ mod tests {
"/usr/share/games/psp/miku.iso",
"/usr/share/games/psp/eternia.iso",
];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let commands = exec_to_command(exec, "mult_path_f_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(paths.len(), commands.len());
for (command, path) in commands.into_iter().zip(paths.iter()) {
@ -559,7 +567,8 @@ mod tests {
"/usr/share/games/doom2/hr.wad",
"/usr/share/games/doom2/hrmus.wad",
];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let commands = exec_to_command(exec, "mult_path_F_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
@ -581,7 +590,8 @@ mod tests {
"https://redox-os.org/",
"https://system76.com/",
];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let commands = exec_to_command(exec, "mult_path_u_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(paths.len(), commands.len());
for (command, path) in commands.into_iter().zip(paths.iter()) {
@ -604,7 +614,8 @@ mod tests {
"frieren01.mkv",
"rtmp://example.org/this/video/doesnt/exist.avi",
];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let commands = exec_to_command(exec, "mult_path_U_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
@ -632,7 +643,8 @@ mod tests {
"@@u",
];
let paths = ["file1.rs", "file2.rs"];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let commands = exec_to_command(exec, "flatpak_style_exec", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
@ -655,7 +667,8 @@ mod tests {
"file:///usr/share/games/roguelike/mods/mod1",
"file:///usr/share/games/roguelike/mods/mod2",
];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let commands = exec_to_command(exec, "multiple_field_codes", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();
@ -688,7 +701,8 @@ mod tests {
];
let paths = ["rust_game_dev.pdf", "superhero_ferris.epub"];
let args_trailing = ["@@"];
let commands = exec_to_command(exec, &paths).expect("Should parse valid exec");
let commands = exec_to_command(exec, "sandwiched_field_code", None, &paths)
.expect("Should parse valid exec");
assert_eq!(1, commands.len());
let command = commands.first().unwrap();