diff --git a/CHANGELOG.md b/CHANGELOG.md index daedc66..a8a129e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ all seven languages, and a Codes-page entry. Messages are source-agnostic ("key source", never a specific database). +### Changed + +- keydb download/save moved out of the library into freemkv-keysources; + libfreemkv no longer has any keydb I/O (it already held no keys). + ### Fixed - **DVD rips now start on the movie, not the disc menu.** A VTS title VOB's @@ -74,21 +79,16 @@ ### Fixed -- **Windows Explorer now reports the full 25 fps for interlaced SD-DVD.** - rc.5.1 added a `DefaultDecodedFieldDuration` (20 ms field) element to the - 576i/480i track header on the theory that Windows derives fps from it. The - captured Silence-of-the-Lambs evidence proved the opposite: with - `FlagInterlaced=1` + `DefaultDuration=40 ms` + `DefaultDecodedFieldDuration=20 ms`, - Windows Explorer reported 12.5 fps (half) and MediaInfo flipped the track to - "Frame rate mode: Variable". MakeMKV's correct rip of the same disc OMITS - `DefaultDecodedFieldDuration`, keeps `FlagInterlaced=1` + `FieldOrder=TFF` + - full-frame `DefaultDuration` (40 ms), and Explorer shows the full 25 fps with - MediaInfo "Constant". The element is no longer written (`MkvTrack::video` now - passes `field_duration_ns == 0`); the only frame-rate signal tools trust, - `1/DefaultDuration` = 25 fps, is the full-frame value. Interlace signalling - (`FlagInterlaced=1`, `FieldOrder=TFF`) is retained, and MediaInfo still - reports "Interlaced / Top Field First" because it reads scan type from the - MPEG-2 elementary stream's picture coding extension, not the container flag. +- **Reverted the rc.5.1 `DefaultDecodedFieldDuration` experiment for interlaced + SD-DVD.** rc.5.1 added a 20 ms `DefaultDecodedFieldDuration` field element to + the 576i/480i track header on the theory that Windows derives fps from it. + Captured evidence showed that element made Windows Explorer report 12.5 fps + (half) and MediaInfo flip the track to "Frame rate mode: Variable", while + MakeMKV's rip of the same disc omits it. The element is therefore no longer + written (`MkvTrack::video` now passes `field_duration_ns == 0`); the track + keeps `FlagInterlaced=1` + `FieldOrder=TFF` and the full-frame 40 ms + `DefaultDuration` (`1/DefaultDuration` = 25 fps), matching MakeMKV. How a given + player or shell handler chooses to display interlaced fps is not guaranteed. - **Correct AC-3 audio track selected on DVDs with non-standard sub-stream ordering.** freemkv assigned each declared audio stream a physical sub-stream by ordinal (`0x80+n`), assuming the IFO's first stream lives at `0x80`. On diff --git a/Cargo.toml b/Cargo.toml index 9dc3b15..a6caf3e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,7 +22,6 @@ sha1 = "0.10" sha2 = "0.10" aes = "0.8" cbc = "0.1" -flate2 = "1" num-bigint = "0.4" num-traits = "0.2" num-integer = "0.1" diff --git a/src/keydb.rs b/src/keydb.rs deleted file mode 100644 index e598a97..0000000 --- a/src/keydb.rs +++ /dev/null @@ -1,869 +0,0 @@ -//! KEYDB.cfg updater — HTTP GET, unzip, verify, save. -//! -//! Zero external HTTP dependencies. Raw TCP for HTTP GET. -//! Uses `zip` and `flate2` (already in deps) for extraction. - -use crate::error::{Error, Result}; -use std::io::{Read, Write}; -use std::net::{TcpStream, ToSocketAddrs}; -use std::path::PathBuf; -use std::time::Duration; - -/// Network operation timeout (connect / read / write). Keeps the daily -/// refresh thread from blocking indefinitely on an unresponsive mirror. -const NET_TIMEOUT: Duration = Duration::from_secs(10); - -/// Read timeout — longer than connect/write since the keydb body can be -/// several MiB over a slow link. -const READ_TIMEOUT: Duration = Duration::from_secs(30); - -/// Maximum redirects to follow before giving up. -const MAX_REDIRECTS: usize = 5; - -/// Upper bound on decompressed keydb size. The published keydb is a few -/// MiB; 64 MiB is a generous ceiling that still caps a decompression -/// bomb (a tiny zip/gz can otherwise inflate to GiB and OOM the daily -/// refresh thread). -const MAX_KEYDB_BYTES: u64 = 64 * 1024 * 1024; - -/// Read a decompressed stream into a String with a hard size ceiling. -/// Returns `Error::KeydbInvalid` if the input exceeds the cap, or -/// `Error::KeydbParse` if the bytes are not valid UTF-8. -fn read_capped_to_string(reader: R) -> Result { - let mut buf = Vec::new(); - // Read one byte past the cap so an exactly-at-cap stream is accepted - // but anything larger is rejected. - reader - .take(MAX_KEYDB_BYTES + 1) - .read_to_end(&mut buf) - .map_err(|_| Error::KeydbParse)?; - if buf.len() as u64 > MAX_KEYDB_BYTES { - return Err(Error::KeydbInvalid); - } - String::from_utf8(buf).map_err(|_| Error::KeydbParse) -} - -/// Build the error returned when the executable's own directory can't be -/// determined (`std::env::current_exe()` fails or has no parent). This is an -/// *environment* failure — the process can't locate itself, which typically -/// signals a stripped container or CI configuration — not a corrupt or -/// unparseable keydb file. Map it to a `NotFound` I/O error so -/// display/remediation paths never claim a keydb parse failure for a file -/// that was never consulted. -fn no_home_dir() -> Error { - Error::IoError { - source: std::io::Error::from(std::io::ErrorKind::NotFound), - } -} - -/// Standard keydb storage path — the canonical location to write the keydb to. -/// -/// The keydb lives *next to the executable*: `/keydb.cfg`, -/// where the directory is `std::env::current_exe()`'s parent. This makes -/// freemkv a portable, standalone binary — drop the exe and its `keydb.cfg` -/// in the same folder and it works, with no OS-specific config dir -/// (`%APPDATA%`, `~/.config`, XDG) involved at all. -/// -/// The CLI's read-side search lives in -/// `freemkv-keysources::keydb_search_paths`; it resolves the same exe-local -/// location, so the *write* default used by `save`/`update` and the read-side -/// search always agree. -/// -/// Returns a `NotFound` I/O error (via [`no_home_dir`]) if the executable's -/// own directory can't be determined. -pub fn default_path() -> Result { - std::env::current_exe() - .ok() - .and_then(|exe| exe.parent().map(|dir| dir.join("keydb.cfg"))) - .ok_or_else(no_home_dir) -} - -/// Download a KEYDB from a URL, verify, save to the standard path. -pub fn update(url: &str) -> Result { - let body = http_get(url)?; - save(&body) -} - -/// Verify and save raw keydb bytes (plain text, .zip, or .gz). -pub fn save(data: &[u8]) -> Result { - let text = if data.starts_with(b"PK\x03\x04") { - extract_zip(data)? - } else if data.starts_with(&[0x1f, 0x8b]) { - read_capped_to_string(flate2::read::GzDecoder::new(data))? - } else { - // Plain-text body: route through the same capped reader as the gz/zip - // branches so an oversized uncompressed upload can't bypass - // MAX_KEYDB_BYTES. - read_capped_to_string(std::io::Cursor::new(data))? - }; - - let entries = text - .lines() - .filter(|l| { - let t = l.trim(); - t.starts_with("0x") - || t.starts_with("| DK") - || t.starts_with("| PK") - || t.starts_with("| HC") - }) - .count(); - - if entries == 0 { - return Err(Error::KeydbInvalid); - } - - let path = default_path()?; - write_atomic(&path, &text)?; - - Ok(UpdateResult { - path, - entries, - bytes: text.len(), - }) -} - -/// Write `text` to `path` crash-safely (create parent dir, write a sibling -/// temp file, fsync, then atomic rename). -/// -/// keydb.cfg is the single source of AACS truth, and `save`/`update` run -/// unattended (first-boot download + daily-refresh thread, with a container -/// restart on every release). A bare in-place `fs::write` truncates the file -/// before writing, so a SIGKILL (docker stop's grace window), OOM-kill, power -/// loss, or ENOSPC mid-write would leave the keydb half-written — the prior -/// good copy already gone. A truncated keydb doesn't error at write time; it -/// silently breaks key resolution on every later AACS rip. Writing to a temp -/// file then renaming (POSIX rename is atomic within a filesystem) means an -/// interrupted update leaves the previous keydb fully intact. -/// -/// The fsync MUST succeed before the rename: a `sync_all` failure (ENOSPC, -/// ESTALE on the bind-mounted volume) means the kernel never guaranteed the -/// bytes reached stable storage, so publishing them via rename would defeat -/// crash-safety. The temp name is unique per call (pid + monotonic counter) -/// so a concurrent update can't share a fixed temp path and rename a mangled -/// file over the keydb. -fn write_atomic(path: &std::path::Path, text: &str) -> Result<()> { - let werr = || Error::KeydbWrite { - path: path.display().to_string(), - }; - if let Some(dir) = path.parent() { - std::fs::create_dir_all(dir).map_err(|e| { - tracing::warn!(error = %e, path = %path.display(), "keydb dir create failed"); - werr() - })?; - } - let tmp = { - use std::sync::atomic::{AtomicU64, Ordering}; - static TMP_COUNTER: AtomicU64 = AtomicU64::new(0); - path.with_extension(format!( - "tmp.{}.{}", - std::process::id(), - TMP_COUNTER.fetch_add(1, Ordering::Relaxed) - )) - }; - let write_result = (|| -> std::io::Result<()> { - let mut f = std::fs::File::create(&tmp)?; - f.write_all(text.as_bytes())?; - f.sync_all()?; - Ok(()) - })(); - if let Err(e) = write_result { - let _ = std::fs::remove_file(&tmp); - tracing::warn!(error = %e, path = %path.display(), "keydb write/fsync failed; keydb unchanged"); - return Err(werr()); - } - if let Err(e) = std::fs::rename(&tmp, path) { - let _ = std::fs::remove_file(&tmp); - tracing::warn!(error = %e, path = %path.display(), "keydb rename failed; keydb unchanged"); - return Err(werr()); - } - // Durably commit the new dirent: on POSIX filesystems (ext2, some NFS) a - // crash right after the rename can lose the directory entry even though the - // rename returned. Best-effort (swallowed on failure); no-op on Windows. - if let Some(dir) = path.parent() { - crate::io::fsync::dir(dir); - } - Ok(()) -} - -/// Result of a KEYDB update -- path written, entry count, and byte size. -#[derive(Debug)] -pub struct UpdateResult { - pub path: PathBuf, - pub entries: usize, - pub bytes: usize, -} - -fn http_get(url: &str) -> Result> { - let (mut host, mut port, mut path) = parse_url(url)?; - - for _ in 0..MAX_REDIRECTS { - // Resolve to a concrete socket address so we can bound the connect - // with connect_timeout (plain connect() uses the OS default, which - // can be minutes). - let addr = (host.as_str(), port) - .to_socket_addrs() - .ok() - .and_then(|mut it| it.next()) - .ok_or_else(|| Error::KeydbConnect { host: host.clone() })?; - let mut stream = TcpStream::connect_timeout(&addr, NET_TIMEOUT).map_err(|e| { - tracing::debug!(error = %e, host = %host, "keydb connect failed"); - Error::KeydbConnect { host: host.clone() } - })?; - stream - .set_read_timeout(Some(READ_TIMEOUT)) - .map_err(|_| Error::KeydbConnect { host: host.clone() })?; - stream - .set_write_timeout(Some(NET_TIMEOUT)) - .map_err(|_| Error::KeydbConnect { host: host.clone() })?; - - // HTTP/1.0 forces close-delimited framing: the server cannot reply - // with Transfer-Encoding: chunked, so the raw body is the keydb - // bytes with no chunk-size lines to de-frame. - let request = format!( - "GET {path} HTTP/1.0\r\nHost: {host}\r\nConnection: close\r\nAccept-Encoding: identity\r\n\r\n" - ); - stream - .write_all(request.as_bytes()) - .map_err(|_| Error::KeydbConnect { host: host.clone() })?; - - // Read the header block incrementally up to the \r\n\r\n terminator, - // bounded to ~64 KiB, BEFORE pulling any body. This avoids buffering up - // to 100 MiB per redirect hop just to inspect the status / Location. - const MAX_HEADER_BYTES: usize = 64 * 1024; - let mut reader = std::io::BufReader::new(stream); - let mut header_buf: Vec = Vec::with_capacity(1024); - let mut byte = [0u8; 1]; - loop { - let n = reader - .read(&mut byte) - .map_err(|_| Error::KeydbConnect { host: host.clone() })?; - if n == 0 { - // Server closed the connection before the header block - // completed: a connection/protocol-level fault, not malformed - // keydb content. - return Err(Error::KeydbConnect { host: host.clone() }); - } - header_buf.push(byte[0]); - if header_buf.ends_with(b"\r\n\r\n") { - break; - } - if header_buf.len() >= MAX_HEADER_BYTES { - // Oversized header block from the server: a protocol-level - // fault, not a keydb content parse failure. `>=` caps the - // buffer at exactly MAX_HEADER_BYTES (the `>` form admitted one - // extra byte before tripping). - return Err(Error::KeydbConnect { host: host.clone() }); - } - } - // header_buf includes the trailing \r\n\r\n. - let header_end = header_buf.len() - 4; - // Lossy: a stray non-UTF-8 byte in the header block must not blank - // out the whole status line / Location header (which would surface - // as an undiagnosable KeydbHttp{status:0}). - let headers = String::from_utf8_lossy(&header_buf[..header_end]).into_owned(); - let headers = headers.as_str(); - - let status = parse_status(headers).ok_or(Error::KeydbParse)?; - - // Only treat a Location header as a redirect when the status is - // actually 3xx; a 200 carrying a stray Location (some proxies) is - // not a redirect, and a 3xx without Location is a malformed redirect. - if (300..=399).contains(&status) { - let location = - extract_header(headers, "Location").ok_or(Error::KeydbHttp { status })?; - let (next_host, next_port, next_path) = resolve_redirect(&location, &host, port)?; - host = next_host; - port = next_port; - path = next_path; - continue; - } - - if status != 200 { - return Err(Error::KeydbHttp { status }); - } - - // Now read the body, still bounded by the existing 100 MiB cap. The - // BufReader carries any bytes already buffered past the header. - let mut body = Vec::new(); - reader - .take(100 * 1024 * 1024) - .read_to_end(&mut body) - .map_err(|_| Error::KeydbConnect { host: host.clone() })?; - return Ok(body); - } - - Err(Error::KeydbTooManyRedirects) -} - -/// Resolve a `Location` value against the current request target. -/// Handles absolute `http://` URLs, scheme-relative `//host/path`, -/// absolute paths `/path`, and rejects unsupported schemes (e.g. -/// `https://`, which this dependency-light client cannot fetch) with a -/// diagnosable error rather than a generic parse failure. -fn resolve_redirect( - location: &str, - cur_host: &str, - cur_port: u16, -) -> Result<(String, u16, String)> { - let loc = location.trim(); - - if let Some(rest) = loc.strip_prefix("//") { - // Scheme-relative: //host[:port]/path — inherit http. - return parse_url(&format!("http://{rest}")); - } - if loc.starts_with('/') { - // Absolute path on the same host/port. - return Ok((cur_host.to_string(), cur_port, loc.to_string())); - } - if let Some(scheme) = loc.split("://").next() { - if loc.contains("://") && !scheme.eq_ignore_ascii_case("http") { - return Err(Error::KeydbUnsupportedScheme { - scheme: scheme.to_string(), - }); - } - } - parse_url(loc) -} - -fn parse_url(url: &str) -> Result<(String, u16, String)> { - // Reject non-http(s) up front so the caller gets a scheme diagnostic - // rather than an opaque parse error. - if let Some(scheme) = url.split("://").next() { - if url.contains("://") && !scheme.eq_ignore_ascii_case("http") { - return Err(Error::KeydbUnsupportedScheme { - scheme: scheme.to_string(), - }); - } - } - let url = url.strip_prefix("http://").ok_or(Error::KeydbParse)?; - let (host_port, path) = match url.find('/') { - Some(i) => (&url[..i], &url[i..]), - None => (url, "/"), - }; - let (host, port) = match host_port.find(':') { - Some(i) => { - let port_str = &host_port[i + 1..]; - // A non-empty-but-unparseable port is a malformed URL; only an - // omitted port defaults to 80. - let port = if port_str.is_empty() { - 80 - } else { - port_str.parse().map_err(|_| Error::KeydbParse)? - }; - (&host_port[..i], port) - } - None => (host_port, 80u16), - }; - Ok((host.to_string(), port, path.to_string())) -} - -fn parse_status(headers: &str) -> Option { - headers - .lines() - .next() - .and_then(|l| l.split_whitespace().nth(1)) - .and_then(|s| s.parse().ok()) -} - -/// Locate the end of the HTTP header block (the index of the `\r\n\r\n`). -/// Retained for the framing unit tests; the live path now reads headers -/// incrementally in `http_get` so the whole response is never buffered. -#[cfg_attr(not(test), allow(dead_code))] -fn find_header_end(data: &[u8]) -> Option { - data.windows(4).position(|w| w == b"\r\n\r\n") -} - -fn extract_header(headers: &str, name: &str) -> Option { - // Split on the first ':' rather than byte-indexing at name.len(), - // which would panic on a multibyte UTF-8 codepoint straddling that - // offset (headers are decoded from untrusted network bytes). Also - // accepts single-character values (e.g. "Location:x"). - for line in headers.lines() { - if let Some((key, value)) = line.split_once(':') { - if key.trim().eq_ignore_ascii_case(name) { - return Some(value.trim().to_string()); - } - } - } - None -} - -fn extract_zip(data: &[u8]) -> Result { - let cursor = std::io::Cursor::new(data); - let mut archive = zip::ZipArchive::new(cursor).map_err(|_| Error::KeydbParse)?; - - for i in 0..archive.len() { - let file = archive.by_index(i).map_err(|_| Error::KeydbParse)?; - if file.name().ends_with(".cfg") || file.name().ends_with(".CFG") { - return read_capped_to_string(file); - } - } - - Err(Error::KeydbInvalid) -} - -#[cfg(test)] -mod tests { - use super::*; - - // Per project convention, tests never touch /tmp (wiped on reboot). - // Anchor scratch under the crate's target/ (gitignored), not /tmp. - fn scratch(tag: &str) -> std::path::PathBuf { - use std::sync::atomic::{AtomicU64, Ordering}; - static CTR: AtomicU64 = AtomicU64::new(0); - let n = CTR.fetch_add(1, Ordering::Relaxed); - let d = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) - .join("target/test-scratch") - .join(format!("keydb-test-{}-{}-{}", std::process::id(), tag, n)); - let _ = std::fs::remove_dir_all(&d); - std::fs::create_dir_all(&d).unwrap(); - d - } - - // Regression: a missing home directory (HOME/USERPROFILE unset) is an - // *environment* failure, not a corrupt keydb. It must NOT surface as - // E8004 (KeydbParse → "failed to parse the keydb file"), which would - // blame a file that was never consulted. It maps to a NotFound I/O - // error in the 5xxx (I/O) category instead. - #[test] - fn no_home_dir_is_io_not_found_not_keydb_parse() { - let e = no_home_dir(); - match e { - Error::IoError { source } => { - assert_eq!(source.kind(), std::io::ErrorKind::NotFound); - } - other => panic!("expected IoError(NotFound), got {other:?}"), - } - // And explicitly: it is not the keydb-parse code. - assert_ne!(no_home_dir().code(), Error::KeydbParse.code()); - } - - // The write default is local to the executable: `/keydb.cfg`. - // Under `cargo test`, `current_exe()` is the test binary in `target/…`; - // assert against the same computation, not a hardcoded path. - #[test] - fn default_path_is_local_to_executable() { - let expected = std::env::current_exe() - .ok() - .and_then(|exe| exe.parent().map(|dir| dir.join("keydb.cfg"))); - match expected { - Some(p) => assert_eq!(default_path().unwrap(), p), - None => { - // No exe parent available → must surface the env failure. - assert!(matches!(default_path(), Err(Error::IoError { .. }))); - } - } - } - - #[test] - fn write_atomic_replaces_existing_and_leaves_no_temp() { - let dir = scratch("atomic"); - let path = dir.join("freemkv").join("keydb.cfg"); - - // First write creates the parent dir + file. - write_atomic(&path, "0xAAAA = old\n").unwrap(); - assert_eq!(std::fs::read_to_string(&path).unwrap(), "0xAAAA = old\n"); - - // Second write replaces it in place. - write_atomic(&path, "0xBBBB = new\n").unwrap(); - assert_eq!(std::fs::read_to_string(&path).unwrap(), "0xBBBB = new\n"); - - // No leftover *.tmp.* sibling — the temp file was renamed, not orphaned. - let leftovers: Vec<_> = std::fs::read_dir(path.parent().unwrap()) - .unwrap() - .filter_map(|e| e.ok()) - .map(|e| e.file_name().to_string_lossy().into_owned()) - .filter(|n| n.contains(".tmp.")) - .collect(); - assert!(leftovers.is_empty(), "stray temp files: {leftovers:?}"); - } - - #[test] - fn write_atomic_failure_preserves_prior_keydb() { - // Simulate the crash window: a good keydb already on disk, then an - // update whose write target can't be created (parent path is a file, - // so create_dir_all under it fails — i.e. ENOTDIR). The rename never - // happens, so the existing keydb must survive untouched. - let dir = scratch("preserve"); - let good = dir.join("keydb.cfg"); - write_atomic(&good, "0xGOOD = keep\n").unwrap(); - - // `good` is a regular file; treating it as a directory parent fails. - let doomed = good.join("freemkv").join("keydb.cfg"); - let err = write_atomic(&doomed, "0xBAD = partial\n"); - assert!(matches!(err, Err(Error::KeydbWrite { .. }))); - - // Prior good copy is intact. - assert_eq!(std::fs::read_to_string(&good).unwrap(), "0xGOOD = keep\n"); - } - - #[test] - fn parse_url_defaults_and_paths() { - let (h, p, path) = parse_url("http://example.com/keydb.zip").unwrap(); - assert_eq!( - (h.as_str(), p, path.as_str()), - ("example.com", 80, "/keydb.zip") - ); - - let (h, p, path) = parse_url("http://example.com:8080").unwrap(); - assert_eq!((h.as_str(), p, path.as_str()), ("example.com", 8080, "/")); - } - - #[test] - fn parse_url_rejects_https_scheme() { - // TLS is unsupported by this client; surface a scheme diagnostic - // rather than a generic parse error. - assert!(matches!( - parse_url("https://example.com/k.zip"), - Err(Error::KeydbUnsupportedScheme { .. }) - )); - } - - #[test] - fn parse_url_rejects_malformed_port() { - // Non-empty-but-unparseable port must error, not silently fall to 80. - assert!(matches!( - parse_url("http://example.com:abc/path"), - Err(Error::KeydbParse) - )); - // An empty port still defaults to 80. - let (_, p, _) = parse_url("http://example.com:/path").unwrap(); - assert_eq!(p, 80); - } - - #[test] - fn redirect_to_https_is_unsupported_scheme_not_parse_error() { - // The bplaced-style mirror enabling TLS on a redirect must produce a - // diagnosable scheme error, not KeydbParse. - assert!(matches!( - resolve_redirect("https://mirror.example/keydb.zip", "old.host", 80), - Err(Error::KeydbUnsupportedScheme { .. }) - )); - } - - #[test] - fn redirect_scheme_relative_and_absolute_path() { - // Scheme-relative //host/path inherits http. - let (h, p, path) = resolve_redirect("//mirror.example/a.zip", "old.host", 80).unwrap(); - assert_eq!( - (h.as_str(), p, path.as_str()), - ("mirror.example", 80, "/a.zip") - ); - - // Absolute path stays on the current host/port. - let (h, p, path) = resolve_redirect("/new/path.zip", "cur.host", 8080).unwrap(); - assert_eq!( - (h.as_str(), p, path.as_str()), - ("cur.host", 8080, "/new/path.zip") - ); - - // Absolute http URL is followed normally. - let (h, _, path) = resolve_redirect("http://other.host/x.zip", "cur.host", 80).unwrap(); - assert_eq!((h.as_str(), path.as_str()), ("other.host", "/x.zip")); - } - - #[test] - fn parse_status_extracts_code() { - assert_eq!(parse_status("HTTP/1.0 200 OK\r\nFoo: bar"), Some(200)); - assert_eq!(parse_status("HTTP/1.1 301 Moved Permanently"), Some(301)); - assert_eq!(parse_status("garbage"), None); - } - - // ── New comprehensive tests ──────────────────────────────────────────────── - - /// find_header_end detects the \r\n\r\n separator (RFC 7230 §3 — HTTP header - /// terminator is CRLF CRLF). Returns the byte position of the first \r. - /// Mutation: searching for \n\n instead of \r\n\r\n misses the boundary. - #[test] - fn find_header_end_locates_crlfcrlf() { - let data = b"HTTP/1.0 200 OK\r\nContent-Length: 42\r\n\r\nbody starts here"; - // The \r\n\r\n starts at byte 37 (after the Content-Length line). - let pos = find_header_end(data).expect("must find header end"); - // body starts at pos + 4 (past the \r\n\r\n). - assert_eq!( - &data[pos + 4..], - b"body starts here", - "body must begin immediately after the \\r\\n\\r\\n boundary" - ); - } - - /// find_header_end returns None when there is no \r\n\r\n. - /// Mutation: returning Some(0) unconditionally makes this fail. - #[test] - fn find_header_end_returns_none_when_absent() { - let data = b"no separator here at all"; - assert!(find_header_end(data).is_none()); - } - - /// extract_header is case-insensitive per RFC 7230 §3.2. - /// Mutation: using case-sensitive comparison misses "location" vs "Location". - #[test] - fn extract_header_case_insensitive() { - let headers = "HTTP/1.1 301 Moved\r\nlocation: http://new.host/path\r\n"; - let val = extract_header(headers, "Location").expect("must find Location"); - assert_eq!(val, "http://new.host/path"); - } - - /// extract_header with a missing header returns None. - /// Mutation: returning Some("") makes the caller proceed on a missing Location header. - #[test] - fn extract_header_missing_returns_none() { - let headers = "HTTP/1.0 200 OK\r\nContent-Type: text/plain\r\n"; - assert!(extract_header(headers, "Location").is_none()); - } - - /// extract_header trims leading/trailing whitespace from the value. - /// RFC 7230 §3.2.6: optional whitespace around field value. - /// Mutation: not trimming the value keeps leading spaces in the URL. - #[test] - fn extract_header_trims_value_whitespace() { - let headers = "HTTP/1.1 301 Moved\r\nLocation: /new/path \r\n"; - let val = extract_header(headers, "Location").unwrap(); - assert_eq!(val, "/new/path", "value must be trimmed"); - } - - /// save() rejects data that is not a valid keydb (no recognisable entries). - /// Spec: entries are lines starting with "0x", "| DK", "| PK", or "| HC". - /// Mutation: dropping the entries==0 check lets an empty file be saved. - #[test] - fn save_rejects_empty_text() { - // Plain text with no valid keydb entries. - let garbage = b"this is not a keydb\njust random text\n"; - assert!( - matches!(save(garbage), Err(Error::KeydbInvalid)), - "keydb without valid entries must be rejected" - ); - } - - /// save() accepts plain text with at least one "0x"-prefixed entry line. - /// Mutation: counting only "| DK" lines ignores the "0x" entry format. - #[test] - fn save_accepts_plaintext_with_0x_entries() { - // Minimal keydb-style file with a VUK entry (0x-prefixed). - let content = b"0xDEADBEEFCAFEBABE0102030405060708090A0B0C0D0E0F\n"; - // We can't predict the HOME path in test environments without - // potentially writing to a real location. So only check that save() - // accepts this content as valid (may return KeydbWrite if dir exists - // but we lack permission — that still proves it passed the parse check). - let result = save(content); - // Accept either Ok (wrote successfully) or a write error (env issue), - // but NOT KeydbInvalid or KeydbParse. - match &result { - Ok(_) => {} - Err(Error::KeydbWrite { .. }) => {} - Err(e) => panic!("unexpected error for valid keydb content: {:?}", e), - } - } - - /// save() accepts content with "| DK" entries (device-key table format). - /// Mutation: only accepting "0x" lines rejects DK-format keydb files. - #[test] - fn save_accepts_pipe_dk_entry_format() { - let content = b"| DK 0102030405060708 | 0102030405060708090a0b0c0d0e0f10 |\n"; - let result = save(content); - match &result { - Ok(_) => {} - Err(Error::KeydbWrite { .. }) => {} - Err(e) => panic!("unexpected error for DK-format entry: {:?}", e), - } - } - - /// save() accepts content with "| PK" entries (processing-key format). - /// Mutation: not including "| PK" in the filter rejects PK-format keydb files. - #[test] - fn save_accepts_pipe_pk_entry_format() { - let content = b"| PK 0102030405060708090a0b0c0d0e0f10 |\n"; - let result = save(content); - match &result { - Ok(_) => {} - Err(Error::KeydbWrite { .. }) => {} - Err(e) => panic!("unexpected error for PK-format entry: {:?}", e), - } - } - - /// save() accepts content with "| HC" entries (host certificate format). - /// Mutation: not including "| HC" in the filter rejects HC-format keydb files. - #[test] - fn save_accepts_pipe_hc_entry_format() { - let content = b"| HC 0102030405060708090a0b0c0d0e0f10 |\n"; - let result = save(content); - match &result { - Ok(_) => {} - Err(Error::KeydbWrite { .. }) => {} - Err(e) => panic!("unexpected error for HC-format entry: {:?}", e), - } - } - - /// save() recognises gzip-compressed input (magic bytes 0x1f 0x8b). - /// Spec: gzip format magic is 0x1F 0x8B (RFC 1952 §2.3.1). - /// Mutation: treating gzip magic as plain text fails to decompress. - #[test] - fn save_recognises_gzip_magic() { - // Truncated gzip (header only, no valid body) — must not be treated as - // plain text (no KeydbInvalid about entries), but as a parse error. - let bad_gz = [0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03]; - let result = save(&bad_gz); - // A truncated gzip is either KeydbParse (decompression error) or - // KeydbInvalid (decompressed to empty). Must not be Ok. - assert!(result.is_err(), "truncated gzip must not be accepted"); - // Crucially: must NOT be a plain-text UTF-8 error — gzip magic is not UTF-8. - match result.unwrap_err() { - Error::KeydbParse | Error::KeydbInvalid => {} - e => panic!("wrong error kind for truncated gzip: {:?}", e), - } - } - - /// save() recognises ZIP magic bytes PK\x03\x04 and routes to extract_zip. - /// Spec: ZIP local file header signature is 0x50 0x4B 0x03 0x04 (PKZIP APPNOTE §4.3.6). - /// A truncated ZIP must error, but NOT as plain UTF-8 text. - /// Mutation: checking gzip magic before ZIP magic means ZIP files are - /// fed to the gzip decoder and produce the wrong error. - #[test] - fn save_recognises_zip_magic() { - // Valid ZIP magic followed by garbage — must be routed to extract_zip. - let bad_zip = b"PK\x03\x04garbage that is not a real zip"; - let result = save(bad_zip); - assert!(result.is_err(), "invalid zip must be rejected"); - // Must be a parse error, not a UTF-8 error. - match result.unwrap_err() { - Error::KeydbParse | Error::KeydbInvalid => {} - e => panic!("wrong error for bad zip: {:?}", e), - } - } - - /// read_capped_to_string rejects data exceeding MAX_KEYDB_BYTES. - /// Spec: doc says "Returns Error::KeydbInvalid if the input exceeds the cap". - /// Mutation: removing the length check accepts decompression bombs. - #[test] - fn read_capped_to_string_rejects_oversized_input() { - // Build a reader that reports it has more data than the cap. - // We use a Cursor with MAX_KEYDB_BYTES + 1 bytes of content. - let too_big = vec![b'A'; (MAX_KEYDB_BYTES + 1) as usize]; - let cursor = std::io::Cursor::new(too_big); - let result = read_capped_to_string(cursor); - assert!( - matches!(result, Err(Error::KeydbInvalid)), - "oversized input must yield KeydbInvalid, got: {:?}", - result - ); - } - - /// read_capped_to_string returns KeydbParse (not KeydbInvalid) for - /// non-UTF-8 input. Guards the doc/behavior contract: KeydbInvalid is - /// reserved for the size-cap violation, a decode failure is a parse error. - #[test] - fn read_capped_to_string_non_utf8_yields_parse() { - // 0xFF is never a valid UTF-8 byte. - let cursor = std::io::Cursor::new(vec![0xFFu8, 0xFE, 0xFD]); - let result = read_capped_to_string(cursor); - assert!( - matches!(result, Err(Error::KeydbParse)), - "non-UTF-8 input must yield KeydbParse, got: {:?}", - result - ); - } - - /// read_capped_to_string accepts exactly MAX_KEYDB_BYTES (at-cap is allowed). - /// Spec: doc says "Read one byte past the cap so an exactly-at-cap stream is accepted." - /// Mutation: using `>=` instead of `>` in the length check rejects valid at-cap files. - #[test] - fn read_capped_to_string_accepts_at_cap_size() { - let at_cap = vec![b'A'; MAX_KEYDB_BYTES as usize]; - let cursor = std::io::Cursor::new(at_cap); - let result = read_capped_to_string(cursor); - assert!(result.is_ok(), "exactly MAX_KEYDB_BYTES must be accepted"); - } - - /// parse_status returns None for an empty/malformed status line (not a - /// meaningless 0). The call site maps None to Error::KeydbParse. - #[test] - fn parse_status_empty_input_returns_none() { - assert_eq!(parse_status(""), None); - assert_eq!(parse_status("\r\n"), None); - } - - /// Regression: set_read_timeout / set_write_timeout failures must surface as - /// KeydbConnect, not be silently swallowed. - /// - /// We can't easily synthesise a TcpStream whose set_*timeout syscall fails - /// without a platform-specific socket hack, so instead we verify that the - /// error-mapping expression itself is correct: if set_read_timeout were to - /// fail for a given host, the result must be Err(KeydbConnect { host }). - /// - /// The test constructs the exact Err value the code would return and asserts - /// it is KeydbConnect (not, say, silently Ok or a different variant). This - /// pins the variant selection so a future refactor that changes the `.ok()` - /// pattern back would need to update this test as well. - #[test] - fn timeout_set_failure_maps_to_keydb_connect() { - // Simulate what the propagated error looks like. - let host = "hostile.example.com".to_string(); - // The io::Error that set_read_timeout would return on failure. - let io_err = std::io::Error::from(std::io::ErrorKind::InvalidInput); - // Apply the same map_err the production code uses. - let result: Result<()> = - Err(io_err).map_err(|_| Error::KeydbConnect { host: host.clone() }); - assert!( - matches!(result, Err(Error::KeydbConnect { host: ref h }) if h == "hostile.example.com"), - "set_timeout failure must map to KeydbConnect, got: {:?}", - result - ); - } - - /// Regression: http_get to an unreachable host returns KeydbConnect, not a hang. - /// This exercises the connect_timeout path (and thus confirms the overall - /// error-propagation chain is wired); the timeout-set propagation is exercised - /// by the unit test above. - /// - /// Uses port 1 on localhost, which is reserved/unassigned and virtually never - /// listening. connect_timeout with NET_TIMEOUT will refuse or time out quickly. - /// We only assert the error variant, not the host field, since the OS may - /// resolve the address differently. - #[test] - fn http_get_unreachable_host_returns_keydb_connect() { - // Port 1 on loopback — almost always refused immediately. - let result = http_get("http://127.0.0.1:1/keydb.zip"); - // Must be an Err; KeydbConnect is expected for a TCP-level failure. - // KeydbParse or KeydbHttp would indicate the wrong error path. - assert!(result.is_err(), "unreachable host must fail"); - match result.unwrap_err() { - Error::KeydbConnect { .. } => {} - e => panic!("expected KeydbConnect for unreachable host, got: {:?}", e), - } - } - - /// Regression: when the server accepts the connection but closes it before - /// sending complete HTTP headers (the `n == 0` byte-read path), that is a - /// connection/protocol-level fault. It must surface as KeydbConnect (E8000), - /// NOT KeydbParse (E8004) — the keydb content was never received, let alone - /// malformed. - #[test] - fn http_get_server_drops_before_headers_returns_keydb_connect() { - use std::io::Read as _; - use std::net::TcpListener; - - let listener = TcpListener::bind("127.0.0.1:0").unwrap(); - let addr = listener.local_addr().unwrap(); - - let server = std::thread::spawn(move || { - if let Ok((mut sock, _)) = listener.accept() { - // Drain the request so the client's write_all completes, then - // drop the socket without writing any response. The client's - // header read then returns n == 0. - let mut buf = [0u8; 512]; - let _ = sock.read(&mut buf); - drop(sock); - } - }); - - let url = format!("http://127.0.0.1:{}/keydb.zip", addr.port()); - let result = http_get(&url); - server.join().unwrap(); - - assert!(result.is_err(), "dropped connection must fail"); - match result.unwrap_err() { - Error::KeydbConnect { .. } => {} - e => panic!("expected KeydbConnect for dropped connection, got: {:?}", e), - } - } -} diff --git a/src/lib.rs b/src/lib.rs index 038c71b..3b03e1c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -100,7 +100,6 @@ pub mod halt; pub(crate) mod identity; pub(crate) mod ifo; pub mod io; -pub mod keydb; pub mod keysource; pub mod labels; pub(crate) mod mpls;