From 168817ee7421db96d70afa5f55b53fa0efb71c12 Mon Sep 17 00:00:00 2001 From: Braces Date: Wed, 23 Sep 2026 07:51:51 +0700 Subject: [PATCH] Verify and harden Windows CLI and TUI behavior --- README.md | 7 +- docs/TESTING.md | 2 +- src/executor.rs | 158 ++++++++++++++++++++++++++++++++++------- src/output.rs | 87 ++++++++++++++++++++--- src/storage/history.rs | 11 +-- src/storage/saved.rs | 70 +++++++++++++++++- 6 files changed, 289 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index bee9b5d..ba87800 100644 --- a/README.md +++ b/README.md @@ -6,10 +6,11 @@ The current implementation includes the CLI/executor, exact pipeline output, sav ## Build -Use stable Rust with the committed lockfile: +For a Windows x64 release build, use the same static CRT setting as the release workflow: -```sh -cargo build --release --locked +```powershell +$env:RUSTFLAGS = "-C target-feature=+crt-static" +cargo build --release --locked --target x86_64-pc-windows-msvc ``` For development: diff --git a/docs/TESTING.md b/docs/TESTING.md index 020acc7..eb5bbe4 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -18,7 +18,7 @@ The current automated suite includes unit tests for request defaults/validation, ## Current local verification -On Windows x64, the required formatting, strict Clippy, and full test commands pass with 41 library tests and 12 subprocess integration tests. The optimized `x86_64-pc-windows-msvc` executable also builds and reports `curly 0.1.0`. Its import table was inspected with `dumpbin`; only Windows system DLLs are referenced. A ZIP and SHA-256 checksum are generated under `dist/`. +On Windows x64, the required formatting, strict Clippy, and full test commands pass with 46 library tests and 12 subprocess integration tests. A fresh local-server smoke test verified exact binary stdout and stderr separation, a byte-exact 2 MiB file download, HTTP error body and exit code 22, JSON POST with ordered query values, saved-request execution, SQLite history, and noninteractive TUI rejection. PTY checks covered terminal JSON formatting, binary summaries, control-character escaping, composer send, history replay and edit, cancellation of a delayed request, and terminal restoration on quit. A 128 MiB streamed download matched its SHA-256 with 15.2 MiB peak process RSS (sampled roughly every 10 ms); an early-closing downstream pipe exited quietly. The Windows x64 release was built with `RUSTFLAGS=-C target-feature=+crt-static`. Its PE import table contains Windows system DLLs only, with no VCRUNTIME, OpenSSL, or external SQLite DLL. The exact rebuilt release executable was smoke-tested and installed on PATH. The local Windows ZIP and SHA-256 checksum were refreshed; the extracted executable matched the build hash and launched successfully. Linux and macOS release packaging is currently out of the supported release matrix and will be added later after native verification is available. PTY-driven terminal formatting checks and manual TUI resize/restoration checks remain outstanding for Windows. diff --git a/src/executor.rs b/src/executor.rs index 7a6ed6d..7168e72 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -1,8 +1,14 @@ -use std::{pin::Pin, time::Duration}; +use std::{ + io::{self, Read}, + path::{Path, PathBuf}, + pin::Pin, + time::Duration, +}; use bytes::Bytes; use futures_util::{Stream, StreamExt}; use reqwest::{Body, Client, Response, header::HeaderValue, redirect::Policy}; +use serde::Deserialize; use tokio_util::{io::ReaderStream, sync::CancellationToken}; use crate::{ @@ -60,7 +66,10 @@ pub async fn execute( None => {} } - let (body, preview) = prepare_body(request).await?; + let (body, preview) = tokio::select! { + _ = cancel.cancelled() => return Err(CurlyError::Cancelled), + result = prepare_body(request, cancel) => result?, + }; if let Some(body) = body { if matches!( request.body.as_ref().map(BodySource::kind), @@ -127,6 +136,7 @@ fn response_parts(response: Response, elapsed_to_headers: Duration) -> ExecutedR async fn prepare_body( request: &RequestDefinition, + cancel: &CancellationToken, ) -> Result<(Option, RequestPreview), CurlyError> { const PREVIEW_LIMIT: usize = 64 * 1024; let Some(source) = &request.body else { @@ -142,29 +152,21 @@ async fn prepare_body( match source { BodySource::Inline { kind, value } => { - let bytes = value.as_bytes(); - validate_json_if_needed(*kind, bytes)?; + let bytes = validate_json_bytes(*kind, value.as_bytes().to_vec(), cancel).await?; + let total_bytes = bytes.len() as u64; let preview = bytes[..bytes.len().min(PREVIEW_LIMIT)].to_vec(); Ok(( - Some(Body::from(value.clone())), + Some(Body::from(bytes)), RequestPreview { bytes: preview, - truncated: bytes.len() > PREVIEW_LIMIT, - total_bytes: bytes.len() as u64, + truncated: total_bytes > PREVIEW_LIMIT as u64, + total_bytes, }, )) } BodySource::File { kind, path } => { if *kind == BodyKind::Json { - let validation_file = std::fs::File::open(path).map_err(|err| { - CurlyError::Invalid(format!( - "cannot open JSON body file {}: {err}", - path.display() - )) - })?; - serde_json::from_reader::<_, serde_json::Value>(validation_file).map_err( - |err| CurlyError::Invalid(format!("invalid JSON in {}: {err}", path.display())), - )?; + validate_json_file(path, cancel).await?; } let metadata = tokio::fs::metadata(path).await.map_err(|err| { CurlyError::Invalid(format!("cannot read body file {}: {err}", path.display())) @@ -190,27 +192,106 @@ async fn prepare_body( BodySource::Stdin { kind } => { let mut bytes = Vec::new(); use tokio::io::AsyncReadExt; - tokio::io::stdin().read_to_end(&mut bytes).await?; - validate_json_if_needed(*kind, &bytes)?; + let mut stdin = tokio::io::stdin(); + tokio::select! { + _ = cancel.cancelled() => return Err(CurlyError::Cancelled), + result = stdin.read_to_end(&mut bytes) => result?, + }; + let bytes = validate_json_bytes(*kind, bytes, cancel).await?; + let total_bytes = bytes.len() as u64; let preview = bytes[..bytes.len().min(PREVIEW_LIMIT)].to_vec(); Ok(( - Some(Body::from(bytes.clone())), + Some(Body::from(bytes)), RequestPreview { bytes: preview, - truncated: bytes.len() > PREVIEW_LIMIT, - total_bytes: bytes.len() as u64, + truncated: total_bytes > PREVIEW_LIMIT as u64, + total_bytes, }, )) } } } -fn validate_json_if_needed(kind: BodyKind, bytes: &[u8]) -> Result<(), CurlyError> { - if kind == BodyKind::Json { - serde_json::from_slice::(bytes) - .map_err(|err| CurlyError::Invalid(format!("invalid JSON request body: {err}")))?; +async fn validate_json_bytes( + kind: BodyKind, + bytes: Vec, + cancel: &CancellationToken, +) -> Result, CurlyError> { + if kind != BodyKind::Json { + return Ok(bytes); + } + + let worker_cancel = cancel.clone(); + let task = tokio::task::spawn_blocking(move || { + let result = { + let reader = CancellationReader::new( + std::io::Cursor::new(bytes.as_slice()), + worker_cancel.clone(), + ); + let mut deserializer = serde_json::Deserializer::from_reader(reader); + serde::de::IgnoredAny::deserialize(&mut deserializer).and_then(|_| deserializer.end()) + }; + if worker_cancel.is_cancelled() { + return Err(CurlyError::Cancelled); + } + result.map_err(|err| CurlyError::Invalid(format!("invalid JSON request body: {err}")))?; + Ok(bytes) + }); + + tokio::select! { + _ = cancel.cancelled() => Err(CurlyError::Cancelled), + result = task => result + .map_err(|err| CurlyError::Io(io::Error::other(format!("JSON validation worker failed: {err}"))))?, + } +} + +async fn validate_json_file(path: &Path, cancel: &CancellationToken) -> Result<(), CurlyError> { + let path = path.to_path_buf(); + let worker_cancel = cancel.clone(); + let task = + tokio::task::spawn_blocking(move || validate_json_file_blocking(path, worker_cancel)); + tokio::select! { + _ = cancel.cancelled() => Err(CurlyError::Cancelled), + result = task => result + .map_err(|err| CurlyError::Io(io::Error::other(format!("JSON validation worker failed: {err}"))))?, + } +} + +fn validate_json_file_blocking(path: PathBuf, cancel: CancellationToken) -> Result<(), CurlyError> { + let file = std::fs::File::open(&path).map_err(|err| { + CurlyError::Invalid(format!( + "cannot open JSON body file {}: {err}", + path.display() + )) + })?; + let reader = CancellationReader::new(file, cancel.clone()); + let mut deserializer = serde_json::Deserializer::from_reader(reader); + let result = + serde::de::IgnoredAny::deserialize(&mut deserializer).and_then(|_| deserializer.end()); + if cancel.is_cancelled() { + return Err(CurlyError::Cancelled); + } + result.map_err(|err| CurlyError::Invalid(format!("invalid JSON in {}: {err}", path.display()))) +} + +struct CancellationReader { + inner: R, + cancel: CancellationToken, +} + +impl CancellationReader { + fn new(inner: R, cancel: CancellationToken) -> Self { + Self { inner, cancel } + } +} + +impl Read for CancellationReader { + fn read(&mut self, buffer: &mut [u8]) -> io::Result { + if self.cancel.is_cancelled() { + return Err(io::Error::other("operation cancelled")); + } + self.inner.read(buffer) } - Ok(()) } pub async fn collect_stream( @@ -260,4 +341,29 @@ mod tests { assert!(matches!(error, CurlyError::Cancelled)); assert_eq!(error.exit_code(), 130); } + + #[tokio::test] + async fn json_validation_honors_cancellation() { + let cancellation = CancellationToken::new(); + cancellation.cancel(); + let result = validate_json_bytes( + BodyKind::Json, + br#"{"large":[1,2,3]}"#.to_vec(), + &cancellation, + ) + .await; + assert!(matches!(result, Err(CurlyError::Cancelled))); + } + + #[tokio::test] + async fn json_validation_rejects_trailing_data() { + let cancellation = CancellationToken::new(); + let result = validate_json_bytes( + BodyKind::Json, + br#"{"ok":true} trailing"#.to_vec(), + &cancellation, + ) + .await; + assert!(matches!(result, Err(CurlyError::Invalid(_)))); + } } diff --git a/src/output.rs b/src/output.rs index 7477c73..6b92467 100644 --- a/src/output.rs +++ b/src/output.rs @@ -76,16 +76,20 @@ pub async fn write_response( total += chunk.len() as u64; append_preview(&mut preview, &chunk); } - let content_type = content_type.as_deref().unwrap_or("unknown"); - println!("[binary response: {total} bytes, content-type: {content_type}]"); + write_stdout(binary_summary(total, content_type.as_deref()).as_bytes()).await?; } else { let mut candidate = Vec::with_capacity(TERMINAL_JSON_LIMIT + 1); let mut overflow = false; + let mut binary_detected = false; + let mut wrote_terminal_text = false; let mut escaper = TerminalEscaper::default(); while let Some(chunk) = next_chunk(&mut stream, cancel).await? { total += chunk.len() as u64; append_preview(&mut preview, &chunk); + if binary_detected { + continue; + } if !overflow && candidate.len() + chunk.len() <= TERMINAL_JSON_LIMIT { candidate.extend_from_slice(&chunk); continue; @@ -93,19 +97,49 @@ pub async fn write_response( if !overflow { overflow = true; - if !candidate.is_empty() { - let rendered = escaper.push(&candidate, false); + let candidate_contains_nul = candidate.contains(&0); + let prefix = escaper.push(&candidate, false); + candidate.clear(); + let rendered = escaper.push(&chunk, false); + if candidate_contains_nul || chunk.contains(&0) || escaper.saw_invalid_utf8() { + binary_detected = true; + continue; + } + if !prefix.is_empty() { + write_stdout(prefix.as_bytes()).await?; + wrote_terminal_text = true; + } + if !rendered.is_empty() { write_stdout(rendered.as_bytes()).await?; - candidate.clear(); + wrote_terminal_text = true; } + continue; } + let rendered = escaper.push(&chunk, false); + if chunk.contains(&0) || escaper.saw_invalid_utf8() { + binary_detected = true; + continue; + } write_stdout(rendered.as_bytes()).await?; + wrote_terminal_text |= !rendered.is_empty(); } - if overflow { + if binary_detected { + if wrote_terminal_text { + write_stdout(b"\n").await?; + } + write_stdout(binary_summary(total, content_type.as_deref()).as_bytes()).await?; + } else if overflow { let rendered = escaper.push(&[], true); - write_stdout(rendered.as_bytes()).await?; + if escaper.saw_invalid_utf8() { + if wrote_terminal_text { + write_stdout(b"\n").await?; + } + write_stdout(binary_summary(total, content_type.as_deref()).as_bytes()).await?; + } else { + write_stdout(rendered.as_bytes()).await?; + } } else { display_terminal_candidate(&candidate, options.color).await?; } @@ -193,6 +227,15 @@ fn is_textual_content_type(value: &str) -> bool { || value.contains("yaml") } +fn binary_summary(total: u64, content_type: Option<&str>) -> String { + match content_type { + Some(content_type) => { + format!("[binary response: {total} bytes, content-type: {content_type}]\n") + } + None => format!("[binary response: {total} bytes]\n"), + } +} + async fn display_terminal_candidate(bytes: &[u8], color: ColorMode) -> Result<(), CurlyError> { if bytes.is_empty() { return Ok(()); @@ -214,7 +257,7 @@ async fn display_terminal_candidate(bytes: &[u8], color: ColorMode) -> Result<() return Ok(()); } - if std::str::from_utf8(bytes).is_err() { + if bytes.contains(&0) || std::str::from_utf8(bytes).is_err() { println!("[binary response: {} bytes]", bytes.len()); return Ok(()); } @@ -241,6 +284,7 @@ async fn write_stdout(bytes: &[u8]) -> Result<(), CurlyError> { #[derive(Default)] struct TerminalEscaper { pending: Vec, + invalid_utf8: bool, } impl TerminalEscaper { @@ -266,12 +310,14 @@ impl TerminalEscaper { } match err.error_len() { Some(length) => { + self.invalid_utf8 = true; for byte in &data[offset..offset + length] { out.push_str(&format!("\\x{byte:02X}")); } offset += length; } None if final_chunk => { + self.invalid_utf8 = true; for byte in &data[offset..] { out.push_str(&format!("\\x{byte:02X}")); } @@ -287,12 +333,18 @@ impl TerminalEscaper { } out } + + fn saw_invalid_utf8(&self) -> bool { + self.invalid_utf8 + } } fn escape_text(text: &str, out: &mut String) { for ch in text.chars() { match ch { - '\n' | '\r' | '\t' => out.push(ch), + '\n' => out.push(ch), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), ch if ch.is_control() => { if (ch as u32) <= 0xff { out.push_str(&format!("\\x{:02X}", ch as u32)); @@ -321,5 +373,22 @@ mod tests { assert_eq!(escaper.push(&euro[..1], false), ""); assert_eq!(escaper.push(&euro[1..], false), "€"); assert_eq!(escaper.push(b"\x1b[31m", true), "\\x1B[31m"); + assert!(!escaper.saw_invalid_utf8()); + } + + #[test] + fn terminal_escaper_flags_binary_utf8_errors() { + let mut escaper = TerminalEscaper::default(); + assert_eq!(escaper.push(b"text", false), "text"); + let _ = escaper.push(&[0xff, 0xfe], false); + assert!(escaper.saw_invalid_utf8()); + } + + #[test] + fn terminal_escaper_does_not_emit_carriage_return_or_tab() { + assert_eq!( + escape_terminal_bytes(b"hello\rworld\tend\n"), + "hello\\rworld\\tend\n" + ); } } diff --git a/src/storage/history.rs b/src/storage/history.rs index ccf3b41..73cbc36 100644 --- a/src/storage/history.rs +++ b/src/storage/history.rs @@ -145,11 +145,10 @@ impl HistoryStore { fn connection(&self) -> Result { let conn = Connection::open(&self.path) .map_err(|err| CurlyError::History(format!("cannot open history database: {err}")))?; - conn.busy_timeout(Duration::from_millis(750)) - .map_err(|err| { - CurlyError::History(format!("cannot configure history database: {err}")) - })?; - conn.execute_batch("PRAGMA foreign_keys = ON; PRAGMA journal_mode = WAL;") + conn.busy_timeout(Duration::from_secs(5)).map_err(|err| { + CurlyError::History(format!("cannot configure history database: {err}")) + })?; + conn.execute_batch("PRAGMA foreign_keys = ON;") .map_err(|err| { CurlyError::History(format!("cannot configure history database: {err}")) })?; @@ -158,6 +157,8 @@ impl HistoryStore { fn migrate(&self) -> Result<(), CurlyError> { let mut conn = self.connection()?; + conn.execute_batch("PRAGMA journal_mode = WAL;") + .map_err(history_err)?; conn.execute_batch( "CREATE TABLE IF NOT EXISTS schema_migrations ( version INTEGER PRIMARY KEY, diff --git a/src/storage/saved.rs b/src/storage/saved.rs index 618ccf8..3e501db 100644 --- a/src/storage/saved.rs +++ b/src/storage/saved.rs @@ -70,12 +70,47 @@ pub fn save( .join(format!(".{name}.toml.tmp-{}", std::process::id())); std::fs::write(&temporary, text)?; if overwrite && destination.exists() { - std::fs::remove_file(&destination)?; + replace_existing(&temporary, &destination, name)?; + } else { + std::fs::rename(&temporary, &destination)?; } - std::fs::rename(&temporary, &destination)?; Ok(destination) } +fn replace_existing(temporary: &Path, destination: &Path, name: &str) -> Result<(), CurlyError> { + // On platforms where rename replaces an existing file, this is atomic. + if std::fs::rename(temporary, destination).is_ok() { + return Ok(()); + } + + // Windows std::fs::rename does not replace existing files. Preserve the old + // definition as a backup until the new complete file is in place so a failed + // second rename cannot destroy the user's saved request. + let backup = destination.with_file_name(format!(".{name}.toml.backup-{}", std::process::id())); + if backup.exists() { + std::fs::remove_file(&backup)?; + } + std::fs::rename(destination, &backup)?; + match std::fs::rename(temporary, destination) { + Ok(()) => { + let _ = std::fs::remove_file(&backup); + Ok(()) + } + Err(err) => { + let restore = std::fs::rename(&backup, destination); + if let Err(restore_err) = restore { + return Err(CurlyError::Io(std::io::Error::new( + err.kind(), + format!( + "could not replace saved request and could not restore the previous file: {err}; restore failed: {restore_err}" + ), + ))); + } + Err(CurlyError::Io(err)) + } + } +} + pub fn load(paths: &StoragePaths, name: &str) -> Result { validate_name(name)?; let path = paths.requests_dir.join(format!("{name}.toml")); @@ -205,6 +240,37 @@ mod tests { assert!(validate_name("good-name_2").is_ok()); } + #[test] + fn overwrite_replaces_existing_request() { + let temp = TempDir::new().unwrap(); + let paths = paths(&temp); + let first = RequestDefinition::new( + "https://example.test/first".into(), + None, + vec![], + vec![], + None, + None, + TransportOptions::default(), + ) + .unwrap(); + let second = RequestDefinition::new( + "https://example.test/second".into(), + None, + vec![], + vec![], + None, + None, + TransportOptions::default(), + ) + .unwrap(); + + save(&paths, "replace-me", &first, false).unwrap(); + save(&paths, "replace-me", &second, true).unwrap(); + + assert_eq!(load(&paths, "replace-me").unwrap().url, second.url); + } + #[test] fn saved_request_round_trip_preserves_env_reference_and_body_file() { let temp = TempDir::new().unwrap();