test(mounts): cover download range logic + add just coverage recipe
- Extract the WebDAV/REST mount-download Range decision (parse + validate + satisfiable) into a pure `plan_mount_range`, unit-tested across no-range, malformed, inclusive, open-ended, suffix, and out-of-bounds cases. This was the one untested branch of `download_mount_file` (its I/O deps are already integration-covered). - Add `just coverage [filter]` and `just coverage-integration [filter]` recipes wrapping cargo-llvm-cov (HTML + summary; integration variant adds the testcontainers tests).
This commit is contained in:
Bradley Nelson
@@ -50,6 +50,18 @@ test:
|
||||
test-mocks:
|
||||
cargo test --features test_utils
|
||||
|
||||
# Source-based line coverage (HTML + summary) via cargo-llvm-cov.
|
||||
# Requires `cargo install cargo-llvm-cov` and llvm-tools (rustup component
|
||||
# llvm-tools-preview, or set LLVM_COV / LLVM_PROFDATA to an llvm-binutils).
|
||||
# Unit tests only (no DB). Pass a filter: `just coverage external_mount`.
|
||||
coverage filter='':
|
||||
cargo llvm-cov --lib --html {{ if filter == '' { '' } else { '--' } }} {{filter}}
|
||||
cargo llvm-cov --lib --summary-only {{ if filter == '' { '' } else { '--' } }} {{filter}}
|
||||
|
||||
# Coverage including the testcontainers integration tests (needs docker).
|
||||
coverage-integration filter='mount':
|
||||
RUSTFLAGS='--cfg integration_tests' cargo llvm-cov --lib --summary-only -- {{filter}}
|
||||
|
||||
# DB-dependent integration tests gated on `--cfg integration_tests`.
|
||||
# Spins up the test postgres on port 5433 first. Requires one row in
|
||||
# auth.users on the test DB (start the server against it once to seed).
|
||||
|
||||
@@ -877,60 +877,48 @@ impl FileHandler {
|
||||
}
|
||||
|
||||
// ── Range Requests ───────────────────────────────────────────
|
||||
if let Some(range_header) = headers.get(header::RANGE)
|
||||
&& let Ok(range_str) = range_header.to_str()
|
||||
&& let Ok(ranges) = parse_range_header(range_str)
|
||||
{
|
||||
match ranges.validate(stat.size) {
|
||||
Ok(valid_ranges) => {
|
||||
if let Some(range) = valid_ranges.first() {
|
||||
let start = *range.start();
|
||||
let end = *range.end();
|
||||
let range_length = end - start + 1;
|
||||
let disposition = Self::content_disposition(name, &stat.mime_type, params);
|
||||
match retrieval
|
||||
.open_mount_file_with_perms(
|
||||
cfg,
|
||||
node_id,
|
||||
caller_id,
|
||||
Some((start, Some(end))),
|
||||
let range_header = headers.get(header::RANGE).and_then(|v| v.to_str().ok());
|
||||
match plan_mount_range(stat.size, range_header) {
|
||||
MountRangePlan::Full => {}
|
||||
MountRangePlan::NotSatisfiable => {
|
||||
return Response::builder()
|
||||
.status(StatusCode::RANGE_NOT_SATISFIABLE)
|
||||
.header(header::CONTENT_RANGE, format!("bytes */{}", stat.size))
|
||||
.body(Body::empty())
|
||||
.unwrap()
|
||||
.into_response();
|
||||
}
|
||||
MountRangePlan::Range { start, end } => {
|
||||
let range_length = end - start + 1;
|
||||
let disposition = Self::content_disposition(name, &stat.mime_type, params);
|
||||
match retrieval
|
||||
.open_mount_file_with_perms(cfg, node_id, caller_id, Some((start, Some(end))))
|
||||
.await
|
||||
{
|
||||
Ok(stream) => {
|
||||
return Response::builder()
|
||||
.status(StatusCode::PARTIAL_CONTENT)
|
||||
.header(header::CONTENT_TYPE, &stat.mime_type)
|
||||
.header(header::CONTENT_DISPOSITION, &disposition)
|
||||
.header(header::CONTENT_LENGTH, range_length)
|
||||
.header(
|
||||
header::CONTENT_RANGE,
|
||||
format!("bytes {}-{}/{}", start, end, stat.size),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(stream) => {
|
||||
return Response::builder()
|
||||
.status(StatusCode::PARTIAL_CONTENT)
|
||||
.header(header::CONTENT_TYPE, &stat.mime_type)
|
||||
.header(header::CONTENT_DISPOSITION, &disposition)
|
||||
.header(header::CONTENT_LENGTH, range_length)
|
||||
.header(
|
||||
header::CONTENT_RANGE,
|
||||
format!("bytes {}-{}/{}", start, end, stat.size),
|
||||
)
|
||||
.header(header::ACCEPT_RANGES, "bytes")
|
||||
.header(header::ETAG, &etag)
|
||||
.header(
|
||||
header::CACHE_CONTROL,
|
||||
"private, max-age=3600, must-revalidate",
|
||||
)
|
||||
.body(Body::from_stream(stream))
|
||||
.unwrap()
|
||||
.into_response();
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::error!("Error creating mount range stream: {}", err);
|
||||
// fall through to full download
|
||||
}
|
||||
}
|
||||
.header(header::ACCEPT_RANGES, "bytes")
|
||||
.header(header::ETAG, &etag)
|
||||
.header(
|
||||
header::CACHE_CONTROL,
|
||||
"private, max-age=3600, must-revalidate",
|
||||
)
|
||||
.body(Body::from_stream(stream))
|
||||
.unwrap()
|
||||
.into_response();
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::error!("Error creating mount range stream: {}", err);
|
||||
// fall through to full download
|
||||
}
|
||||
}
|
||||
Err(_) => {
|
||||
return Response::builder()
|
||||
.status(StatusCode::RANGE_NOT_SATISFIABLE)
|
||||
.header(header::CONTENT_RANGE, format!("bytes */{}", stat.size))
|
||||
.body(Body::empty())
|
||||
.unwrap()
|
||||
.into_response();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1543,3 +1531,93 @@ pub async fn move_file_simple(
|
||||
) -> impl IntoResponse {
|
||||
FileHandler::move_file_simple_impl(state, auth_user, path, json).await
|
||||
}
|
||||
|
||||
/// The download decision for a mount file given a `Range` header — the gnarly
|
||||
/// parse-and-validate logic, extracted so it is unit-testable without I/O.
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub(super) enum MountRangePlan {
|
||||
/// Serve the whole file (no/invalid range header).
|
||||
Full,
|
||||
/// Serve `start..=end` (inclusive) as 206 Partial Content.
|
||||
Range { start: u64, end: u64 },
|
||||
/// The requested range is unsatisfiable for this size → 416.
|
||||
NotSatisfiable,
|
||||
}
|
||||
|
||||
/// Decide how to serve a mount file for a given size + optional `Range` header.
|
||||
/// A missing or unparseable range → `Full`; a valid range → `Range`; an
|
||||
/// out-of-bounds range → `NotSatisfiable`.
|
||||
pub(super) fn plan_mount_range(size: u64, range_header: Option<&str>) -> MountRangePlan {
|
||||
let Some(rh) = range_header else {
|
||||
return MountRangePlan::Full;
|
||||
};
|
||||
let Ok(ranges) = parse_range_header(rh) else {
|
||||
// Malformed range header: ignore it and serve the whole file (RFC 7233).
|
||||
return MountRangePlan::Full;
|
||||
};
|
||||
match ranges.validate(size) {
|
||||
Ok(valid) => match valid.first() {
|
||||
Some(r) => MountRangePlan::Range {
|
||||
start: *r.start(),
|
||||
end: *r.end(),
|
||||
},
|
||||
None => MountRangePlan::Full,
|
||||
},
|
||||
Err(_) => MountRangePlan::NotSatisfiable,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod mount_range_tests {
|
||||
use super::{MountRangePlan, plan_mount_range};
|
||||
|
||||
#[test]
|
||||
fn no_range_header_is_full() {
|
||||
assert_eq!(plan_mount_range(100, None), MountRangePlan::Full);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn malformed_range_falls_back_to_full() {
|
||||
assert_eq!(
|
||||
plan_mount_range(100, Some("not-a-range")),
|
||||
MountRangePlan::Full
|
||||
);
|
||||
assert_eq!(
|
||||
plan_mount_range(100, Some("bytes=abc")),
|
||||
MountRangePlan::Full
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn valid_range_is_parsed_inclusive() {
|
||||
assert_eq!(
|
||||
plan_mount_range(100, Some("bytes=10-19")),
|
||||
MountRangePlan::Range { start: 10, end: 19 }
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn open_ended_range_extends_to_eof() {
|
||||
assert_eq!(
|
||||
plan_mount_range(100, Some("bytes=90-")),
|
||||
MountRangePlan::Range { start: 90, end: 99 }
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn suffix_range_counts_from_end() {
|
||||
// last 10 bytes of a 100-byte file => 90..=99
|
||||
assert_eq!(
|
||||
plan_mount_range(100, Some("bytes=-10")),
|
||||
MountRangePlan::Range { start: 90, end: 99 }
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn out_of_bounds_range_is_not_satisfiable() {
|
||||
assert_eq!(
|
||||
plan_mount_range(100, Some("bytes=200-300")),
|
||||
MountRangePlan::NotSatisfiable
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user