audio: parse object.serial as u64 (phase 0a)
`object.serial` is a 64-bit PipeWire counter, not a u32 object id. Parsing it with `parse::<u32>()` 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<u64>` (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<u64>` - log a warning when the sink's serial is unusable instead of returning silently - audit the other `parse::<u32>` 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 <noreply@anthropic.com>
This commit is contained in:
+96
-5
@@ -365,6 +365,9 @@ fn load_module(args: &[&str]) -> Result<u32> {
|
||||
.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::<u32>()
|
||||
.with_context(|| format!("pactl returned unexpected module ID: {id_str:?}"))
|
||||
@@ -532,14 +535,22 @@ 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::<u32>().ok())
|
||||
{
|
||||
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("<absent>"),
|
||||
"audio routing: pixelpass sink has no usable object.serial; \
|
||||
stream rerouting disabled"
|
||||
),
|
||||
}
|
||||
return;
|
||||
}
|
||||
if props.get("media.class") != Some("Stream/Output/Audio") {
|
||||
@@ -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<u64> {
|
||||
if raw.is_empty() || !raw.bytes().all(|b| b.is_ascii_digit()) {
|
||||
return None;
|
||||
}
|
||||
raw.parse::<u64>().ok()
|
||||
}
|
||||
|
||||
struct RouterState {
|
||||
sink_serial: Option<u32>,
|
||||
/// See [`parse_object_serial`] — 64-bit, and not interchangeable with
|
||||
/// the `u32` node ids in `routed_node_ids` / `pending`.
|
||||
sink_serial: Option<u64>,
|
||||
default_metadata: Option<pipewire::metadata::Metadata>,
|
||||
routed_node_ids: Vec<u32>,
|
||||
pending: Vec<u32>,
|
||||
@@ -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:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user