Guard KNOWN_IDS from pathname expansion; CI checks the id mirrors - #7
Merged
Merged
Conversation
…rrors is_known() splits $KNOWN_IDS unquoted so that entries like 2d84:* work as case patterns, but the same expansion also globs each entry against the current directory first. cupsd only chdirs to / when it daemonizes; under cupsd -f, or the launchd/systemd -l mode, the backend inherits whatever directory cupsd was started from, and a file named 2d84:anything there replaces the pattern with a filename, so every Poskey printer stops matching and tspl://auto reports no TSPL printer at all. Reproduced on sh, dash, bash and ksh. Pathname expansion is now off around the loop; case patterns are unaffected by set -f, so the wildcards keep working. The backend tests now run from a scratch directory seeded with decoy files named after the wildcard entries, so the wildcard checks go red if that guard is ever dropped. The test loop also says when a shell is skipped, and CI installs ksh so the ksh leg actually runs there rather than silently passing on two interpreters. tests/ids.sh checks that every KNOWN_IDS entry has its rule in udev/99-tspl-label.rules and its mention in the README, the two hand-kept copies of the list, and CI runs it. The comment above the list now says what it really gates: auto-match and discovery only consider these ids, so an unlisted printer has to be pinned.
…uns all Review pass on the previous commit. tests/ids.sh matched too loosely to be a gate: the README half was a whole-file grep for the backticked id, so a mention in prose passed with the printer table deleted, and the udev half was a substring match, so a commented-out rule passed too. Both now anchor: a live ATTRS line with the right keys in any order and spacing (idVendor alone for a wildcard), an entry in the rules file's header inventory, which turns out to be a third hand-kept copy, and a printer-table row. The extractor also tolerates anything after the closing quote, so a trailing comment on the KNOWN_IDS line no longer reports the variable as missing. is_known() now does the split inside a subshell instead of toggling set -f around it, so no exit path can leak the option to the caller and there is nothing to restore. Same 68 checks green on sh, dash, bash and ksh; the decoys still go red if the inner set -f is removed. The cwd story was overstated: launchd and systemd start cupsd in / by default, so the real case is a hand-run cupsd -f (or a unit with its own WorkingDirectory). Comments say that now. The comment above KNOWN_IDS also claimed discovery advertises the printers; it advertises a single tspl://auto line when any of them is present. CI checks ksh is actually present before the backend tests, since a missing shell is skipped rather than fatal, and the parse step loops over the same files shellcheck sees, which picks up packaging/deb/postinst. make test now runs the backend and id tests as well as the smoke test.
The previous commit loosened the udev check while making it tolerant of key order: it no longer required the rule to jump to tspl_link, so a rule with a missing or misspelled GOTO, or one that jumps to tspl_end, passed as "ok" while udev created no symlink for that id. That is the exact drift the script exists to catch. A live rule now has to name GOTO="tspl_link", and a vendor-wide entry has to be the only ATTRS key on its line, so a per-serial rule on that vendor no longer stands in for it. The header check reads only the "Known ids" comment block, so a stray entry elsewhere in the file does not count, and accepts any whitespace around the id rather than exactly three spaces and a trailing one. The README check reads only the printer table, from its header row to the next blank line, so an id in the options table or a fenced block does not count. The ksh guard in CI now says why it failed instead of exiting silently, and stops printing the ksh path into the log. make test runs the shell tests before the smoke test and no longer gates them on the C build, which smoke.sh performs itself. Two stale comments in tests/backend.sh brought in line with what CI and the backend now say.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #6, kept separate so that PR stayed "add this printer".
is_known() splits $KNOWN_IDS unquoted so entries like 2d84:* work as case
patterns, but that also globs each entry against the CWD first. cupsd only
chdirs to / when it daemonizes, so a hand-run cupsd -f (or a unit with its
own WorkingDirectory) leaves the backend in that directory, and a file named
2d84:anything there swaps the pattern for a filename: every Poskey printer
stops matching and tspl://auto finds nothing. Reproduced on sh, dash, bash
and ksh. The split now happens in a subshell with pathname expansion off;
case patterns don't care about set -f, so the wildcards keep working, and
nothing can leak to the caller.
Also in here:
after the wildcard entries, so the guard can't be dropped silently, and
says when a shell is skipped rather than passing quietly on fewer legs.
actually runs.
to tspl_link (idVendor-only for a wildcard), an entry in the "Known ids"
block of the rules header, and a row of the README printer table; Add QIN LabelPrinter / LW650XL PRO (2e3c:5757) to known TSPL ids #6
shipped without two of those until fixed up. Prose mentions, commented
rules, rules that jump elsewhere and ids in other tables don't count.
CI runs it, and so does make test.
between printers; it gates auto-match and discovery outright, so it now
says so.
Tests: backend.sh green on sh/dash/bash/ksh; removing the inner set -f
fails the wildcard checks on all four. ids.sh fails on a commented-out
rule, a GOTO that is missing, misspelled or points at tspl_end, a
per-serial rule standing in for a wildcard, a removed or relocated header
entry, and a deleted table row; it still passes reordered keys, a
SUBSYSTEM prefix, an indented line, a trailing comment on the KNOWN_IDS
line, and tabs or line-end in the header entry.