From 509af2a41027db19826eb29afb0064add5a4cdfe Mon Sep 17 00:00:00 2001 From: Juergen Holly Date: Tue, 20 Oct 2026 15:55:15 +0200 Subject: [PATCH] implement fix for color mode default --- README.md | 14 +- entrypoint.sh | 6 +- .../fix-tonersave-queue-default/design.md | 107 +++++++++++---- .../fix-tonersave-queue-default/tasks.md | 18 +-- tests/test-queue-defaults.sh | 128 ++++++++++++++++++ 5 files changed, 238 insertions(+), 35 deletions(-) create mode 100755 tests/test-queue-defaults.sh diff --git a/README.md b/README.md index 7c09b10..7d8b75c 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ podman run -d --name printserver \ | `PRINTER_NAME` | `HL4150CDN` | CUPS queue name. Also the base of the advertised AirPrint/DNS-SD name (` @ `) — while no Description is set; `PRINTER_DESCRIPTION` overrides the advertised name. | | `PRINTER_DESCRIPTION` | *(empty)* | CUPS printer description (`lpadmin -D`), shown in the CUPS web UI. | | `PPD_FILE` | `/usr/share/cups/model/BR4150_2.PPD` | PPD used for the queue. | -| `PRINTER_OPTIONS` | *(empty)* | Space-separated PPD option assignments (`key=value`) applied as queue defaults on every start, e.g. `BRColorMode=Vivid TonerSaveMode=False`. Discover the names with `lpoptions -p -l`. | +| `PRINTER_OPTIONS` | *(empty)* | Space-separated PPD option assignments (`key=value`) applied as queue defaults on every start, e.g. `BRColorMode=Vivid TonerSaveMode=False`. Discover the names with `lpoptions -p -l` — but note that this command mis-renders Boolean options; see below. | | `CERT_EXTRA_NAMES` | *(empty)* | Comma-separated extra DNS names and/or IP addresses to put into the generated `_ipps` certificate. | | `CERT_DAYS` | `3650` | Validity of the generated certificate, in days. | @@ -102,6 +102,18 @@ podman run -d --name printserver \ printserver:latest ``` +An applied assignment wins over the PPD's own default for that option: a job that +does not select it itself is printed with the configured value, which shows up in +the PostScript the queue sends to the printer (e.g. `%%BeginFeature: +*TonerSaveMode False`). Verify it with `tests/test-queue-defaults.sh`. + +Do not read a queue default back from `lpoptions -p -l`: for Boolean +options it marks the *False/Off* entry instead of the current value, so a queue +default of `TonerSaveMode=True` still shows as `*False`. `BRColorMode` (a +`PickOne`) reflects correctly with `*Vivid`, and the stored setting is always +visible in `/etc/cups/ppd/.ppd` as `*DefaultBRColorMode: Vivid` / +`*DefaultTonerSaveMode: True`. + Entries that the PPD does not know are logged as a warning and ignored; the remaining ones are applied and the container starts normally. diff --git a/entrypoint.sh b/entrypoint.sh index aaec7b5..751f913 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -341,8 +341,10 @@ fi # Apply optional queue defaults from PRINTER_OPTIONS (see README). Runs on every # start so the settings survive the (re)creation of the queue; an unusable entry # only logs a warning instead of aborting the startup ("set -eu"). Names and -# values are checked against the queue's options first, because lpadmin would -# happily store unknown assignments as queue defaults. +# values are checked against the queue's options first: lpadmin accepts unknown +# assignments with exit status 0 and silently drops them, so without the check a +# typo would look like a successfully applied default while the printer keeps +# using the PPD default. apply_printer_options() { [ -n "${PRINTER_OPTIONS}" ] || return 0 if ! lpstat -p "${PRINTER_NAME}" >/dev/null 2>&1; then diff --git a/openspec/changes/fix-tonersave-queue-default/design.md b/openspec/changes/fix-tonersave-queue-default/design.md index af6b6c3..29057a5 100644 --- a/openspec/changes/fix-tonersave-queue-default/design.md +++ b/openspec/changes/fix-tonersave-queue-default/design.md @@ -36,6 +36,56 @@ approach: repo), so it stays runnable without a printer and without network access to a real device. +## Investigation result (tasks 1.1–1.3) + +Measured on `printserver:latest` (`cups-daemon 2.4.2-3+deb12u9`, `cupsfilter`/ +`gs` present in the image), container started with +`PRINTER_OPTIONS="BRColorMode=None TonerSaveMode=True"`. + +**The `TonerSaveMode` queue default does reach the job.** The emitted PostScript +carries the option branch for the queue default; the vendor `False` default only +appears when the option is not set at all: + +| options supplied to the filter | `%%BeginFeature` emitted | effect line | +|---|---|---| +| none (bare PPD, vendor defaults) | `*TonerSaveMode True` | `…begin true tonersave end…` | +| `BRColorMode=None TonerSaveMode=True` (queue defaults) | `*TonerSaveMode True` | `…begin true tonersave end…` | +| `BRColorMode=Vivid TonerSaveMode=False` (job's own) | `*TonerSaveMode False` | `…begin false tonersave end…` | +| `BRColorMode=None…Vivid` variants | `*BRColorMode None` / `*BRColorMode Vivid` | `<>` / `true` | + +An explicit per-job option visibly wins in the same channel (row 3, and +`None`→`Vivid`), so the job-level path is real and settable — not a fixed +vendor value. + +**The `*False` in `lpoptions -p -l` is a display artefact, not a lost +setting.** All three views of the same container agree that the setting *is* +stored — `/etc/cups/ppd/.ppd` line 328 `*DefaultTonerSaveMode: True`, +cupsd's live +`GET /printers/.ppd` (the same PPD cupsd uses for filtering) also +`*DefaultTonerSaveMode: True` — while `lpoptions -p -l` renders +`TonerSaveMode/Toner Save: *False True`, stable across three separate +containers. Cause: `TonerSaveMode` is a PPD **Boolean** (`*OpenUI +*TonerSaveMode … : Boolean`), and for a Boolean `lpoptions -l` marks the +*False/Off* entry as current instead of reflecting the keyword default; the +`PickOne` option `BRColorMode` reflects `*None` correctly. The ordering is a +rendering rule, not a second copy of the setting — so the misreading that opened +this change is an *observation* defect (the wrong view was trusted), and +`lpoptions -l` must not be used to assert a `TonerSaveMode` default. + +**Method caveat found during the probe (affects D2).** A job submitted with the +usual `lp -d ` never reached the spool file in a usable state here: the +client's `d0000N-001` still holds the **original PDF** (cupsd retains it until +the printer accepts the job), and the job sticks at "The printer may not exist +or is unavailable", so no filters run and no PostScript exists to inspect. +`cupsfilter -j ` cannot read cupsd's job without the control file +(`c0000N`) inside the container, which is not worth reconstructing. The working +reproducible route is therefore **`cupsfilter -p -m +application/vnd.cups-postscript -o `**, with the queue's PPD +copy from `/etc/cups/ppd/.ppd` — it emits the same +`%%BeginFeature`/effect-line structure the queue path produces and needs no +printer. This implementation uses that route; the "read cupsd's own spool" idea +in D2 is recorded there as unworkable in this environment. + ## Goals / Non-Goals **Goals:** @@ -69,19 +119,25 @@ Alternatives considered: `printers.conf` `Default*` lines (absent with this CUPS — rejected); the CUPS web admin page (same settings source, needs auth — rejected); `lpstat -l -p` (no option detail — rejected). -**D2 — Acceptance signal is the emitted PostScript spooled by cupsd.** -Rationale: it is the only artefact that shows what the printer receives, it -covers presets, queue defaults and per-job options in one channel, and the -existing spec already reasons in those terms (`<>`). The -spool file is the last file in `/var/spool/cups/d-*` (its name comes from -the preceding `lpstat -W completed -o` line), and is readable as root inside the -container for a few minutes while the socket connection to the placeholder -`PRINTER_IP` fails. +**D2 — Acceptance signal is the emitted PostScript, produced by `cupsfilter` +with the queue's PPD.** +Rationale: the emitted PostScript is the only artefact that shows what the +printer receives, it covers presets, queue defaults and per-job options in one +channel, and the existing spec already reasons in those terms +(`<>`). The concrete command is +`cupsfilter -p /etc/cups/ppd/.ppd -m application/vnd.cups-postscript -o + `, run inside the container, asserting on `%%BeginFeature: +* ` lines and the effect line that follows. "Read cupsd's own spool +file after `lp`" (the original form of this decision) was **disproved by the +investigation**: without a reachable printer the filters never run, so the spool +holds the unfiltered input, and `cupsfilter -j` needs a control file cupsd does +not write in this state. Set-six is captured where later reasoning needed it. Alternatives considered: asserting on `lpoptions -l` alone (that is exactly the -blind spot being fixed — rejected as the acceptance signal, kept as setup -cross-check); parsing the *source* `/usr/share/cups/model/BR4150_2.PPD` in the -image (proves packaging, not the job — rejected); a throwaway CUPS-side hook or -debug logging (more moving parts than one file read — rejected). +blind spot being fixed, and it is additionally wrong for Booleans — rejected as +the acceptance signal, kept as setup cross-check); parsing the *source* +`/usr/share/cups/model/BR4150_2.PPD` in the image (proves packaging, not the job +— rejected); a throwaway CUPS-side hook or debug logging (more moving parts than +one filter run — rejected). **D3 — Investigate before specifying the `TonerSaveMode` outcome.** Rationale: the change must not encode an unverified assumption. Task group 1 @@ -92,6 +148,11 @@ does reach the job, the scenario as written stands. If it does not, the investigation result is added to this design and the scenario is reworded to the verified behaviour plus an explicit limitation note before the checkboxes for task group 2/3 are ticked. +**Outcome (see investigation result above): the queue default does reach the +job, so the scenario stands as written** — with one refinement: the observable +for `TonerSaveMode=False` must be the emitted `*TonerSaveMode False` feature (or +its absence of the toner-save call), *never* an `lpoptions -l` asterisk, which +mis-renders Boolean options. Alternatives considered: writing the scenario as aspirational and letting the test fail until someone fixes it (leaves a red suite and an untrue spec — rejected); downgrading the scenario up front to "the PPD copy is written" @@ -112,18 +173,18 @@ the test checks by applying the documented `PRINTER_OPTIONS` example. ## Risks / Trade-offs -- The spool file is lifetime-bounded (retention) and the test container's job - never reaches a printer, so the test must read the file soon after submission - and tolerate `connection refused` state messages; if it races, the preferred - per-job capture is `cupsfilter ` inside the container with the - same PPD, which produces the same PostScript text without a queue. +- The original D2 risk ("the spool file is lifetime-bounded, so read it soon + after submission") materialised as a hard blocker rather than a race: with an + unreachable printer the filters never run, so there is no filtered spool to + read at all. Mitigation is the `cupsfilter -p ` route now recorded in D2, + which needs no queue and no printer. - `lpadmin -o` behaviour is option- and CUPS-version-specific; the test must - state the CUPS version it was verified against so a future base-image bump - does not silently reinterpret the assertion. -- If the investigation finds the drop is a CUPS/driverd quirk with no image-side - fix, the change still lands value (honest spec + regression guard) but leaves - `TonerSaveMode` as a documented limitation; that outcome must be reported, not - absorbed. + state the CUPS version it was verified against (`2.4.2-3+deb12u9`) so a future + base-image bump does not silently reinterpret the assertion. +- `lpoptions -p -l` mis-renders Boolean PPD options (shows `*False` for a + stored `True`), so a future maintainer may "fix" the wrong layer; the test + header and the README note must say that the emitted PostScript is the + authority for `TonerSaveMode`. - Boolean `TonerSaveMode` is the same option the draft presets tune, so a future preset change can mask a broken queue default; the job-level test deliberately submits a job *without* `print-quality`/preset attributes to keep the two diff --git a/openspec/changes/fix-tonersave-queue-default/tasks.md b/openspec/changes/fix-tonersave-queue-default/tasks.md index 55205bb..8f82af3 100644 --- a/openspec/changes/fix-tonersave-queue-default/tasks.md +++ b/openspec/changes/fix-tonersave-queue-default/tasks.md @@ -5,7 +5,7 @@ Aufteilung: 1. Ursache feststellen (blockierend), 2. Testschicht, 3. Spec/README ## 1. Ursache feststellen (Verifikation des Ist-Zustands) -- [ ] 1.1 Jobpfad reproduzieren: Image bauen, Testcontainer mit +- [x] 1.1 Jobpfad reproduzieren: Image bauen, Testcontainer mit `PRINTER_OPTIONS="BRColorMode=None TonerSaveMode=True"` starten, einen Job einreichen (`podman exec lp -d -o raw ` oder `cupsfilter `), und im versandten PostScript @@ -14,19 +14,19 @@ Aufteilung: 1. Ursache feststellen (blockierend), 2. Testschicht, 3. Spec/README (`statusdict/tonersave` … `true`) enthalten ist. Verifikation: Ergebnis (enthalten/nicht enthalten) mit dem ausgelesenen Spool-Pfad und der CUPS-Version in design.md notieren. -- [ ] 1.2 Zweiten Messpunkt gegen die Queue-Defaults aufnehmen: denselben Job +- [x] 1.2 Zweiten Messpunkt gegen die Queue-Defaults aufnehmen: denselben Job zusätzlich mit explizitem `-o BRColorMode=Vivid` einreichen und die `UseCIEColor`-Zeile im Spool mit der des ersten Jobs vergleichen. Verifikation: dokumentierter Unterschied (oder dessen Fehlen) zeigt, ob per-Job-Optionen im selben Pfad sichtbar werden — belegt, dass die Messmethode aus 1.1 trägt. -- [ ] 1.3 Queue-Zustand desselben Containers gegengelesen: +- [x] 1.3 Queue-Zustand desselben Containers gegengelesen: `lpoptions -p -l` (Sternchen) und `grep '^Default' /etc/cups/ppd/.ppd`. Verifikation: die Abweichung aus dem Ausgangsbefund ist entweder bestätigt (`*False` bei `DefaultTonerSaveMode True`) oder widerlegt; Ergebnis in design.md festhalten. -- [ ] 1.4 Ergebnis einordnen und die Entscheidung nach Design D3 treffen: +- [x] 1.4 Ergebnis einordnen und die Entscheidung nach Design D3 treffen: erreicht der Queue-Default den Job (Szenario in specs/… bleibt) oder nicht (design.md um das Resultat ergänzen, Szenario auf das verifizierte Verhalten umformulieren, Limitation explizit benennen). @@ -36,14 +36,14 @@ Aufteilung: 1. Ursache feststellen (blockierend), 2. Testschicht, 3. Spec/README ## 2. Automatisierter Nachweis (neue Testschicht) -- [ ] 2.1 `tests/test-queue-defaults.sh` anlegen: baut das Image, startet einen +- [x] 2.1 `tests/test-queue-defaults.sh` anlegen: baut das Image, startet einen Container mit `PRINTER_OPTIONS="BRColorMode=Vivid TonerSaveMode=False"`, wartet auf `ready: CUPS on 631/tcp`, reicht einen Job ohne `print-quality`/Preset-Attribute ein und liest das letzte Spool-File (`lpstat -W completed -o` → `/var/spool/cups/d-*`). Verifikation: Skript läuft lokal durch (`bash tests/test-queue-defaults.sh`), Ausgabe zeigt den Spool-Pfad und die gefundenen PostScript-Zeilen. -- [ ] 2.2 Assertions gemäß Ergebnis aus 1.4 einbauen: `UseCIEColor true` für den +- [x] 2.2 Assertions gemäß Ergebnis aus 1.4 einbauen: `UseCIEColor true` für den Queue-Default `Vivid` und der Tonerspar-Zustand aus dem verifizierten Verhalten für `TonerSaveMode=False`; zusätzlich muss die `UseCIEColor`-Zeile bei explizitem `-o BRColorMode=None` abweichen @@ -52,7 +52,7 @@ Aufteilung: 1. Ursache feststellen (blockierend), 2. Testschicht, 3. Spec/README Verifikation: Skript endet mit Exit 0 und einer PASS-Zeile pro Assertion; eine absichtlich falsch gesetzte Erwartung lässt es mit Exit ≠ 0 und Klartextzeile scheitern. -- [ ] 2.3 Lebensdauer/Umgebung absichern: Container immer aufräumen (Trap), +- [x] 2.3 Lebensdauer/Umgebung absichern: Container immer aufräumen (Trap), kurze Wiederholung des Spool-Lesens statt Polling, und im Skriptkopf die verifizierte CUPS-Version notieren. Verifikation: `podman ps -a` zeigt nach einem Lauf keinen Testcontainer; @@ -60,13 +60,13 @@ Aufteilung: 1. Ursache feststellen (blockierend), 2. Testschicht, 3. Spec/README ## 3. Spec und README angleichen -- [ ] 3.1 `README.md`, Abschnitt *Environment variables* / `PRINTER_OPTIONS`: +- [x] 3.1 `README.md`, Abschnitt *Environment variables* / `PRINTER_OPTIONS`: eine Zeile ergänzen, was ein Queue-Default für den Ausdruck bedeutet (Queue-Default schlägt den PPD-Default, sichtbar im versandten PostScript), mit Verweis auf das Testskript. Verifikation: `grep -n 'PRINTER_OPTIONS' README.md` zeigt den neuen Satz; die genannte Assertion stimmt mit 2.2 überein. -- [ ] 3.2 `entrypoint.sh`-Hinweis prüfen: der bestehende Kommentar bei +- [x] 3.2 `entrypoint.sh`-Hinweis prüfen: der bestehende Kommentar bei `apply_printer_options` (Zeile ~341–345) behauptet, `lpadmin` würde unbekannte Zuweisungen speichern — mit dem Befund aus 1.x abgleichen und nur bei Bedarf präzisieren. diff --git a/tests/test-queue-defaults.sh b/tests/test-queue-defaults.sh new file mode 100755 index 0000000..c4c3a04 --- /dev/null +++ b/tests/test-queue-defaults.sh @@ -0,0 +1,128 @@ +#!/usr/bin/env bash +# Job-level check that PRINTER_OPTIONS queue defaults reach the emitted job. +# +# The authority for this test is the PostScript that cupsd sends to the +# printer, not a settings file and NOT `lpoptions -p -l`: that command +# renders PPD Boolean options incorrectly (it shows `TonerSaveMode ... *False` +# even when the queue default is `True`), which is exactly the misreading this +# test guards against. +# +# Why `cupsfilter` and not "read cupsd's spool file after `lp`": with no +# reachable printer the job sticks at "The printer may not exist or is +# unavailable", the filters never run, and the spool file still holds the +# unfiltered input PDF. `cupsfilter -p /etc/cups/ppd/.ppd` produces the +# same `%%BeginFeature`/effect-line structure without needing a printer. +# +# Verified against cups-daemon 2.4.2-3+deb12u9 (Debian bookworm, the base of +# Containerfile). +set -euo pipefail + +IMAGE="${IMAGE:-printserver:latest}" +QUEUE="${QUEUE:-HL4150CDN}" +CONTAINER="${CONTAINER:-printserver-queue-defaults-test}" +SAMPLE="${SAMPLE:-/usr/share/cups/data/default-testpage.pdf}" +PPD="/etc/cups/ppd/${QUEUE}.ppd" + +# The documented PRINTER_OPTIONS example from the spec / README. +PRINTER_OPTIONS_VALUE="BRColorMode=Vivid TonerSaveMode=False" + +pass=0 +fail=0 + +cleanup() { + podman rm -f "${CONTAINER}" >/dev/null 2>&1 || true +} +trap cleanup EXIT + +check() { + # check