core: trim before sanitising GC text, cover scan-failure key restore, doc sampler stop

src/disc/mod.rs

@@ -18,7 +18,7 @@ mod hddvd;
 pub(crate) mod pgs_forced_probe;
 pub mod profile;
 #[cfg(test)]
-mod scan_order_tests;
+pub(crate) mod scan_order_tests;
 
 use crate::drive::Drive;
 use crate::error::{Error, Result};

src/disc/scan_order_tests.rs

@@ -256,6 +256,11 @@ fn bd_disc(aacs: Option<bool>) -> MemDisc {
     mem
 }
 
+// A drive over an unencrypted BD that scans successfully.
+pub(crate) fn scannable_drive() -> Drive {
+    Rig::new(bd_disc(None), |_| {}).drive
+}
+
 struct Rig {
     drive: Drive,
     log: Log,

src/error.rs

@@ -1651,7 +1651,8 @@ mod tests {
         assert!(!is_disc_level_no_key(&per_title));
     }
 
-    // A typed payload is read directly; a bare io::Error keeps the string fallback.
+    // Pin, not a guard: typed and string paths agree for every `Error` (Display leads
+    // with its code). The fallback stays for consumers' string-coded io::Errors.
     #[test]
     fn error_code_reads_the_typed_payload() {
         let typed: std::io::Error = Error::Halted.into();

src/identity.rs

@@ -172,9 +172,8 @@ impl std::fmt::Display for DriveId {
 
 // A GET CONFIGURATION text field: trims spaces and the NUL padding `trim` keeps.
 fn gc_text(field: &[u8]) -> String {
-    printable(&String::from_utf8_lossy(field))
-        .trim_matches(|c: char| c.is_whitespace() || c == '\0')
-        .to_string()
+    let text = String::from_utf8_lossy(field);
+    printable(text.trim_matches(|c: char| c.is_whitespace() || c == '\0'))
 }
 
 // Drive-supplied text is untrusted: control characters (bar NUL padding) become `?`
@@ -298,7 +297,8 @@ mod tests {
         assert!(DriveId::from_drive(&mut script(None)).is_ok());
     }
 
-    // A lying INQUIRY transfer count is clamped to the buffer, not trusted.
+    // A lying INQUIRY transfer count is not trusted. Pin only: `truncate` past the
+    // length is a no-op, so the `.min` clamp is behaviour-neutral.
     #[test]
     fn from_drive_clamps_oversized_inquiry_count() {
         let mut t = script(None);
@@ -315,6 +315,8 @@ mod tests {
         let id = DriveId::from_drive(&mut t).expect("from_drive");
         assert_eq!(id.vendor_id, "A?[31mZ ");
         assert_eq!(gc_text(b"SN\r\n1\x1b "), "SN??1?");
+        // Trailing padding is trimmed before sanitising, not turned into '?'.
+        assert_eq!(gc_text(b"SN1\r\n\t\0 "), "SN1");
     }
 
     // INQUIRY byte 0: low 5 bits are the peripheral device type (5 = MMC), the

src/keysource.rs

@@ -368,6 +368,7 @@ pub trait KeySource {
 /// candidate against.
 /// Lives in the library, not a key-source crate: carving units is decryption *mechanism*.
 /// "Encrypted" is the AACS CPI (`buf[0] & 0xc0`), NOT the `is_clean` TS-sync heuristic.
+/// A Stop ends sampling early with the partial set; callers must check their own `Halt`.
 pub fn read_encrypted_units(
     reader: &mut dyn crate::sector::SectorSource,
     title: &crate::disc::DiscTitle,

src/session/stop_tests.rs

@@ -240,3 +240,44 @@ fn failed_scan_keeps_the_key_sources() {
         assert_eq!(s.spec.key_sources.len(), 1);
     }
 }
+
+fn keyed_session(t: FakeTransport, halt: Option<Halt>) -> DiscSession {
+    let drive = Drive::from_transport(Box::new(t));
+    let spec = KeySpec {
+        key_sources: vec![Box::new(NoKeys)],
+        ..KeySpec::default()
+    };
+    DiscSession::bring_up(drive, spec, halt).expect("brought up")
+}
+
+/// A `Disc::scan` failure on a present drive (not the missing-drive arm) keeps the sources.
+#[test]
+fn scan_error_on_a_faulting_drive_keeps_the_key_sources() {
+    let (t, _fake) = FakeTransport::new();
+    let mut s = keyed_session(t, None);
+    s.drive = Some(Drive::from_transport(Box::new(
+        FakeTransport::new()
+            .0
+            .rule(|_| true, crate::test_util::FakeMode::Fault),
+    )));
+    assert!(s.scan(ScanOptions::default()).is_err());
+    assert_eq!(s.spec.key_sources.len(), 1);
+    assert!(s.scan_with(ScanOptions::default()).is_err());
+    assert_eq!(s.spec.key_sources.len(), 1);
+}
+
+/// `scan_with`'s own post-scan halt check (a Stop after a scan that succeeded) keeps the
+/// sources too: the drive's own token is live, only the session's op token is cancelled.
+#[test]
+fn scan_with_stop_after_scan_keeps_the_key_sources() {
+    use crate::disc::scan_order_tests::scannable_drive;
+    let h = Halt::new();
+    let (t, _fake) = FakeTransport::new();
+    let mut s = keyed_session(t, Some(h.clone()));
+    s.drive = Some(scannable_drive());
+    h.cancel();
+    let r = s.scan_with(ScanOptions::default());
+    assert!(matches!(r, Err(Error::Halted)), "{:?}", r.err());
+    assert!(s.disc.is_none());
+    assert_eq!(s.spec.key_sources.len(), 1);
+}