hls: remove legacy /video/stream + /video/{path} routes

The hash-keyed `/video/hls/{hash}/{file}` route fully covers HLS
playback now and both clients (Apollo, FileViewer-React) have
shipped updates that use it directly. Keeping the basename-keyed
fallback only encouraged stale URLs to keep flowing — every legacy
file was deleted by the startup migration, so the routes were
guaranteed 404 machines.

Dropped:
- `stream_video` handler (`GET /video/stream?path=…`) — the original
  basename-keyed playlist serve.
- `get_video_part` handler (`GET /video/{path}`) — bare-filename
  segment serve. The new layout's segments live in
  `<shard>/<hash>/segment_NNN.ts` and reach the client via
  `stream_hls_file`.
- `legacy_path` field on `GenerateVideoResponse` (serialised as
  `playlist`). The field always pointed at a file the migration had
  deleted; current clients ignore it entirely.
- Their service registrations in `main.rs`.
- The body-side `filename` extraction in `generate_video` (existed
  only to construct `legacy_path`) and the now-unused `global`
  opentelemetry import in `handlers/video.rs`.

All 707 tests still pass. Same hand-rolled validators (`is_valid_hash`
/ `is_allowed_hls_filename`) keep the new route's defense-in-depth
intact.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cameron Cordes
2026-05-15 16:00:19 -04:00
parent c30cadde02
commit 0168a4b574
2 changed files with 4 additions and 122 deletions

View File

@@ -11,8 +11,8 @@ use actix_web::{
web::{self, Data},
};
use log::{debug, error, info, warn};
use opentelemetry::KeyValue;
use opentelemetry::trace::{Span, Status, Tracer};
use opentelemetry::{KeyValue, global};
use serde::Serialize;
use crate::content_hash;
@@ -28,12 +28,9 @@ use crate::state::AppState;
use crate::video::actors::{GeneratePreviewClipMessage, QueueVideosMessage, VideoToQueue};
use crate::video::hls_paths;
/// Response body for `POST /video/generate`. New clients should consume
/// `playlist_url` (hash-keyed, stable across libraries and renames) and
/// poll for readiness via the URL itself. Legacy clients reading the
/// raw `playlist` string will be served the legacy basename-keyed path
/// for as long as the field exists — that field will be dropped once
/// every shipped client has migrated.
/// Response body for `POST /video/generate`. Clients consume
/// `playlist_url` (hash-keyed, stable across libraries and renames)
/// and poll for readiness via the URL itself.
#[derive(Serialize, Debug)]
struct GenerateVideoResponse {
/// Hash-keyed URL to the HLS playlist. Resolves to
@@ -49,11 +46,6 @@ struct GenerateVideoResponse {
/// transcode was queued; clients should retry the URL after a short
/// delay (or rely on HLS.js's own retry policy).
ready: bool,
/// Legacy basename-keyed playlist *path string*. Returned for older
/// clients that read the response body as a single string under the
/// pre-2026-05 wire format. New clients should ignore this field.
#[serde(rename = "playlist")]
legacy_path: String,
}
#[post("/video/generate")]
@@ -68,18 +60,6 @@ pub async fn generate_video(
let context = extract_context_from_request(&request);
let mut span = tracer.start_with_context("generate_video", &context);
let filename_pb = PathBuf::from(&body.path);
let Some(filename) = filename_pb
.file_name()
.and_then(|n| n.to_str())
.map(str::to_string)
else {
let message = format!("Unable to get file name: {:?}", &body.path);
error!("{}", message);
span.set_status(Status::error(message));
return HttpResponse::BadRequest().finish();
};
let preferred_library = libraries::resolve_library_param(&app_state, body.library.as_deref())
.ok()
.flatten()
@@ -201,14 +181,12 @@ pub async fn generate_video(
content_hash_str,
hls_paths::PLAYLIST_FILENAME
);
let legacy_path = format!("{}/{}.m3u8", app_state.video_path, filename);
span.set_status(Status::Ok);
HttpResponse::Ok().json(GenerateVideoResponse {
playlist_url,
content_hash: content_hash_str,
ready,
legacy_path,
})
}
@@ -310,100 +288,6 @@ fn is_allowed_hls_filename(name: &str) -> bool {
false
}
#[get("/video/stream")]
pub async fn stream_video(
request: HttpRequest,
_: Claims,
path: web::Query<ThumbnailRequest>,
app_state: Data<AppState>,
) -> impl Responder {
let tracer = global::tracer("image-server");
let context = extract_context_from_request(&request);
let mut span = tracer.start_with_context("stream_video", &context);
let playlist = &path.path;
debug!("Playlist: {}", playlist);
// Only serve files under video_path (HLS playlists) or base_path (source videos)
if playlist.starts_with(&app_state.video_path)
|| is_valid_full_path(&app_state.base_path, playlist, false).is_some()
{
match NamedFile::open(playlist) {
Ok(file) => {
span.set_status(Status::Ok);
file.into_response(&request)
}
_ => {
span.set_status(Status::error(format!("playlist not found {}", playlist)));
HttpResponse::NotFound().finish()
}
}
} else {
span.set_status(Status::error(format!("playlist not valid {}", playlist)));
HttpResponse::BadRequest().finish()
}
}
#[get("/video/{path}")]
pub async fn get_video_part(
request: HttpRequest,
_: Claims,
path: web::Path<ThumbnailRequest>,
app_state: Data<AppState>,
) -> impl Responder {
let tracer = global_tracer();
let context = extract_context_from_request(&request);
let mut span = tracer.start_with_context("get_video_part", &context);
let part = &path.path;
debug!("Video part: {}", part);
let mut file_part = PathBuf::new();
file_part.push(app_state.video_path.clone());
file_part.push(part);
// Guard against directory traversal attacks
let canonical_base = match std::fs::canonicalize(&app_state.video_path) {
Ok(path) => path,
Err(e) => {
error!("Failed to canonicalize video path: {:?}", e);
span.set_status(Status::error("Invalid video path configuration"));
return HttpResponse::InternalServerError().finish();
}
};
let canonical_file = match std::fs::canonicalize(&file_part) {
Ok(path) => path,
Err(_) => {
warn!("Video part not found or invalid: {:?}", file_part);
span.set_status(Status::error(format!("Video part not found '{}'", part)));
return HttpResponse::NotFound().finish();
}
};
// Ensure the resolved path is still within the video directory
if !canonical_file.starts_with(&canonical_base) {
warn!("Directory traversal attempt detected: {:?}", part);
span.set_status(Status::error("Invalid video path"));
return HttpResponse::Forbidden().finish();
}
match NamedFile::open(&canonical_file) {
Ok(file) => {
span.set_status(Status::Ok);
file.into_response(&request)
}
_ => {
error!("Video part not found: {:?}", file_part);
span.set_status(Status::error(format!(
"Video part not found '{}'",
file_part.to_str().unwrap()
)));
HttpResponse::NotFound().finish()
}
}
}
#[get("/video/preview")]
pub async fn get_video_preview(
_claims: Claims,