From d4c913e0d325ac54f7c8f7386a2dd7933081afb4 Mon Sep 17 00:00:00 2001
From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com>
Date: Wed, 29 Jul 2026 19:00:28 -0700
Subject: [PATCH] Validate stream-selection PIDs per class, not across both
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

StreamSelection::apply validated a listed PID by scanning ALL streams, so a PID
named in the wrong class's filter passed validation — an audio filter listing a
subtitle PID, say. `keeps` then matched it against the audio streams only, so
the requested track was silently absent from the output. That is precisely the
outcome this validation documents itself as preventing: "fail loud rather than
silently emit an MKV missing a requested track".

Each filter is now checked against its own stream class. The `listed_pids`
helper existed only for the cross-class scan and is removed rather than left
behind as dead code.

Test covers both directions plus the sanity case, and asserts a rejected
selection leaves the title unpruned.
---
 src/mux/select.rs | 89 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 73 insertions(+), 16 deletions(-)

diff --git a/src/mux/select.rs b/src/mux/select.rs
index 8191232..6ceedb7 100644
--- a/src/mux/select.rs
+++ b/src/mux/select.rs
@@ -64,10 +64,33 @@ impl StreamSelection {
 
         // Validate every listed PID exists in the title before mutating, so an
         // unknown PID leaves the title untouched (no partial prune).
-        for pid in self.listed_pids() {
-            let present = title.streams.iter().any(|s| stream_pid(s) == Some(pid));
-            if !present {
-                return Err(Error::SelectionPidUnknown { pid });
+        //
+        // Validate PER CLASS. Scanning both classes let a PID listed in the WRONG
+        // filter pass validation — an audio filter naming a subtitle PID, say —
+        // and `keeps` then matches it against the audio streams only, so the
+        // requested track is silently absent from the output. That is exactly the
+        // "fail loud rather than silently emit an MKV missing a requested track"
+        // contract this validation exists to enforce.
+        if let PidFilter::Only(pids) = &self.audio {
+            for &pid in pids {
+                let present = title
+                    .streams
+                    .iter()
+                    .any(|s| matches!(s, Stream::Audio(_)) && stream_pid(s) == Some(pid));
+                if !present {
+                    return Err(Error::SelectionPidUnknown { pid });
+                }
+            }
+        }
+        if let PidFilter::Only(pids) = &self.subtitle {
+            for &pid in pids {
+                let present = title
+                    .streams
+                    .iter()
+                    .any(|s| matches!(s, Stream::Subtitle(_)) && stream_pid(s) == Some(pid));
+                if !present {
+                    return Err(Error::SelectionPidUnknown { pid });
+                }
             }
         }
 
@@ -105,18 +128,6 @@ impl StreamSelection {
             Stream::Subtitle(s) => filter_keeps(&self.subtitle, s.pid),
         }
     }
-
-    /// Every PID explicitly listed across both filters (for existence checking).
-    fn listed_pids(&self) -> Vec<u16> {
-        let mut v = Vec::new();
-        if let PidFilter::Only(pids) = &self.audio {
-            v.extend_from_slice(pids);
-        }
-        if let PidFilter::Only(pids) = &self.subtitle {
-            v.extend_from_slice(pids);
-        }
-        v
-    }
 }
 
 fn filter_keeps(filter: &PidFilter, pid: u16) -> bool {
@@ -296,4 +307,50 @@ mod tests {
             "codec_privates pruned to match the retained streams, in order"
         );
     }
+    /// A PID listed in the WRONG class's filter must fail loud, not validate and
+    /// then quietly vanish. Validation used to scan both audio and subtitle
+    /// streams, so an audio filter naming a subtitle PID passed — and `keeps`
+    /// then matched it against audio streams only, dropping the requested track
+    /// from the output with no error. That defeats the documented "fail loud
+    /// rather than silently emit an MKV missing a requested track" contract.
+    #[test]
+    fn a_pid_listed_in_the_wrong_class_filter_is_rejected() {
+        let mut t = title();
+        let before = t.streams.len();
+
+        // 0x1200 is a SUBTITLE pid, listed here in the AUDIO filter.
+        let sel = StreamSelection {
+            audio: PidFilter::Only(vec![0x1200]),
+            subtitle: PidFilter::All,
+        };
+        assert!(
+            sel.apply(&mut t).is_err(),
+            "a subtitle PID in the audio filter must be rejected"
+        );
+        assert_eq!(
+            t.streams.len(),
+            before,
+            "a rejected selection must not prune"
+        );
+
+        // And the mirror case: an audio pid listed in the subtitle filter.
+        let sel = StreamSelection {
+            audio: PidFilter::All,
+            subtitle: PidFilter::Only(vec![0x1100]),
+        };
+        assert!(
+            sel.apply(&mut t).is_err(),
+            "an audio PID in the subtitle filter must be rejected"
+        );
+
+        // Sanity: each PID in its OWN class still validates.
+        let sel = StreamSelection {
+            audio: PidFilter::Only(vec![0x1100]),
+            subtitle: PidFilter::Only(vec![0x1200]),
+        };
+        assert!(
+            sel.apply(&mut t).is_ok(),
+            "correctly-classed PIDs must apply"
+        );
+    }
 }