From 9b6c8bb5c32fc082acfc19945a7250600af44d1a Mon Sep 17 00:00:00 2001 From: Mollusk Date: Tue, 21 Jul 2026 15:47:30 -0400 Subject: [PATCH 1/2] audio: parse object.serial as u64 (phase 0a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `object.serial` is a 64-bit PipeWire counter, not a u32 object id. Parsing it with `parse::()` returns None past u32::MAX, which silently leaves `RouterState::sink_serial` unset — `try_flush` then routes nothing and app-filter mode is dead with no diagnostic. - factor the parse into a pure `parse_object_serial(&str) -> Option` (strict decimal; rejects signs, padding, overflow) with unit tests at the u32 boundary, past it, and at u64::MAX - widen `RouterState::sink_serial` to `Option` - log a warning when the sink's serial is unusable instead of returning silently - audit the other `parse::` in this file: `load_module` returns a PulseAudio module index (uint32_t), genuinely 32-bit — annotated, not changed Prerequisite for the taint engine's lifetime-awareness, which is keyed on object.serial (screenshare-audio-exclusion-impl-plan.md §1, §2/0a). Co-Authored-By: Claude Opus 4.8 --- src/host/audio.rs | 107 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 99 insertions(+), 8 deletions(-) diff --git a/src/host/audio.rs b/src/host/audio.rs index 8eaa342..b62229e 100644 --- a/src/host/audio.rs +++ b/src/host/audio.rs @@ -365,6 +365,9 @@ fn load_module(args: &[&str]) -> Result { .context("pactl returned non-UTF-8")? .trim() .to_string(); + // Genuinely 32-bit, unlike `object.serial`: this is a PulseAudio module + // index (`pa_module.index`, `uint32_t`), which `pactl unload-module` takes + // back verbatim. Do not widen it. id_str .parse::() .with_context(|| format!("pactl returned unexpected module ID: {id_str:?}")) @@ -532,13 +535,21 @@ fn run_router( return; }; if props.get("node.name") == Some(sink_name_owned.as_str()) { - if let Some(serial) = props - .get("object.serial") - .and_then(|s| s.parse::().ok()) - { - state_for_reg.borrow_mut().sink_serial = Some(serial); - tracing::info!(serial, "audio routing: pixelpass sink registered"); - try_flush(&state_for_reg, &event_tx_for_reg); + match props.get("object.serial").and_then(parse_object_serial) { + Some(serial) => { + state_for_reg.borrow_mut().sink_serial = Some(serial); + tracing::info!(serial, "audio routing: pixelpass sink registered"); + try_flush(&state_for_reg, &event_tx_for_reg); + } + // Never silently: without a serial `try_flush` can + // never route anything, so the whole app-filter mode + // is dead and the only symptom is missing audio. + None => tracing::warn!( + node_id = obj.id, + serial = props.get("object.serial").unwrap_or(""), + "audio routing: pixelpass sink has no usable object.serial; \ + stream rerouting disabled" + ), } return; } @@ -591,8 +602,29 @@ fn run_router( Ok(()) } +/// Parse a PipeWire `object.serial` property value. +/// +/// `object.serial` is a **64-bit** monotonically-increasing counter +/// (`pw_global`'s serial is `uint64_t`); it is *not* a `pw` object id +/// (those are `u32` and get recycled — the serial exists precisely so +/// that recycled ids can be disambiguated). Parsing it as `u32` silently +/// yields `None` past `u32::MAX`, which on a long-lived daemon means the +/// sink is never registered and no stream is ever routed. +/// +/// Strict on purpose: PipeWire emits a bare decimal, so anything else +/// (empty, signed, padded, non-numeric) is a property we do not +/// understand and must not guess at. +fn parse_object_serial(raw: &str) -> Option { + if raw.is_empty() || !raw.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + raw.parse::().ok() +} + struct RouterState { - sink_serial: Option, + /// See [`parse_object_serial`] — 64-bit, and not interchangeable with + /// the `u32` node ids in `routed_node_ids` / `pending`. + sink_serial: Option, default_metadata: Option, routed_node_ids: Vec, pending: Vec, @@ -654,3 +686,62 @@ fn try_flush( let _ = event_tx.send(Event::FirstRoutedStream); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn object_serial_parses_past_u32() { + // The regression this fix exists for: a serial one past `u32::MAX` + // used to parse as `None` and silently disable rerouting. + let beyond = u64::from(u32::MAX) + 1; + assert_eq!(parse_object_serial(&beyond.to_string()), Some(beyond)); + assert_eq!( + parse_object_serial(&u64::MAX.to_string()), + Some(u64::MAX), + "the full 64-bit range must round-trip" + ); + } + + #[test] + fn object_serial_boundary_values() { + assert_eq!(parse_object_serial("0"), Some(0)); + assert_eq!(parse_object_serial("1"), Some(1)); + let max32 = u64::from(u32::MAX); + assert_eq!(parse_object_serial(&max32.to_string()), Some(max32)); + assert_eq!( + parse_object_serial(&(max32 - 1).to_string()), + Some(max32 - 1) + ); + } + + #[test] + fn object_serial_round_trips_through_the_metadata_string() { + // `try_flush` writes the serial back out as a decimal string for + // `target.object`; widening must not introduce a formatting change. + for raw in ["0", "4294967296", "18446744073709551615"] { + let parsed = parse_object_serial(raw).expect("valid serial"); + assert_eq!(parsed.to_string(), raw); + } + } + + #[test] + fn object_serial_rejects_malformed() { + for raw in [ + "", + " 12", + "12 ", + "+12", + "-1", + "1.0", + "0x10", + "12a", + "abc", + // u64::MAX + 1 — overflow must be rejected, not wrapped. + "18446744073709551616", + ] { + assert_eq!(parse_object_serial(raw), None, "should reject {raw:?}"); + } + } +} From 87de5213fef9ca864a5d0f47c1c785306b885961 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Tue, 21 Jul 2026 16:00:34 -0400 Subject: [PATCH 2/2] audio: cover ordinary serial lengths in parse tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 1 (P3): the valid cases were only 1, 10 and 20 digits long, so `if (2..10).contains(&raw.len()) { return None }` survived all four tests while rejecting every serial a freshly started daemon hands out. Verified: that mutant passes the old suite and fails the new test. Also corrects the doc comment — leading zeroes are accepted (harmless and unambiguous), only whitespace padding is rejected. Co-Authored-By: Claude Opus 4.8 --- src/host/audio.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/host/audio.rs b/src/host/audio.rs index b62229e..b782857 100644 --- a/src/host/audio.rs +++ b/src/host/audio.rs @@ -612,8 +612,9 @@ fn run_router( /// sink is never registered and no stream is ever routed. /// /// Strict on purpose: PipeWire emits a bare decimal, so anything else -/// (empty, signed, padded, non-numeric) is a property we do not -/// understand and must not guess at. +/// (empty, signed, whitespace-padded, non-numeric, overflowing) is a +/// property we do not understand and must not guess at. Leading zeroes +/// are accepted — they are unambiguous and parse to the same value. fn parse_object_serial(raw: &str) -> Option { if raw.is_empty() || !raw.bytes().all(|b| b.is_ascii_digit()) { return None; @@ -704,6 +705,23 @@ mod tests { ); } + #[test] + fn object_serial_accepts_ordinary_serials() { + // Without this the valid cases are only 1, 10 and 20 digits long, and + // a length-gated mutant (`if (2..10).contains(&raw.len()) { None }`) + // survives the whole suite while rejecting every serial a freshly + // started daemon actually hands out. (Codex, round 1.) + for serial in 0_u64..=1024 { + assert_eq!(parse_object_serial(&serial.to_string()), Some(serial)); + } + assert_eq!(parse_object_serial("123456789"), Some(123_456_789)); + assert_eq!( + parse_object_serial("007"), + Some(7), + "leading zeroes are fine" + ); + } + #[test] fn object_serial_boundary_values() { assert_eq!(parse_object_serial("0"), Some(0));