From efd7c9fc245d1660388208a37df3d3b5036584ba Mon Sep 17 00:00:00 2001 From: lordwotton29 <57874567+lordwotton29@users.noreply.github.com> Date: Tue, 10 Nov 2026 00:24:22 +0100 Subject: [PATCH] fix(print): fall back to the lpoptions listing when the PPD is unreadable (#4699) (#4703) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(print): fall back to the lpoptions listing when the PPD is unreadable cupsd keeps /etc/cups/ppd/.ppd at 0640 root:cups on Arch and similar setups, so an ordinary desktop user cannot open it. The option reader treated that permission error as "no options", and Print ▸ Properties… said "This printer offers no settings of its own" for a queue whose driver options cupsd was serving all along through lpoptions. When the PPD is missing, unreadable, or parses to no options, the list is now built from `lpoptions -p -l`. The options Craft handles itself are skipped, as the PPD parser does, and the starred choice is the fallback default. Fixes #4699 * fix(print): skip CUPS custom-value templates in the lpoptions fallback `lpoptions -l` prints `Custom`, `Custom.` (`Custom.PASSCODE`, `Custom.STRING`, …) or `Custom.WIDTHxHEIGHT` in place of the choices of an option whose value the user has to type. Nothing in the dialog collects that value, so offering the template sent its literal text to the driver. Those entries are now dropped, and a starred template never becomes the default. The choice cap is also tested before the duplicate scan, so a listing past `MAX_LPOPTIONS_CHOICES` no longer walks the stored choices on every token. Refs #4699 * fix(print): refuse a destination that starts with a dash `lpoptions` parses its argv with getopt, so `-p -E -l` reads `-E` as an option rather than as the destination; `--` cannot help, because it ends option parsing before the `-p` that names the queue. Reject a leading `-` on the queue and on the instance component beside the existing name checks, before the PPD is opened or the command is spawned. * fix(print): read the listing by its last colon, and stop inventing a default `list_group()` prints the option as `"%s/%s:"` from the PPD's keyword and translated text and each choice as a bare `" %s"`, so a colon can sit in the text but not in a choice keyword. Splitting at the first colon turned the rest of the text ("Supporto: Tipo") into invented choices; split at the last one instead. When the choice CUPS marks is a custom-value template, the parser dropped it and stood the first retained choice in as the default. The UI then showed a value the job does not use, because only options changed away from the default are submitted. Leave the default empty in that case: there is nothing the dialog could collect, and an invented choice is worse than no selection. --------- Co-authored-by: Jarvis (Hermes) Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> --- crates/print/src/spool.rs | 145 +++++++++++++++++++++++++++++++++++--- crates/print/src/tests.rs | 125 +++++++++++++++++++++++++++++++- 2 files changed, 260 insertions(+), 10 deletions(-) diff --git a/crates/print/src/spool.rs b/crates/print/src/spool.rs index b16f57026d..fd61301209 100644 --- a/crates/print/src/spool.rs +++ b/crates/print/src/spool.rs @@ -164,10 +164,14 @@ pub(crate) fn printer_options_via_within( ) -> Vec { let (queue, instance) = printer.split_once('/').map_or((printer, None), |(q, i)| (q, Some(i))); // Retain the conservative name checks, applied to each component. Neither component is - // a path: reject dot components, backslashes and any additional separator before I/O. + // a path: reject dot components, backslashes and any additional separator before I/O. A + // leading `-` is refused too: `lpoptions` parses its argv with getopt, so `-p -E -l` would + // read `-E` as an option rather than as the destination, and `--` cannot help because it + // ends option parsing before the `-p` that names the queue. let valid = |name: &str| { !name.is_empty() && name.len() <= 64 + && !name.starts_with('-') && !matches!(name, "." | "..") && name.bytes().all(|b| b.is_ascii_graphic() && !matches!(b, b'/' | b'\\' | b':' | b'=' | b'"')) }; @@ -181,11 +185,20 @@ pub(crate) fn printer_options_via_within( f.take(PPD_CAP).read_to_end(&mut bytes).ok().map(|_| bytes) }); let mut options = ppd.as_deref().map(|b| parse_ppd(&ppd_text(b))).unwrap_or_default(); + let lpoptions_out = output_within(lpoptions.args(["-p", printer, "-l"]), limit).ok(); + let current = lpoptions_out.as_deref().map(|out| parse_lpoptions(&String::from_utf8_lossy(out))).unwrap_or_default(); + if options.is_empty() { + // The PPD is missing or unreadable (#4699). The same `lpoptions -l` listing carries every + // option with its choices and default; build a fallback list from it. + if let Some(out) = &lpoptions_out { + options = parse_lpoptions_list(&String::from_utf8_lossy(out)); + } + } + // After the fallback: a readable PPD whose `*OpenUI` blocks yielded nothing still supplies the + // translations, and `localize` leaves options it cannot translate untouched. if let Some(b) = &ppd { localize(&mut options, b, lang); } - let current = - output_within(lpoptions.args(["-p", printer, "-l"]), limit).map(|out| parse_lpoptions(&String::from_utf8_lossy(&out))).unwrap_or_default(); for (key, value) in current { if let Some(o) = options.iter_mut().find(|o| o.key == key) && o.choices.iter().any(|(c, _)| *c == value) @@ -196,6 +209,108 @@ pub(crate) fn printer_options_via_within( options } +/// Caps that match [`craft_print::parse_ppd`]: a malformed spooler answer is not allowed to +/// dominate memory or the dialog. +#[cfg(all(unix, not(target_arch = "wasm32")))] +const MAX_LPOPTIONS_OPTIONS: usize = 400; +#[cfg(all(unix, not(target_arch = "wasm32")))] +const MAX_LPOPTIONS_CHOICES: usize = 400; +#[cfg(all(unix, not(target_arch = "wasm32")))] +const MAX_LPOPTIONS_LABEL: usize = 256; + +/// A PPD keyword: printable ASCII without spaces, `/`, `:`, `=` or `"`. +#[cfg(all(unix, not(target_arch = "wasm32")))] +// The three name helpers below mirror `is_ppd_keyword`, `name_label` and `ppd_label` in craft-print, +// which are private there: keep them in step with that crate, or they will accept in one reader what +// the other refuses. +fn is_lpoptions_keyword(s: &str) -> bool { + !s.is_empty() && s.len() <= 64 && s.bytes().all(|b| b.is_ascii_graphic() && !matches!(b, b'/' | b':' | b'=' | b'"')) +} + +/// `Name/Label` → (`Name`, `Label`), the label falling back to the name. +#[cfg(all(unix, not(target_arch = "wasm32")))] +fn lpoptions_name_label(s: &str) -> (&str, &str) { + match s.split_once('/') { + Some((n, l)) if !l.trim().is_empty() => (n.trim(), l.trim()), + Some((n, _)) => (n.trim(), n.trim()), + None => (s.trim(), s.trim()), + } +} + +/// `label`, cut to [`MAX_LPOPTIONS_LABEL`] characters. +#[cfg(all(unix, not(target_arch = "wasm32")))] +fn lpoptions_label(label: &str) -> String { + label.chars().take(MAX_LPOPTIONS_LABEL).collect() +} + +/// Whether `choice` is a custom-value template. For a choice the user has to type, `lpoptions -l` +/// prints `Custom`, `Custom.` (`Custom.PASSCODE`, `Custom.STRING`, …) or `Custom.WIDTHxHEIGHT` +/// in place of the choices, and the value is not something Craft can collect: it would be sent to +/// the driver as the literal text. The prefix is reserved by CUPS, case-insensitively. +#[cfg(all(unix, not(target_arch = "wasm32")))] +fn is_lpoptions_custom_template(choice: &str) -> bool { + match choice.split_once('.') { + Some((name, _)) => name.eq_ignore_ascii_case("Custom"), + None => choice.eq_ignore_ascii_case("Custom"), + } +} + +/// Build [`PrinterOption`]s from the `lpoptions -p NAME -l` listing (`Key/Label: a *b c`). This is +/// the fallback when the PPD cannot be read, so there is no group information: every option lands in +/// one unnamed group, and the listing cannot say which options sit under `InstallableOptions`, so an +/// optional-hardware entry that [`parse_ppd`] drops may appear here. The starred choice is the +/// driver's default. Options Craft handles itself ([`HANDLED_OPTIONS`]) and custom-value templates +/// ([`is_lpoptions_custom_template`]) are skipped. +/// +/// The line is split at its **last** colon, not its first: `list_group()` prints the option as +/// `"%s/%s:"` from the PPD's keyword and translated text, then every choice as a bare `" %s"`, so +/// text such as `Supporto: Tipo` carries colons while a choice keyword cannot. Splitting at the +/// first colon would turn the rest of the label into invented choices. +#[cfg(all(unix, not(target_arch = "wasm32")))] +fn parse_lpoptions_list(out: &str) -> Vec { + let mut options = Vec::new(); + for line in out.lines() { + let line = line.trim_end_matches('\r'); + let Some((head, choices)) = line.rsplit_once(':') else { + continue; + }; + let (key, label) = lpoptions_name_label(head); + if !is_lpoptions_keyword(key) || HANDLED_OPTIONS.contains(&key) { + continue; + } + let mut choice_list = Vec::new(); + let mut default = String::new(); + // Set when the choice CUPS marks is a template the dialog cannot collect. The printer keeps + // that value, so standing in the first retained choice would show a default the job does not + // use: only options changed away from the default are submitted. + let mut marked_template = false; + for token in choices.split_whitespace() { + let starred = token.starts_with('*'); + let (choice_key, choice_label) = lpoptions_name_label(token.strip_prefix('*').unwrap_or(token)); + if !is_lpoptions_keyword(choice_key) || is_lpoptions_custom_template(choice_key) { + marked_template |= starred && is_lpoptions_custom_template(choice_key); + continue; + } + if starred { + default = choice_key.to_string(); + } + if choice_list.len() < MAX_LPOPTIONS_CHOICES && choice_list.iter().all(|(c, _)| c != choice_key) { + choice_list.push((choice_key.to_string(), lpoptions_label(choice_label))); + } + } + if choice_list.is_empty() || options.len() >= MAX_LPOPTIONS_OPTIONS { + continue; + } + if default.is_empty() && !marked_template { + // No choice marked at all: the dialog needs something selected, and the listing has + // nothing better than the first one to offer. + default = choice_list.first().map(|(c, _)| c.clone()).unwrap_or_default(); + } + options.push(PrinterOption { key: key.to_string(), label: lpoptions_label(label), group: String::new(), choices: choice_list, default }); + } + options +} + /// The keys the macOS print dialog names a job's title and owner under. #[cfg_attr(not(target_os = "macos"), allow(dead_code))] const JOB_NAME_KEY: &str = "com.apple.print.JobInfo.PMJobName"; @@ -271,7 +386,9 @@ pub fn open_printer_preferences(printer: &str) -> Result<(), crate::PrintError> fn ppd_languages(lang: &str) -> Vec { let lang = lang.to_ascii_lowercase(); let mut parts = lang.split(['-', '_']).filter(|p| !p.is_empty()); - let Some(primary) = parts.next() else { return Vec::new() }; + let Some(primary) = parts.next() else { + return Vec::new(); + }; let region = match (primary, parts.next()) { ("zh", Some("hans")) => Some("CN".to_string()), ("zh", Some("hant")) => Some("TW".to_string()), @@ -303,7 +420,9 @@ pub(crate) fn localize(options: &mut [PrinterOption], ppd: &[u8], lang: &str) { let mut choices: HashMap<(String, String), String> = HashMap::new(); let mut groups: HashMap = HashMap::new(); for line in ppd.split(|b| *b == b'\n') { - let Some(text) = std::str::from_utf8(line).ok() else { continue }; + let Some(text) = std::str::from_utf8(line).ok() else { + continue; + }; let text = text.trim_end_matches('\r'); if let Some(rest) = text.strip_prefix("*OpenGroup:") { // Group label → keyword, since options carry their group's label. @@ -313,10 +432,18 @@ pub(crate) fn localize(options: &mut [PrinterOption], ppd: &[u8], lang: &str) { } continue; } - let Some(rest) = text.strip_prefix(prefix.as_str()) else { continue }; - let Some((head, _)) = rest.split_once(':') else { continue }; - let Some((main, spec)) = head.split_once(' ') else { continue }; - let Some((key, label)) = spec.split_once('/') else { continue }; + let Some(rest) = text.strip_prefix(prefix.as_str()) else { + continue; + }; + let Some((head, _)) = rest.split_once(':') else { + continue; + }; + let Some((main, spec)) = head.split_once(' ') else { + continue; + }; + let Some((key, label)) = spec.split_once('/') else { + continue; + }; let label = label.trim(); if label.is_empty() { continue; diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index b206822e32..e8b48650cf 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -759,7 +759,9 @@ mod cups_options { /// The child process stands in for lpoptions, recording its real arguments. #[test] fn stand_in_for_lpoptions() { - let Some(record) = std::env::var_os(RECORD) else { return }; + let Some(record) = std::env::var_os(RECORD) else { + return; + }; let args: Vec = std::env::args().skip_while(|a| a != "--").skip(1).collect(); std::fs::write(std::path::Path::new(&record).join("args"), args.join("\n")).unwrap(); let answer = std::env::var(ANSWER).unwrap(); @@ -877,6 +879,13 @@ mod cups_options { long_queue.as_str(), long_queue_instance.as_str(), long_instance.as_str(), + // A destination cannot be an argument: `lpoptions` would take these as options. + "-Office", + "-E", + "-", + "--", + "Office/-l", + "Office/--", ] { assert!( spool::printer_options_via_within(destination, &dir, command(&dir, "InputSlot: *Auto"), LIMIT, "en").is_empty(), @@ -886,6 +895,120 @@ mod cups_options { } std::fs::remove_dir_all(dir).unwrap(); } + + /// #4699: when the queue's PPD is not readable, build the option list from `lpoptions -l` + /// instead of returning an empty dialog. + #[test] + fn unreadable_ppd_falls_back_to_lpoptions_list() { + let dir = folder(); + // No PPD file: the PPD path is unreadable, just as on Arch where cupsd keeps it at + // 0640 root:cups. + let lpoptions_out = "MediaType/Media Type: *Stationery PhotographicGlossy StationeryInkjet Com.brotherBp71\ncupsPrintQuality/cupsPrintQuality: Draft *Normal High\nPageSize/Page Size: A4 Letter\nDuplex/Duplex: *None DuplexNoTumble\nno colon on this line\nEmpty/Empty:\n"; + let options = spool::printer_options_via_within("Office", &dir, command(&dir, lpoptions_out), LIMIT, "en"); + let keys: Vec<&str> = options.iter().map(|o| o.key.as_str()).collect(); + assert_eq!(keys, ["MediaType", "cupsPrintQuality"], "PageSize is handled by the dialog and must be skipped"); + + let media_type = options.iter().find(|o| o.key == "MediaType").unwrap(); + assert_eq!(media_type.label, "Media Type"); + assert_eq!(media_type.default, "Stationery"); + assert_eq!( + media_type.choices, + [ + ("Stationery".into(), "Stationery".into()), + ("PhotographicGlossy".into(), "PhotographicGlossy".into()), + ("StationeryInkjet".into(), "StationeryInkjet".into()), + ("Com.brotherBp71".into(), "Com.brotherBp71".into()), + ] + ); + + let quality = options.iter().find(|o| o.key == "cupsPrintQuality").unwrap(); + assert_eq!(quality.label, "cupsPrintQuality"); + assert_eq!(quality.default, "Normal"); + assert_eq!(quality.choices, [("Draft".into(), "Draft".into()), ("Normal".into(), "Normal".into()), ("High".into(), "High".into())]); + + // The starring decides the fallback default, wherever the starred choice sits. + let lpoptions_out = "MediaType/Media Type: Plain *Glossy Matte\n"; + let options = spool::printer_options_via_within("Office", &dir, command(&dir, lpoptions_out), LIMIT, "en"); + assert_eq!(options.len(), 1); + assert_eq!(options[0].default, "Glossy", "the starred choice is the fallback default"); + assert_eq!(options[0].choices, [("Plain".into(), "Plain".into()), ("Glossy".into(), "Glossy".into()), ("Matte".into(), "Matte".into())]); + + // A PPD path that exists but cannot be opened at all, for a reason other than NotFound: + // ENOTDIR fails for root as well, where a 0o000 file would still be readable. + let blocker = dir.join("not-a-directory"); + std::fs::write(&blocker, b"x").unwrap(); + let options = spool::printer_options_via_within("Office", &blocker, command(&dir, lpoptions_out), LIMIT, "en"); + assert_eq!(options.len(), 1, "an unopenable PPD falls back as an unreadable one does"); + + // A readable PPD whose *OpenUI blocks yield nothing (driverless queues): the fallback still + // fills the list, and the PPD's translations apply to it. + std::fs::write(dir.join("Empty.ppd"), b"*PPD-Adobe: \"4.3\"\n*cupsLanguages: \"it\"\n*it.Translation MediaType/Tipo di supporto: \"\"\n") + .unwrap(); + let options = spool::printer_options_via_within("Empty", &dir, command(&dir, lpoptions_out), LIMIT, "it"); + assert_eq!(options.len(), 1, "a readable PPD without options falls back too"); + assert_eq!(options[0].label, "Tipo di supporto", "the readable PPD still translates the fallback list"); + + // A queue instance reaches lpoptions as the destination, and falls back the same way. + let options = spool::printer_options_via_within("Office/Instance", &dir, command(&dir, lpoptions_out), LIMIT, "en"); + assert_eq!(options.len(), 1); + let recorded = std::fs::read_to_string(dir.join("args")).unwrap(); + assert!(recorded.contains("Office/Instance"), "the instance is the destination: {recorded}"); + + std::fs::remove_dir_all(dir).unwrap(); + } + + /// CUPS prints a custom option's template instead of its choices — `Custom`, `Custom.PASSCODE`, + /// `Custom.STRING`, `Custom.WIDTHxHEIGHT`: the dialog has no field to collect the value, so + /// offering the template would send that literal text to the driver. + #[test] + fn custom_value_templates_are_not_offered() { + let dir = folder(); + let lpoptions_out = "Pin/Pin: *Custom.PASSCODE\nJobPassword/Job Password: 1234 *5678 Custom.PASSWORD\nScale/Scale: *Custom.REAL\nSolo/Solo: *Custom\nMediaType/Media Type: *Plain Custom.Glossy Matte\n"; + let options = spool::printer_options_via_within("Office", &dir, command(&dir, lpoptions_out), LIMIT, "en"); + let keys: Vec<&str> = options.iter().map(|o| o.key.as_str()).collect(); + assert_eq!(keys, ["JobPassword", "MediaType"], "an option whose choices are all templates is dropped"); + + let password = options.iter().find(|o| o.key == "JobPassword").unwrap(); + assert_eq!(password.choices, [("1234".into(), "1234".into()), ("5678".into(), "5678".into())]); + assert_eq!(password.default, "5678", "a template is never a choice"); + + let media_type = options.iter().find(|o| o.key == "MediaType").unwrap(); + assert_eq!(media_type.choices, [("Plain".into(), "Plain".into()), ("Matte".into(), "Matte".into())]); + assert_eq!(media_type.default, "Plain"); + + // A starred template is never offered as a choice, and never replaced by an invented + // default: the printer keeps the template, so standing the first real choice in its + // place would show a value the job does not use. + let options = spool::printer_options_via_within("Office", &dir, command(&dir, "Pin/Pin: *Custom.PASSCODE 1234\n"), LIMIT, "en"); + assert_eq!(options.len(), 1); + assert_eq!(options[0].choices, [("1234".into(), "1234".into())]); + assert_eq!(options[0].default, "", "an uncollectable default is not a choice"); + std::fs::remove_dir_all(dir).unwrap(); + } + + /// `list_group()` prints `Keyword/Text:` and then bare choice keywords, so the text may carry a + /// colon while the choices cannot: the split belongs at the last one. + #[test] + fn an_option_text_may_contain_a_colon() { + let dir = folder(); + let lpoptions_out = "Media/Supporto: Tipo: *A B\n"; + let options = spool::printer_options_via_within("Office", &dir, command(&dir, lpoptions_out), LIMIT, "en"); + assert_eq!(options.len(), 1); + assert_eq!(options[0].key, "Media"); + assert_eq!(options[0].label, "Supporto: Tipo", "the text keeps its colon"); + assert_eq!(options[0].choices, [("A".into(), "A".into()), ("B".into(), "B".into())], "no choice is invented from the text"); + assert_eq!(options[0].default, "A"); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn the_listing_is_capped_the_way_the_ppd_is() { + let dir = folder(); + let listing: String = (0..500).map(|i| format!("Option{i}/Option {i}: *A B\n")).collect(); + let options = spool::printer_options_via_within("Office", &dir, command(&dir, &listing), LIMIT, "en"); + assert_eq!(options.len(), 400, "the fallback honours the same cap as parse_ppd"); + std::fs::remove_dir_all(dir).unwrap(); + } } #[test]