diff --git a/devolutions-gateway/openapi/gateway-api.yaml b/devolutions-gateway/openapi/gateway-api.yaml index 851fdd6af..34ff45970 100644 --- a/devolutions-gateway/openapi/gateway-api.yaml +++ b/devolutions-gateway/openapi/gateway-api.yaml @@ -315,6 +315,22 @@ paths: '200': description: Recording file content: + video/webm: + schema: + type: string + format: binary + application/x-asciicast: + schema: + type: string + format: binary + application/x-ndjson: + schema: + type: string + format: binary + application/json: + schema: + type: string + format: binary application/octet-stream: schema: type: string diff --git a/devolutions-gateway/src/api/jrec.rs b/devolutions-gateway/src/api/jrec.rs index b010f13a6..5212eb095 100644 --- a/devolutions-gateway/src/api/jrec.rs +++ b/devolutions-gateway/src/api/jrec.rs @@ -563,7 +563,18 @@ pub(crate) async fn pull_recording_session( ("filename" = String, Path, description = "Name of recording file to retrieve"), ), responses( - (status = 200, description = "Recording file", body = Vec), + ( + status = 200, + description = "Recording file", + body = Vec, + content_type = [ + "video/webm", + "application/x-asciicast", + "application/x-ndjson", + "application/json", + "application/octet-stream", + ], + ), (status = 400, description = "Bad request"), (status = 401, description = "Invalid or missing authorization token"), (status = 403, description = "Insufficient permissions"), @@ -609,16 +620,10 @@ where .await .map_err(HttpError::internal().err())?; - let content_type = path - .extension() - .and_then(RecordingFileType::from_extension) - .and_then(RecordingFileType::content_type); - - if let Some(content_type) = content_type { - response - .headers_mut() - .insert(CONTENT_TYPE, HeaderValue::from_static(content_type)); - } + let content_type = recording_file_content_type(&path); + response + .headers_mut() + .insert(CONTENT_TYPE, HeaderValue::from_static(content_type)); Ok(response) } @@ -640,6 +645,17 @@ fn is_safe_recording_file_name(file_name: &str) -> bool { !file_name.is_empty() && !file_name.contains("..") && !file_name.contains('/') && !file_name.contains('\\') } +fn recording_file_content_type(path: &Utf8Path) -> &'static str { + if path.file_name() == Some("recording.json") { + return "application/json"; + } + + path.extension() + .and_then(RecordingFileType::from_extension) + .map(RecordingFileType::content_type) + .unwrap_or("application/octet-stream") +} + /// Immutable package membership for one download attempt. /// /// `manifest_bytes` are the exact `recording.json` contents used to derive `clip_names`, @@ -1016,6 +1032,27 @@ mod tests { assert!(!is_safe_recording_file_name("a\\b.webm")); } + #[test] + fn detects_recording_file_content_types() { + let expected_content_types = [ + ("recording-0.webm", "video/webm"), + ("recording-0.trp", "application/octet-stream"), + ("recording-0.cast", "application/x-asciicast"), + ("recording-0.slog", "application/x-ndjson"), + ("recording.json", "application/json"), + ("recording-0.bin", "application/octet-stream"), + ("recording-0", "application/octet-stream"), + ]; + + for (file_name, expected_content_type) in expected_content_types { + assert_eq!( + recording_file_content_type(Utf8Path::new(file_name)), + expected_content_type, + "unexpected content type for {file_name}" + ); + } + } + #[tokio::test] async fn snapshots_manifest_files_for_zip() { let dir = tempfile::tempdir().expect("temp dir"); diff --git a/devolutions-gateway/src/streaming.intent.md b/devolutions-gateway/src/streaming.intent.md new file mode 100644 index 000000000..f3be7928e --- /dev/null +++ b/devolutions-gateway/src/streaming.intent.md @@ -0,0 +1,49 @@ +# Recording streaming intent +## Scope + +These rules apply to the `/shadow` WebSocket streaming path implemented by `streaming.rs`. + +This includes: + +- recording file-type classification +- streamability decisions +- streaming implementation selection +- terminal input-format selection +They do not define the following: + +- JREC push behaviour +- JREC pull behaviour +- artifact storage +- download MIME types +- consumer-side rendering. + +## Streaming contract + +Only WebM, asciicast, and TRP recording artifacts are accepted by the `/shadow` streaming path. + +| Recording file type | Extension | Streaming behaviour | +| --- | --- | --- | +| `WebM` | `.webm` | WebM streaming | +| `Asciicast` | `.cast` | Terminal streaming using asciinema input | +| `TRP` | `.trp` | Terminal streaming using TRP input | +| `SessionRecordingLog` | `.slog` | Explicitly rejected by the `/shadow` streaming path | + +- A recognised`RecordingFileType` is not automatically supported by `/shadow` streaming. Each recognised recording file type must have explicitly defined behaviour for the `/shadow` streaming path. +- Files with missing or unrecognised extensions must be rejected before WebSocket streaming begins. + +## Architectural invariants + +- Recording artifact streaming must use the canonical `RecordingFileType` extension mapping as its source of truth. +- A recording file must be classified once. The resulting `RecordingFileType` must determine: + - if the artifact is supported by the `/shadow` streaming path + - which streaming implementation is used (when applicable) + - which terminal input format is used (when applicable) + +- Streaming validation, streamer selection, and terminal input selection must not maintain separate extension mappings or independently compare known recording extensions as raw strings. +- Adding a new `RecordingFileType` requires an explicit decision about whether it is supported by the `/shadow` streaming path and, if supported, how it is streamed. +- A new or unsupported recording file type must not silently fall back to an existing streaming implementation or terminal input format. +## Component boundaries +JREC artifact handling, storage, download content types, and consumer-side rendering are outside the scope of this document. + + +> **Boundary:** Session Recording Log artifacts are supported elsewhere in Gateway through the JREC recording flow. Their rejection by `/shadow` applies only to the WebSocket streaming path covered by this document. \ No newline at end of file diff --git a/devolutions-gateway/src/streaming.rs b/devolutions-gateway/src/streaming.rs index b62f4994c..61dca69f2 100644 --- a/devolutions-gateway/src/streaming.rs +++ b/devolutions-gateway/src/streaming.rs @@ -32,10 +32,11 @@ pub(crate) async fn stream_file( let path = Arc::new(path.to_owned()); let upgrade_result = match streaming_type { - StreamingType::Terminal => { + StreamingType::Terminal(input_type) => { let shutdown_notify = Arc::clone(&shutdown_notify); ws.on_upgrade(move |socket| async move { - if let Err(e) = setup_terminal_streaming(&path, socket, shutdown_notify, when_new_chunk_appended).await + if let Err(e) = + setup_terminal_streaming(&path, input_type, socket, shutdown_notify, when_new_chunk_appended).await { error!(error = ?e, "Terminal streaming failed"); } @@ -77,34 +78,35 @@ impl terminal_streamer::TerminalStreamSocket for TerminalStreamSocketImpl { } enum StreamingType { - Terminal, + Terminal(terminal_streamer::InputStreamType), WebM, } +/// Determines streamability from recording type, which is stricter than pull MIME handling. +/// A file may be downloadable but still rejected here when there is no streaming backend. async fn validate_streaming_file(path: &camino::Utf8Path) -> anyhow::Result { let path_extension = path .extension() .context("no extension found in the recording file path")?; info!(?path, extension = ?path_extension, "Streaming file"); - if !(path_extension == RecordingFileType::WebM.extension() - || path_extension == RecordingFileType::Asciicast.extension() - || path_extension == RecordingFileType::TRP.extension()) - { - anyhow::bail!("invalid file type"); - } + let file_type = + RecordingFileType::from_extension(path_extension).ok_or_else(|| anyhow::anyhow!("invalid file type"))?; + streaming_type_for_file_type(file_type) +} - if path_extension == RecordingFileType::Asciicast.extension() - || path_extension == RecordingFileType::TRP.extension() - { - Ok(StreamingType::Terminal) - } else { - Ok(StreamingType::WebM) +fn streaming_type_for_file_type(file_type: RecordingFileType) -> anyhow::Result { + match file_type { + RecordingFileType::Asciicast => Ok(StreamingType::Terminal(terminal_streamer::InputStreamType::Asciinema)), + RecordingFileType::TRP => Ok(StreamingType::Terminal(terminal_streamer::InputStreamType::Trp)), + RecordingFileType::WebM => Ok(StreamingType::WebM), + RecordingFileType::SessionRecordingLog => anyhow::bail!("invalid file type"), } } async fn setup_terminal_streaming( path: &camino::Utf8Path, + input_type: terminal_streamer::InputStreamType, socket: WebSocket, shutdown_notify: Arc, when_new_chunk_appended: impl Fn() -> tokio::sync::oneshot::Receiver<()> + Send + 'static, @@ -127,15 +129,6 @@ async fn setup_terminal_streaming( .await .with_context(|| format!("failed to open file: {path:?}"))?; - let path_extension = path - .extension() - .context("no extension found in the recording file path")?; - let input_type = if path_extension == RecordingFileType::Asciicast.extension() { - terminal_streamer::InputStreamType::Asciinema - } else { - terminal_streamer::InputStreamType::Trp - }; - terminal_stream( TerminalStreamSocketImpl(socket), streaming_file, @@ -192,3 +185,71 @@ async fn setup_webm_streaming( } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn validates_streaming_behavior_from_file_extension() { + let webm_type = validate_streaming_file(camino::Utf8Path::new("recording-0.webm")) + .await + .expect("webm should be accepted"); + assert!(matches!(webm_type, StreamingType::WebM)); + + let cast_type = validate_streaming_file(camino::Utf8Path::new("recording-0.cast")) + .await + .expect("cast should be accepted"); + assert!(matches!( + cast_type, + StreamingType::Terminal(terminal_streamer::InputStreamType::Asciinema) + )); + + let trp_type = validate_streaming_file(camino::Utf8Path::new("recording-0.trp")) + .await + .expect("trp should be accepted"); + assert!(matches!( + trp_type, + StreamingType::Terminal(terminal_streamer::InputStreamType::Trp) + )); + + assert!( + validate_streaming_file(camino::Utf8Path::new("recording-0.slog")) + .await + .is_err(), + "slog should be rejected for streaming" + ); + assert!( + validate_streaming_file(camino::Utf8Path::new("recording-0.bin")) + .await + .is_err(), + "unknown extension should be rejected" + ); + assert!( + validate_streaming_file(camino::Utf8Path::new("recording-0")) + .await + .is_err(), + "missing extension should be rejected" + ); + } + + #[test] + fn maps_recording_file_type_to_streaming_type() { + let asciicast_type = + streaming_type_for_file_type(RecordingFileType::Asciicast).expect("asciicast should stream in terminal"); + assert!(matches!( + asciicast_type, + StreamingType::Terminal(terminal_streamer::InputStreamType::Asciinema) + )); + + let trp_type = streaming_type_for_file_type(RecordingFileType::TRP).expect("trp should stream in terminal"); + assert!(matches!( + trp_type, + StreamingType::Terminal(terminal_streamer::InputStreamType::Trp) + )); + + let webm_type = streaming_type_for_file_type(RecordingFileType::WebM).expect("webm should stream as video"); + assert!(matches!(webm_type, StreamingType::WebM)); + assert!(streaming_type_for_file_type(RecordingFileType::SessionRecordingLog).is_err()); + } +} diff --git a/devolutions-gateway/src/token.rs b/devolutions-gateway/src/token.rs index a3f6e0e7f..1b2e194e0 100644 --- a/devolutions-gateway/src/token.rs +++ b/devolutions-gateway/src/token.rs @@ -286,6 +286,9 @@ pub enum RecordingFileType { } impl RecordingFileType { + pub const WEBM_CONTENT_TYPE: &'static str = "video/webm"; + pub const TRP_CONTENT_TYPE: &'static str = "application/octet-stream"; + pub const ASCIICAST_CONTENT_TYPE: &'static str = "application/x-asciicast"; pub const SLOG_CONTENT_TYPE: &'static str = "application/x-ndjson"; pub const fn format_name(self) -> &'static str { @@ -316,10 +319,12 @@ impl RecordingFileType { } } - pub const fn content_type(self) -> Option<&'static str> { + pub const fn content_type(self) -> &'static str { match self { - RecordingFileType::SessionRecordingLog => Some(Self::SLOG_CONTENT_TYPE), - RecordingFileType::WebM | RecordingFileType::TRP | RecordingFileType::Asciicast => None, + RecordingFileType::WebM => Self::WEBM_CONTENT_TYPE, + RecordingFileType::TRP => Self::TRP_CONTENT_TYPE, + RecordingFileType::Asciicast => Self::ASCIICAST_CONTENT_TYPE, + RecordingFileType::SessionRecordingLog => Self::SLOG_CONTENT_TYPE, } } } @@ -1890,4 +1895,18 @@ mod tests { assert_ne!(claims.jti, Uuid::nil()); assert!(matches!(claims.destination, KdcDestination::Inject { .. })); } + + #[test] + fn recording_file_types_have_concrete_content_types() { + let expected = [ + (RecordingFileType::WebM, "video/webm"), + (RecordingFileType::TRP, "application/octet-stream"), + (RecordingFileType::Asciicast, "application/x-asciicast"), + (RecordingFileType::SessionRecordingLog, "application/x-ndjson"), + ]; + + for (recording_file_type, expected_content_type) in expected { + assert_eq!(recording_file_type.content_type(), expected_content_type); + } + } }