From f1894b999787affacddd7895c57e91312b7f8a6d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 16:59:30 +0000 Subject: [PATCH 1/2] ci: give dtolnay/rust-toolchain a valid toolchain input dtolnay/rust-toolchain at @v1 (and at its master-branch commits) declares `toolchain` as required with no default; only the toolchain-named branches such as @stable supply one. Steps here either: - set no `toolchain:` at all, so the action fails with "'toolchain' is a required input"; - set an invalid value (v1, master, or a note), which rustup cannot install; or - carry a second `with:` key injected above the real one by an earlier bulk edit. A duplicate key makes GitHub reject the whole workflow file. Each affected step now has exactly one `with:` with a valid toolchain: an injected toolchain-only `with:` block is dropped, an invalid value becomes `stable`, and a step with none gets `toolchain: stable`. An existing valid value ('1.97.0', ${{ env.RUST_VERSION }}, stable) is kept as is. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_0132Cdz3iwk2QGKv2sUhhsye --- .github/workflows/bench.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/security.yml | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 27dbec1..0a88a57 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -22,7 +22,7 @@ jobs: - uses: actions/checkout@v7.0.1 - uses: dtolnay/rust-toolchain@v1 with: - toolchain: v1 + toolchain: stable - uses: Swatinem/rust-cache@v2.9.2 - name: Install system dependencies diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8f3b4b3..312fa70 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,7 +22,7 @@ jobs: - uses: actions/checkout@v7.0.1 - uses: dtolnay/rust-toolchain@v1 with: - toolchain: v1 + toolchain: stable targets: x86_64-unknown-linux-gnu - uses: Swatinem/rust-cache@v2.9.2 diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 211913d..623df8b 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -24,7 +24,7 @@ jobs: - uses: actions/checkout@v7.0.1 - uses: dtolnay/rust-toolchain@v1 with: - toolchain: v1 + toolchain: stable - uses: Swatinem/rust-cache@v2.9.2 - name: Install cargo-audit @@ -40,6 +40,7 @@ jobs: - uses: actions/checkout@v7.0.1 - uses: dtolnay/rust-toolchain@v1 with: + toolchain: stable components: clippy - uses: Swatinem/rust-cache@v2.9.2 From 5c2cb77d97b8472cb692f38140ecd959f409c6da Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Mon, 19 Oct 2026 18:14:11 +0000 Subject: [PATCH 2/2] fix(ci): adapt to IPP and mDNS API changes, resolve Clippy warnings, and sync Cargo.lock --- Cargo.lock | 3 + crates/presswerk-core/benches/core_bench.rs | 22 +-- crates/presswerk-core/tests/aspect_test.rs | 19 +-- crates/presswerk-core/tests/e2e_test.rs | 3 +- crates/presswerk-core/tests/property_test.rs | 24 ++-- .../benches/document_bench.rs | 4 +- crates/presswerk-print/benches/ipp_bench.rs | 4 +- crates/presswerk-print/src/capabilities.rs | 20 ++- crates/presswerk-print/src/discovery.rs | 11 +- crates/presswerk-print/src/ipp_client.rs | 133 +++++++++++++----- crates/presswerk-print/src/ipp_server.rs | 1 - crates/presswerk-print/src/retry.rs | 10 ++ .../benches/security_bench.rs | 4 +- 13 files changed, 175 insertions(+), 83 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 010f418..ad043a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5212,6 +5212,7 @@ dependencies = [ "rten-imageproc", "rten-tensor", "serde", + "serde_json", "sha2 0.11.0", "thiserror 2.0.21", "tracing", @@ -5225,8 +5226,10 @@ dependencies = [ "criterion", "hex", "ipp", + "lopdf 0.40.0", "mdns-sd", "presswerk-core", + "presswerk-document", "presswerk-security", "rusqlite", "serde", diff --git a/crates/presswerk-core/benches/core_bench.rs b/crates/presswerk-core/benches/core_bench.rs index 900f299..061af04 100644 --- a/crates/presswerk-core/benches/core_bench.rs +++ b/crates/presswerk-core/benches/core_bench.rs @@ -4,7 +4,9 @@ // Benchmarks for presswerk-core critical operations. // Run with: cargo bench -p presswerk-core -use criterion::{Criterion, black_box, criterion_group, criterion_main}; +use std::hint::black_box; + +use criterion::{Criterion, criterion_group, criterion_main}; use presswerk_core::{AppConfig, DocumentType, JobSource, PrintJob, PrintSettings}; fn bench_config_creation(c: &mut Criterion) { @@ -39,7 +41,7 @@ fn bench_config_serialization(c: &mut Criterion) { c.bench_function("config_deserialize_json", |b| { b.iter(|| { - let _config: AppConfig = serde_json::from_str(&black_box(&json)).expect("deserialize"); + let _config: AppConfig = serde_json::from_str(black_box(&json)).expect("deserialize"); }) }); } @@ -98,7 +100,7 @@ fn bench_job_serialization(c: &mut Criterion) { c.bench_function("job_deserialize_json", |b| { b.iter(|| { - let _job: PrintJob = serde_json::from_str(&black_box(&json)).expect("deserialize"); + let _job: PrintJob = serde_json::from_str(black_box(&json)).expect("deserialize"); }) }); } @@ -112,10 +114,12 @@ fn bench_print_settings(c: &mut Criterion) { c.bench_function("settings_custom", |b| { b.iter(|| { - let mut _settings = PrintSettings::default(); - _settings.copies = black_box(5); - _settings.color = black_box(false); - _settings.scale_to_fit = black_box(false); + let _settings = PrintSettings { + copies: black_box(5), + color: black_box(false), + scale_to_fit: black_box(false), + ..Default::default() + }; }) }); @@ -132,7 +136,7 @@ fn bench_print_settings(c: &mut Criterion) { c.bench_function("settings_deserialize_json", |b| { b.iter(|| { let _settings: PrintSettings = - serde_json::from_str(&black_box(&json)).expect("deserialize"); + serde_json::from_str(black_box(&json)).expect("deserialize"); }) }); } @@ -183,7 +187,7 @@ fn bench_job_id_operations(c: &mut Criterion) { c.bench_function("job_id_clone", |b| { b.iter(|| { - let _id = black_box(&id).clone(); + let _id = *black_box(&id); }) }); } diff --git a/crates/presswerk-core/tests/aspect_test.rs b/crates/presswerk-core/tests/aspect_test.rs index 391937b..e80079b 100644 --- a/crates/presswerk-core/tests/aspect_test.rs +++ b/crates/presswerk-core/tests/aspect_test.rs @@ -124,14 +124,14 @@ fn aspect_invalid_dpi_detection() { for dpi in valid_dpis { assert!( - dpi >= MIN_DPI && dpi <= MAX_DPI, + (MIN_DPI..=MAX_DPI).contains(&dpi), "DPI {} should be valid", dpi ); } for dpi in invalid_dpis { - let is_invalid = dpi < MIN_DPI || dpi > MAX_DPI; + let is_invalid = !(MIN_DPI..=MAX_DPI).contains(&dpi); assert!(is_invalid, "DPI {} should be detected as invalid", dpi); } } @@ -295,10 +295,12 @@ fn aspect_bytes_progress_invariant() { #[test] fn aspect_encryption_setting_consistency() { - let mut config = AppConfig::default(); + let mut config = AppConfig { + encryption_enabled: false, + ..Default::default() + }; // If encryption is disabled, audit should still be optional - config.encryption_enabled = false; assert!(!config.encryption_enabled); // audit can be true or false independently @@ -334,10 +336,11 @@ fn aspect_tls_requirement_with_network_jobs() { #[test] fn aspect_auto_accept_policy_consistency() { - let mut config = AppConfig::default(); - - // auto_accept_network_jobs decision affects workflow - config.auto_accept_network_jobs = false; + let config = AppConfig { + auto_accept_network_jobs: false, + ..Default::default() + }; + assert!(!config.auto_accept_network_jobs); // When false, jobs should be held for review let job = PrintJob::new( JobSource::Network { diff --git a/crates/presswerk-core/tests/e2e_test.rs b/crates/presswerk-core/tests/e2e_test.rs index 9733dd2..ff9fae0 100644 --- a/crates/presswerk-core/tests/e2e_test.rs +++ b/crates/presswerk-core/tests/e2e_test.rs @@ -31,6 +31,7 @@ fn e2e_config_with_custom_settings() { print_timeout_secs: 120, query_timeout_secs: 30, easy_mode: false, + ..Default::default() }; assert_eq!(config.default_paper_size, PaperSize::Letter); @@ -112,7 +113,7 @@ fn e2e_document_type_detection() { ]; for (filename, expected) in types { - let ext = filename.split('.').last().unwrap_or(""); + let ext = filename.split('.').next_back().unwrap_or(""); let detected = DocumentType::from_extension(ext); assert_eq!( detected, expected, diff --git a/crates/presswerk-core/tests/property_test.rs b/crates/presswerk-core/tests/property_test.rs index 1e566db..d90a490 100644 --- a/crates/presswerk-core/tests/property_test.rs +++ b/crates/presswerk-core/tests/property_test.rs @@ -16,19 +16,21 @@ use proptest::prelude::*; proptest! { #[test] fn prop_config_port_in_valid_range(port in 1u16..=65535u16) { - let mut config = AppConfig::default(); - config.server_port = port; + let config = AppConfig { + server_port: port, + ..Default::default() + }; prop_assert!(config.server_port >= 1); - prop_assert!(config.server_port <= 65535); } #[test] fn prop_config_timeouts_positive(timeout_secs in 1u64..=3600u64) { - let mut config = AppConfig::default(); - config.print_timeout_secs = timeout_secs; - config.query_timeout_secs = timeout_secs / 2; + let config = AppConfig { + print_timeout_secs: timeout_secs, + query_timeout_secs: timeout_secs / 2, + ..Default::default() + }; prop_assert!(config.print_timeout_secs > 0); - prop_assert!(config.query_timeout_secs >= 0); prop_assert!(config.print_timeout_secs >= config.query_timeout_secs); } @@ -127,8 +129,10 @@ proptest! { proptest! { #[test] fn prop_print_settings_copies_positive(copies in 1u32..=1000u32) { - let mut settings = PrintSettings::default(); - settings.copies = copies; + let settings = PrintSettings { + copies, + ..Default::default() + }; prop_assert!(settings.copies > 0); } @@ -176,7 +180,7 @@ proptest! { }; let enum_val = orientation.ipp_enum_value(); - prop_assert!(enum_val >= 3 && enum_val <= 6); + prop_assert!((3..=6).contains(&enum_val)); } } diff --git a/crates/presswerk-document/benches/document_bench.rs b/crates/presswerk-document/benches/document_bench.rs index ca2df55..b3595ac 100644 --- a/crates/presswerk-document/benches/document_bench.rs +++ b/crates/presswerk-document/benches/document_bench.rs @@ -5,7 +5,9 @@ // Currently benchmarks the perspective correction pipeline on a small synthetic // test image. -use criterion::{Criterion, black_box, criterion_group, criterion_main}; +use std::hint::black_box; + +use criterion::{Criterion, criterion_group, criterion_main}; use image::{DynamicImage, GrayImage, Luma}; use presswerk_core::PaperSize; diff --git a/crates/presswerk-print/benches/ipp_bench.rs b/crates/presswerk-print/benches/ipp_bench.rs index 486fc5f..5c291b4 100644 --- a/crates/presswerk-print/benches/ipp_bench.rs +++ b/crates/presswerk-print/benches/ipp_bench.rs @@ -4,7 +4,9 @@ // Criterion benchmarks for IPP request parsing, response building, and // document content hashing in the presswerk-print crate. -use criterion::{Criterion, black_box, criterion_group, criterion_main}; +use std::hint::black_box; + +use criterion::{Criterion, criterion_group, criterion_main}; use sha2::{Digest, Sha256}; use presswerk_print::ipp_server::{ diff --git a/crates/presswerk-print/src/capabilities.rs b/crates/presswerk-print/src/capabilities.rs index afe53b3..d63e51b 100644 --- a/crates/presswerk-print/src/capabilities.rs +++ b/crates/presswerk-print/src/capabilities.rs @@ -289,8 +289,10 @@ mod tests { attrs.insert("sides-supported".into(), "one-sided".into()); let caps = PrinterCapabilities::from_attributes(&attrs); - let mut settings = PrintSettings::default(); - settings.duplex = DuplexMode::LongEdge; + let settings = PrintSettings { + duplex: DuplexMode::LongEdge, + ..Default::default() + }; let (corrected, result) = auto_correct_settings(&settings, &caps); assert!(!result.valid); @@ -318,8 +320,10 @@ mod tests { attrs.insert("copies-supported".into(), "1-10".into()); let caps = PrinterCapabilities::from_attributes(&attrs); - let mut settings = PrintSettings::default(); - settings.copies = 50; + let settings = PrintSettings { + copies: 50, + ..Default::default() + }; let (corrected, result) = auto_correct_settings(&settings, &caps); assert_eq!(corrected.copies, 10); @@ -329,9 +333,11 @@ mod tests { #[test] fn unknown_caps_allows_everything() { let caps = PrinterCapabilities::from_attributes(&HashMap::new()); - let mut settings = PrintSettings::default(); - settings.duplex = DuplexMode::ShortEdge; - settings.copies = 99; + let settings = PrintSettings { + duplex: DuplexMode::ShortEdge, + copies: 99, + ..Default::default() + }; let (_, result) = auto_correct_settings(&settings, &caps); // No corrections when capabilities are unknown diff --git a/crates/presswerk-print/src/discovery.rs b/crates/presswerk-print/src/discovery.rs index ab56675..c44c0d2 100644 --- a/crates/presswerk-print/src/discovery.rs +++ b/crates/presswerk-print/src/discovery.rs @@ -14,7 +14,7 @@ use std::sync::{Arc, Mutex}; use std::time::Duration; use chrono::Utc; -use mdns_sd::{ServiceDaemon, ServiceEvent, ServiceInfo}; +use mdns_sd::{ResolvedService, ServiceDaemon, ServiceEvent}; use tracing::{debug, info, warn}; use presswerk_core::error::{PresswerkError, Result}; @@ -210,6 +210,7 @@ impl PrinterDiscovery { debug!(service_type = %stype, "mDNS search stopped"); break; } + _ => {} } } }) @@ -217,7 +218,7 @@ impl PrinterDiscovery { } } -/// Convert a resolved `ServiceInfo` into a `DiscoveredPrinter`. +/// Convert a resolved `ResolvedService` into a `DiscoveredPrinter`. /// /// TXT record keys (case-insensitive) commonly found on IPP printers: /// - `printer-make-and-model` — human-readable make/model string @@ -225,7 +226,7 @@ impl PrinterDiscovery { /// - `Color` — "T" or "F" /// - `Duplex` — "T" or "F" /// - `rp` — resource path (e.g. "ipp/print") -fn service_info_to_printer(info: &ServiceInfo, tls: bool) -> Result { +fn service_info_to_printer(info: &ResolvedService, tls: bool) -> Result { let name = info.get_fullname().to_owned(); let port = info.get_port(); @@ -235,7 +236,7 @@ fn service_info_to_printer(info: &ServiceInfo, tls: bool) -> Result Result bool { +fn txt_bool(info: &ResolvedService, key: &str) -> bool { info.get_property_val_str(key) .map(|v| v.eq_ignore_ascii_case("t") || v.eq_ignore_ascii_case("true")) .unwrap_or(false) diff --git a/crates/presswerk-print/src/ipp_client.rs b/crates/presswerk-print/src/ipp_client.rs index ed5c786..8e26e2d 100644 --- a/crates/presswerk-print/src/ipp_client.rs +++ b/crates/presswerk-print/src/ipp_client.rs @@ -13,6 +13,7 @@ use std::collections::HashMap; use std::io::Cursor; use std::time::Duration; +use ipp::parser::IppParseError; use ipp::prelude::*; use tracing::{debug, error, info, instrument}; @@ -74,7 +75,9 @@ impl IppClient { /// flat map of attribute names to their string representations. #[instrument(skip(self), fields(uri = %self.uri))] pub async fn get_printer_attributes(&self) -> Result { - let operation = IppOperationBuilder::get_printer_attributes(self.uri.clone()).build(); + let operation = IppOperationBuilder::get_printer_attributes(self.uri.clone()) + .build() + .map_err(ipp_parse_error)?; let client = AsyncIppClient::new(self.uri.clone()); debug!("sending Get-Printer-Attributes"); @@ -129,46 +132,74 @@ impl IppClient { .document_format(document_type.mime_type()); // Inject print settings as IPP job-template attributes. - builder = builder.attribute(IppAttribute::new( - "copies", - IppValue::Integer(settings.copies as i32), - )); - builder = builder.attribute(IppAttribute::new( - "media", - IppValue::Keyword(settings.paper_size.ipp_media_keyword().into()), - )); - builder = builder.attribute(IppAttribute::new( - "sides", - IppValue::Keyword(settings.duplex.ipp_sides_keyword().into()), - )); - builder = builder.attribute(IppAttribute::new( - "orientation-requested", - IppValue::Enum(settings.orientation.ipp_enum_value()), - )); - builder = builder.attribute(IppAttribute::new( - "print-color-mode", - IppValue::Keyword( - if settings.color { - "color" - } else { - "monochrome" - } - .into(), - ), - )); + builder = builder.attribute( + IppAttribute::with_name("copies", IppValue::Integer(settings.copies as i32)) + .map_err(ipp_parse_error)?, + ); + builder = builder.attribute( + IppAttribute::with_name( + "media", + IppValue::Keyword( + settings + .paper_size + .ipp_media_keyword() + .try_into() + .map_err(ipp_parse_error)?, + ), + ) + .map_err(ipp_parse_error)?, + ); + builder = builder.attribute( + IppAttribute::with_name( + "sides", + IppValue::Keyword( + settings + .duplex + .ipp_sides_keyword() + .try_into() + .map_err(ipp_parse_error)?, + ), + ) + .map_err(ipp_parse_error)?, + ); + builder = builder.attribute( + IppAttribute::with_name( + "orientation-requested", + IppValue::Enum(settings.orientation.ipp_enum_value()), + ) + .map_err(ipp_parse_error)?, + ); + builder = builder.attribute( + IppAttribute::with_name( + "print-color-mode", + IppValue::Keyword( + if settings.color { + "color" + } else { + "monochrome" + } + .try_into() + .map_err(ipp_parse_error)?, + ), + ) + .map_err(ipp_parse_error)?, + ); // Page ranges (1-indexed, inclusive) if let Some(ref range) = settings.page_range { - builder = builder.attribute(IppAttribute::new( - "page-ranges", - IppValue::RangeOfInteger { - min: range.start as i32, - max: range.end as i32, - }, - )); + builder = builder.attribute( + IppAttribute::with_name( + "page-ranges", + IppValue::RangeOfInteger { + min: range.start as i32, + max: range.end as i32, + }, + ) + .map_err(ipp_parse_error)?, + ); } - let operation = builder.build(); + let operation = builder.build().map_err(ipp_parse_error)?; let client = AsyncIppClient::new(self.uri.clone()); info!( @@ -212,7 +243,9 @@ impl IppClient { /// Retrieve the list of jobs currently known to the printer. #[instrument(skip(self), fields(uri = %self.uri))] pub async fn get_jobs(&self) -> Result> { - let operation = IppOperationBuilder::get_jobs(self.uri.clone()).build(); + let operation = IppOperationBuilder::get_jobs(self.uri.clone()) + .build() + .map_err(ipp_parse_error)?; let client = AsyncIppClient::new(self.uri.clone()); debug!("sending Get-Jobs"); @@ -244,7 +277,9 @@ impl IppClient { /// Returns `Ok(())` if the printer accepted the cancellation. #[instrument(skip(self), fields(uri = %self.uri, job_id))] pub async fn cancel_job(&self, job_id: i32) -> Result<()> { - let operation = IppOperationBuilder::cancel_job(self.uri.clone(), job_id).build(); + let operation = IppOperationBuilder::cancel_job(self.uri.clone(), job_id) + .build() + .map_err(ipp_parse_error)?; let client = AsyncIppClient::new(self.uri.clone()); info!(job_id, "sending Cancel-Job"); @@ -278,6 +313,10 @@ impl IppClient { // Helper functions for parsing IPP responses // --------------------------------------------------------------------------- +fn ipp_parse_error(error: IppParseError) -> PresswerkError { + PresswerkError::IppRequest(format!("invalid IPP operation: {error}")) +} + /// Flatten all attribute groups in an IPP response into a single map. /// /// Multi-valued attributes are joined with `", "`. This intentionally @@ -286,7 +325,7 @@ fn flatten_attributes(attrs: &IppAttributes) -> PrinterAttributes { let mut map = HashMap::new(); for group in attrs.groups() { for (name, attr) in group.attributes() { - map.insert(name.clone(), format!("{}", attr.value())); + map.insert(name.to_string(), format!("{}", attr.value())); } } map @@ -354,6 +393,22 @@ mod tests { assert!(result.is_err()); } + #[tokio::test] + async fn print_job_rejects_overlong_job_name() { + let client = IppClient::new("ipp://127.0.0.1:631/ipp/print").expect("valid URI"); + let result = client + .print_job( + Vec::new(), + DocumentType::Pdf, + &"x".repeat(256), + &PrintSettings::default(), + ) + .await; + assert!( + matches!(result, Err(PresswerkError::IppRequest(detail)) if detail.contains("invalid IPP operation")) + ); + } + #[test] fn new_accepts_valid_ipp_uri() { let client = IppClient::new("ipp://192.168.1.100:631/ipp/print"); diff --git a/crates/presswerk-print/src/ipp_server.rs b/crates/presswerk-print/src/ipp_server.rs index 5f51878..bc99556 100644 --- a/crates/presswerk-print/src/ipp_server.rs +++ b/crates/presswerk-print/src/ipp_server.rs @@ -1795,7 +1795,6 @@ mod tests { let Some(req) = result else { panic!("parse_http_envelope failed") }; - let req = req; assert_eq!(req.content_length, Some(42)); assert!(req.body_offset > 0); assert_eq!(&http[req.body_offset..], b""); diff --git a/crates/presswerk-print/src/retry.rs b/crates/presswerk-print/src/retry.rs index 4aad80a..b99c13b 100644 --- a/crates/presswerk-print/src/retry.rs +++ b/crates/presswerk-print/src/retry.rs @@ -55,6 +55,7 @@ pub fn classify_error(err: &PresswerkError) -> ErrorClass { // User action needed PresswerkError::NoPrinterSelected => ErrorClass::UserAction, + PresswerkError::InvalidState(_) => ErrorClass::UserAction, // Permanent — wrong format, bad data, platform missing PresswerkError::UnsupportedDocument(_) => ErrorClass::Permanent, @@ -222,6 +223,15 @@ mod tests { )); } + #[test] + fn invalid_state_requires_user_action() { + let err = PresswerkError::InvalidState("server already running".into()); + assert!(matches!( + should_retry(&err, 0, &RetryConfig::default()), + RetryDecision::GiveUp(ErrorClass::UserAction) + )); + } + #[test] fn delay_increases_with_attempts() { let config = RetryConfig::default(); diff --git a/crates/presswerk-security/benches/security_bench.rs b/crates/presswerk-security/benches/security_bench.rs index 494886a..b3dc7c0 100644 --- a/crates/presswerk-security/benches/security_bench.rs +++ b/crates/presswerk-security/benches/security_bench.rs @@ -4,7 +4,9 @@ // Criterion benchmarks for encryption, integrity hashing, and audit logging // in the presswerk-security crate. -use criterion::{Criterion, black_box, criterion_group, criterion_main}; +use std::hint::black_box; + +use criterion::{Criterion, criterion_group, criterion_main}; use presswerk_security::{AuditLog, EncryptedStorage, hash_bytes};