diff --git a/src-tauri/src/download/worker.rs b/src-tauri/src/download/worker.rs index ead2e04e3..4665605bf 100644 --- a/src-tauri/src/download/worker.rs +++ b/src-tauri/src/download/worker.rs @@ -183,6 +183,16 @@ impl DownloadWorker { .await .map_err(|e| DownloadError::FileSystem(e.to_string()))?; + // A media file is never legitimately empty, and completing one is worse + // than failing: the row goes `completed`, the item shows as available + // offline, and playback then stalls on a file with nothing in it. A + // server that answered 200 with no body — an error page, a transcode + // that produced nothing — used to land here. Treat it as the network + // failure it is so the retry budget applies and the `.part` is kept. + if let Some(reason) = rejects_as_empty(downloaded) { + return Err(DownloadError::Network(reason.to_string())); + } + // Move from .part to final location fs::rename(&temp_path, &task.target_path) .await @@ -229,6 +239,23 @@ pub fn resume_offset(existing_bytes: u64, status: u16) -> u64 { } } +/// Why a finished transfer must not be accepted, if it must not be. +/// +/// A media file is never legitimately empty, and *completing* an empty one is +/// worse than failing: the row goes `completed`, the item shows as available +/// offline, and playback later stalls on a file with nothing in it. A server +/// that answered 200 with no body — an error page, a transcode that produced +/// nothing — used to land exactly there. +/// +/// Reported as a network error so the existing retry budget applies and the +/// `.part` file is kept for a resume. +/// +/// TRACES: UR-019 | DR-168 | UT-168 +pub fn rejects_as_empty(downloaded: u64) -> Option<&'static str> { + (downloaded == 0) + .then_some("server sent an empty body; refusing to complete a zero-byte download") +} + /// The partial-download sidecar for `target`. /// /// **Appends** `.part` rather than replacing the extension. The worker used @@ -305,6 +332,23 @@ impl std::error::Error for DownloadError {} mod tests { use super::*; + /// A transfer that produced no bytes must never be marked complete. + /// + /// Completing it publishes an empty file as playable offline; the media + /// server then answers a request for it with a 416 and the item simply + /// never starts, with nothing in the UI explaining why. + /// + /// TRACES: UR-019 | DR-168 | UT-168 + #[test] + fn test_a_zero_byte_transfer_is_rejected_rather_than_completed() { + assert!( + rejects_as_empty(0).is_some(), + "a zero-byte download must not be completed" + ); + assert!(rejects_as_empty(1).is_none()); + assert!(rejects_as_empty(4 * 1024 * 1024).is_none()); + } + /// The bitrate-download corruption: a transcode ignores `Range` and answers /// `200` with the whole stream. Appending that to the bytes already on disk /// duplicated them, so every retry grew the file past its real size and left diff --git a/src-tauri/src/media_server.rs b/src-tauri/src/media_server.rs index 13427db40..2d92d66a4 100644 --- a/src-tauri/src/media_server.rs +++ b/src-tauri/src/media_server.rs @@ -326,8 +326,12 @@ impl Span { /// /// TRACES: UR-071 | DR-137 | UT-127 pub fn span_for(range: Option<&str>, len: u64) -> Option { + // A zero-length file has no byte to serve. `end` is inclusive, so the + // shortest span this type can express is one byte — returning one for an + // empty file declared `Content-Length: 1` and then streamed nothing, which + // Chromium's media loader waits on forever. 416 says so honestly instead. if len == 0 { - return Some(Span { start: 0, end: 0 }); + return None; } let last = len - 1; let first_chunk = Span { @@ -442,6 +446,42 @@ fn content_type(path: &Path, head: &[u8]) -> &'static str { mod tests { use super::*; + /// An empty file must not be answered with a span that promises a byte. + /// + /// `Span::len()` is `end + 1 - start`, so the `Span { start: 0, end: 0 }` + /// that a zero-length file used to produce reported a length of **one**. + /// The response then declared `Content-Length: 1` and streamed nothing, + /// which Chromium's media loader waits on forever — reaching the user as a + /// downloaded item that never starts. A zero-byte file has no satisfiable + /// range, so 416 is the honest answer. + /// + /// TRACES: UR-071 | DR-137 | UT-127 + #[test] + fn test_span_for_an_empty_file_is_unsatisfiable() { + assert!( + span_for(None, 0).is_none(), + "a zero-length file has no byte to serve" + ); + assert!(span_for(Some("bytes=0-"), 0).is_none()); + assert!(span_for(Some("bytes=0-100"), 0).is_none()); + } + + /// Whatever a span says, its length must match the bytes that follow it. + /// + /// TRACES: UR-071 | DR-137 | UT-127 + #[test] + fn test_span_len_never_exceeds_the_file() { + for len in [0u64, 1, 2, 4095, CHUNK_LEN, CHUNK_LEN + 1] { + if let Some(span) = span_for(None, len) { + assert!( + span.len() <= len, + "span for a {len}-byte file claims {} bytes", + span.len() + ); + } + } + } + /// The whole point: a request with no `Range` must still come back bounded. /// That is the case Tauri's asset protocol answers with the entire file — /// the read Chromium abandoned after 31s.