Skip to content

Commit 325609f

Browse files
committed
test: handoff-40 Required #3 behavioral tests and Consider fixes
- Fix if-range-strict fixture arg order (was passing MODE as DISPOSITION) - Add 206-validator-mismatch test (non-strict mode, different ETag) - Add probe EngineMsg::Validator persistence test - Add StoredItem validator roundtrip and old-queue compatibility tests - Clean orphaned .part on live retry (read old manifest before overwrite) - Extract digest_for_asset for unit testing; add digest tests
1 parent 669bc18 commit 325609f

3 files changed

Lines changed: 273 additions & 8 deletions

File tree

‎src/download_tests.rs‎

Lines changed: 184 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6384,7 +6384,7 @@ fn piece_rejects_200_after_if_range_mismatch() {
63846384
"v.bin",
63856385
300_000,
63866386
"0",
6387-
&["if-range-strict", "\"server-etag\""],
6387+
&["", "if-range-strict", "\"server-etag\""],
63886388
45,
63896389
);
63906390
let (tx, _rx) = tokio::sync::mpsc::unbounded_channel();
@@ -6412,5 +6412,188 @@ fn piece_rejects_200_after_if_range_mismatch() {
64126412
),
64136413
Err(_) => abort(&server, "wrong error kind"),
64146414
}
6415+
// Prove the 200 path was taken: the server logged our If-Range header,
6416+
// and in strict mode a mismatched If-Range yields 200 (not 206). The
6417+
// 206-validator-mismatch path cannot trigger here.
6418+
let log = std::fs::read_to_string(dir.join("ranges.log")).unwrap_or_default();
6419+
assert!(
6420+
log.contains("if-range=\"client-etag\""),
6421+
"server never saw our If-Range; strict mode not engaged. Log:\n{log}"
6422+
);
64156423
cleanup(&server, &dir);
64166424
}
6425+
6426+
/// 206 with mismatched validator yields Changed: when the server ignores
6427+
/// If-Range (returns 206 anyway) but the response ETag differs from our
6428+
/// session validator, fetch_piece must fail with Changed, not splice versions.
6429+
#[test]
6430+
fn piece_rejects_206_with_mismatched_validator() {
6431+
// Normal mode (not strict): server returns 206 with its ETag even when
6432+
// If-Range doesn't match. Our validator differs from the server's ETag.
6433+
let Fixture {
6434+
dir: _dir,
6435+
dl,
6436+
payload,
6437+
port,
6438+
server,
6439+
} = spawn_fixture(
6440+
"ifrange206",
6441+
"v.bin",
6442+
300_000,
6443+
"0",
6444+
&["", "", "\"server-etag-v2\""],
6445+
46,
6446+
);
6447+
let (tx, _rx) = tokio::sync::mpsc::unbounded_channel();
6448+
let if_range = std::sync::Arc::new(std::sync::OnceLock::new());
6449+
// Our validator differs from the server's ETag ("server-etag-v2").
6450+
let _ = if_range.set("\"client-etag-v1\"".to_string());
6451+
let ctx = FetchCtx {
6452+
client: http_client().clone(),
6453+
url: format!("http://127.0.0.1:{port}/v.bin"),
6454+
dest: dl.join("v.bin"),
6455+
opts: DownloadOptions {
6456+
..Default::default()
6457+
},
6458+
cookies: None,
6459+
timeout: Duration::from_secs(30),
6460+
tx,
6461+
if_range,
6462+
};
6463+
let total = payload.len() as u64;
6464+
match tokio_rt().block_on(fetch_piece(&ctx, 0, 1023, total)) {
6465+
Err(AttemptFail::Changed(_)) => {}
6466+
Ok(_) => abort(
6467+
&server,
6468+
"206 with mismatched validator must fail Changed, not succeed",
6469+
),
6470+
Err(e) => abort(&server, &format!("wrong error kind: {e:?}")),
6471+
}
6472+
cleanup(&server, &_dir);
6473+
}
6474+
6475+
/// Probe validator is persisted via EngineMsg::Validator: when a fresh
6476+
/// segmented download starts, the probe's ETag must be sent through the
6477+
/// channel before workers proceed, so pause/restart can resume with If-Range.
6478+
#[test]
6479+
fn probe_validator_sent_before_workers() {
6480+
let Fixture {
6481+
dir,
6482+
dl,
6483+
payload: _payload,
6484+
port,
6485+
server,
6486+
} = spawn_fixture(
6487+
"probeval",
6488+
"v.bin",
6489+
5_000_000,
6490+
"0",
6491+
&["", "", "\"probe-etag-456\""],
6492+
47,
6493+
);
6494+
let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel();
6495+
let ctx = FetchCtx {
6496+
client: http_client().clone(),
6497+
url: format!("http://127.0.0.1:{port}/v.bin"),
6498+
dest: dl.join("v.bin"),
6499+
opts: DownloadOptions {
6500+
..Default::default()
6501+
},
6502+
cookies: None,
6503+
timeout: Duration::from_secs(30),
6504+
tx,
6505+
if_range: std::sync::Arc::new(std::sync::OnceLock::new()),
6506+
};
6507+
// Run the download in the background; we only care about the Validator message.
6508+
let dl_clone = dl.clone();
6509+
let handle = std::thread::spawn(move || {
6510+
tokio_rt().block_on(run_download(ctx, 4, StartMode::Fresh));
6511+
let _ = std::fs::remove_dir_all(dl_clone);
6512+
});
6513+
// Wait for the Validator message (probe must send it before workers start).
6514+
let validator = tokio_rt().block_on(async {
6515+
let timeout = tokio::time::timeout(Duration::from_secs(30), async {
6516+
while let Some(msg) = rx.recv().await {
6517+
if let EngineMsg::Validator(info) = msg {
6518+
return Some(info);
6519+
}
6520+
// Stop waiting once the download finishes or fails.
6521+
if matches!(msg, EngineMsg::Finished { .. } | EngineMsg::Failed(_)) {
6522+
return None;
6523+
}
6524+
}
6525+
None
6526+
});
6527+
timeout.await.unwrap_or(None)
6528+
});
6529+
let _ = handle.join();
6530+
let info = validator.unwrap_or_else(|| abort(&server, "probe never sent EngineMsg::Validator"));
6531+
assert_eq!(
6532+
info.etag.as_deref(),
6533+
Some("\"probe-etag-456\""),
6534+
"validator ETag must match the probe response"
6535+
);
6536+
cleanup(&server, &dir);
6537+
}
6538+
6539+
/// StoredItem validator persistence: etag/last_modified round-trip through
6540+
/// JSON serialization, so cross-session resume has the validator.
6541+
#[test]
6542+
fn stored_item_validator_roundtrip() {
6543+
let mut item = stored_row(
6544+
"http://example.com/f.bin",
6545+
"/tmp",
6546+
"f.bin",
6547+
DownloadStatus::Paused,
6548+
);
6549+
item.etag = Some("\"test-etag-789\"".to_string());
6550+
item.last_modified = Some("Wed, 21 Oct 2015 07:28:00 GMT".to_string());
6551+
6552+
let json = serde_json::to_string(&item).expect("serialize");
6553+
assert!(json.contains("test-etag-789"), "etag not persisted: {json}");
6554+
assert!(
6555+
json.contains("Wed, 21 Oct 2015"),
6556+
"last_modified not persisted: {json}"
6557+
);
6558+
6559+
let restored: StoredItem = serde_json::from_str(&json).expect("deserialize");
6560+
assert_eq!(
6561+
restored.etag.as_deref(),
6562+
Some("\"test-etag-789\""),
6563+
"etag not restored"
6564+
);
6565+
assert_eq!(
6566+
restored.last_modified.as_deref(),
6567+
Some("Wed, 21 Oct 2015 07:28:00 GMT"),
6568+
"last_modified not restored"
6569+
);
6570+
}
6571+
6572+
/// Old queue compatibility: a queue file predating the validator fields
6573+
/// (no etag/last_modified keys) must deserialize with None, not fail.
6574+
#[test]
6575+
fn old_queue_without_validators_deserializes() {
6576+
// Simulate an old queue file: StoredItem JSON without etag/last_modified.
6577+
let old_json = r#"{
6578+
"id": null,
6579+
"url": "http://example.com/old.bin",
6580+
"dest_dir": "/tmp",
6581+
"filename": "old.bin",
6582+
"status": "Paused",
6583+
"progress": 0.5,
6584+
"segments": null,
6585+
"selected_files": null,
6586+
"output_dir": null,
6587+
"video_source": null,
6588+
"started": true,
6589+
"scheduled_at": null
6590+
}"#;
6591+
let item: StoredItem =
6592+
serde_json::from_str(old_json).expect("old queue format must deserialize");
6593+
assert_eq!(item.etag, None, "missing etag must default to None");
6594+
assert_eq!(
6595+
item.last_modified, None,
6596+
"missing last_modified must default to None"
6597+
);
6598+
assert_eq!(item.url, "http://example.com/old.bin");
6599+
}

‎src/video_runner.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,6 +1119,17 @@ pub(crate) async fn run_live_ytdlp(
11191119
// (wget-style dedup). The chosen name is stored in the manifest for cleanup.
11201120
// Never equal to job.dest (the final file); staging is always distinct.
11211121
let (out, staging_name) = claim_staging_name(job, ext)?;
1122+
// Clean up the previous attempt's .part if the manifest points to a
1123+
// different staging name (crash/retry orphan). Never delete the base
1124+
// recording — only the .part shell.
1125+
if let Some(old) = read_manifest(&staging, job.item_id) {
1126+
if let Some(old_name) = old.staging_name.as_deref() {
1127+
if old_name != staging_name {
1128+
let old_part = staging.join(format!("{old_name}.part"));
1129+
let _ = std::fs::remove_file(&old_part);
1130+
}
1131+
}
1132+
}
11221133
// Overwrite pre-flight: refuse if dest exists. The manifest proves ownership for future retries,
11231134
// but never delete here — a weak match could delete a completed output from a quality/format-changed retry.
11241135
if job.dest.exists() {

‎src/video_tools.rs‎

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1301,6 +1301,18 @@ pub async fn latest_quickjs_tag() -> Option<String> {
13011301
latest_quickjs_release().await.map(|(tag, _)| tag)
13021302
}
13031303

1304+
/// SHA-256 digest for a release asset, stripped of the "sha256:" prefix.
1305+
/// Returns None if the asset is absent or has no digest: callers fail closed.
1306+
fn digest_for_asset(release: &yt_dlp::client::deps::github::Release, name: &str) -> Option<String> {
1307+
release
1308+
.assets
1309+
.iter()
1310+
.find(|a| a.name == name)
1311+
.and_then(|a| a.digest.as_deref())
1312+
.and_then(|d| d.strip_prefix("sha256:"))
1313+
.map(str::to_owned)
1314+
}
1315+
13041316
/// Latest quickjs-ng release tag and its SHA-256 digest for this architecture.
13051317
/// The digest comes from GitHub's release API (`digest: "sha256:…"` per asset).
13061318
/// Returns None if the digest is absent: we fail closed rather than install
@@ -1310,13 +1322,7 @@ pub async fn latest_quickjs_release() -> Option<(String, Option<String>)> {
13101322
let fetcher = yt_dlp::client::deps::github::GitHubFetcher::new("quickjs-ng", "quickjs");
13111323
let release = fetcher.fetch_latest_release(None).await.ok()?;
13121324
let want = format!("qjs-linux-{}", std::env::consts::ARCH);
1313-
let digest = release
1314-
.assets
1315-
.iter()
1316-
.find(|a| a.name == want)
1317-
.and_then(|a| a.digest.as_deref())
1318-
.and_then(|d| d.strip_prefix("sha256:"))
1319-
.map(str::to_owned);
1325+
let digest = digest_for_asset(&release, &want);
13201326
Some((release.tag_name, digest))
13211327
});
13221328
handle.await.ok().flatten()
@@ -1662,3 +1668,68 @@ mod tests {
16621668
std::fs::remove_dir_all(&dir).ok();
16631669
}
16641670
}
1671+
1672+
#[cfg(test)]
1673+
mod digest_tests {
1674+
use super::*;
1675+
1676+
/// Build a minimal Release from JSON for testing digest extraction.
1677+
fn release_from_json(json: &str) -> yt_dlp::client::deps::github::Release {
1678+
serde_json::from_str(json).expect("test Release JSON must parse")
1679+
}
1680+
1681+
#[test]
1682+
fn digest_for_asset_extracts_sha256() {
1683+
let release = release_from_json(
1684+
r#"{
1685+
"tag_name": "v1.0.0",
1686+
"assets": [
1687+
{"name": "qjs-linux-x86_64", "digest": "sha256:abc123"},
1688+
{"name": "qjs-linux-aarch64", "digest": "sha256:def456"}
1689+
]
1690+
}"#,
1691+
);
1692+
assert_eq!(
1693+
digest_for_asset(&release, "qjs-linux-x86_64").as_deref(),
1694+
Some("abc123")
1695+
);
1696+
assert_eq!(
1697+
digest_for_asset(&release, "qjs-linux-aarch64").as_deref(),
1698+
Some("def456")
1699+
);
1700+
}
1701+
1702+
#[test]
1703+
fn digest_for_asset_returns_none_when_missing() {
1704+
// No digest field: fail-closed path.
1705+
let release = release_from_json(
1706+
r#"{
1707+
"tag_name": "v1.0.0",
1708+
"assets": [
1709+
{"name": "qjs-linux-x86_64"}
1710+
]
1711+
}"#,
1712+
);
1713+
assert_eq!(digest_for_asset(&release, "qjs-linux-x86_64"), None);
1714+
1715+
// Asset not found.
1716+
let release = release_from_json(
1717+
r#"{
1718+
"tag_name": "v1.0.0",
1719+
"assets": []
1720+
}"#,
1721+
);
1722+
assert_eq!(digest_for_asset(&release, "qjs-linux-x86_64"), None);
1723+
1724+
// Digest without sha256: prefix.
1725+
let release = release_from_json(
1726+
r#"{
1727+
"tag_name": "v1.0.0",
1728+
"assets": [
1729+
{"name": "qjs-linux-x86_64", "digest": "md5:abc123"}
1730+
]
1731+
}"#,
1732+
);
1733+
assert_eq!(digest_for_asset(&release, "qjs-linux-x86_64"), None);
1734+
}
1735+
}

0 commit comments

Comments
 (0)