From 3468b25cf3c63ad10d3d8fa7cbf262e4c2edf666 Mon Sep 17 00:00:00 2001 From: huqiantao Date: Sun, 7 Jun 2026 18:58:17 +0800 Subject: [PATCH] fix: critical bugs in tools, client, and commands 1. Fix UTF-8 boundary panic in clean_pdf_text (tools/file.rs:295) - rfind returns byte index of char start, i+1 may not be char boundary - Use char-aware byte offset calculation instead 2. Fix integer overflow in context_lines (tools/search.rs:103) - Clamp model-provided context_lines to 1000 to prevent massive allocations - On 32-bit, usize::try_from(u64::MAX) falls back to usize::MAX causing overflow 3. Fix u64->usize truncating cast (tools/file.rs:117,127) - Use usize::try_from() with proper error instead of silent truncation - Prevents reading from wrong line on 32-bit platforms 4. Fix ContentBlockStop wrong index in SSE stream cleanup (client/chat.rs:435) - saturating_sub(1) on 0u32 wraps to u32::MAX when stream breaks during thinking - Merge thinking/text close into single guard to avoid duplicate stops 5. Fix missing providers in provider_accepts_reasoning_content (client/chat.rs:1966) - Add SiliconflowCn and Volcengine which are in apply_reasoning_effort - Without this, non-DeepSeek reasoning models on these providers lose thinking traces 6. Fix TOCTOU double-call in run_skill_by_name (commands/mod.rs:671) - Replace is_some()+unwrap() with if-let-Some pattern - Prevents potential panic if state changes between calls 7. Fix incomplete hex decoding in from_api_tool_name (client.rs:54-76) - Require exactly 6 hex digits to match encoder output - Short sequences from malformed model output now pass through as-is 8. Fix token count u64->u32 truncation (client.rs:1298-1299) - Use .min(u32::MAX) saturating cast consistent with sanitizer at line 1807 - Prevents silent wraparound for extremely large token counts 9. Fix HTTP response body read errors silently swallowed (client/chat.rs:170, client.rs:750) - Replace unwrap_or_default() with .context()? propagation - Connection drops mid-body now surface as clear error instead of JSON parse failure --- crates/tui/src/client.rs | 14 ++++++++++---- crates/tui/src/client/chat.rs | 20 +++++++++++++------- crates/tui/src/commands/mod.rs | 4 ++-- crates/tui/src/tools/file.rs | 17 ++++++++++++++--- crates/tui/src/tools/search.rs | 5 +++-- 5 files changed, 42 insertions(+), 18 deletions(-) diff --git a/crates/tui/src/client.rs b/crates/tui/src/client.rs index 25c92188..9efc2b13 100644 --- a/crates/tui/src/client.rs +++ b/crates/tui/src/client.rs @@ -61,7 +61,10 @@ pub(super) fn from_api_tool_name(name: &str) -> String { break; } } - if let Ok(code) = u32::from_str_radix(&hex, 16) + // Only decode if we got exactly 6 hex digits (matching encoder output). + // Fewer digits means a truncated/malformed sequence — pass through as-is. + if hex.len() == 6 + && let Ok(code) = u32::from_str_radix(&hex, 16) && let Some(decoded) = std::char::from_u32(code) { if let Some('-') = iter.peek().copied() { @@ -747,7 +750,10 @@ impl DeepSeekClient { ); anyhow::bail!("Failed to list models: HTTP {status}: {error_text}"); } - let response_text = response.text().await.unwrap_or_default(); + let response_text = response + .text() + .await + .context("Failed to read models response body")?; parse_models_response(&response_text) } @@ -1295,8 +1301,8 @@ pub(super) fn parse_usage(usage: Option<&Value>) -> Usage { }); Usage { - input_tokens: input_tokens as u32, - output_tokens: output_tokens as u32, + input_tokens: input_tokens.min(u64::from(u32::MAX)) as u32, + output_tokens: output_tokens.min(u64::from(u32::MAX)) as u32, prompt_cache_hit_tokens, prompt_cache_miss_tokens, reasoning_tokens, diff --git a/crates/tui/src/client/chat.rs b/crates/tui/src/client/chat.rs index e8fb42c4..751698a7 100644 --- a/crates/tui/src/client/chat.rs +++ b/crates/tui/src/client/chat.rs @@ -167,7 +167,10 @@ impl DeepSeekClient { anyhow::bail!("Failed to call DeepSeek Chat API: HTTP {status}: {error_text}"); } - let response_text = response.text().await.unwrap_or_default(); + let response_text = response + .text() + .await + .context("Failed to read Chat API response body")?; let value: Value = serde_json::from_str(&response_text).context("Failed to parse Chat API JSON")?; parse_chat_message(&value) @@ -431,12 +434,13 @@ impl DeepSeekClient { } } - // Close any open blocks - if thinking_started { - yield Ok(StreamEvent::ContentBlockStop { index: content_index.saturating_sub(1) }); - } - if text_started { - yield Ok(StreamEvent::ContentBlockStop { index: content_index.saturating_sub(1) }); + // Close any open blocks — use the current content_index + // (which points to the next unused slot, so -1 gives the + // last-opened block) but guard against underflow when no + // content block was ever opened. + if thinking_started || text_started { + let idx = content_index.saturating_sub(1); + yield Ok(StreamEvent::ContentBlockStop { index: idx }); } release_stream_buffer(byte_buf); @@ -1974,6 +1978,8 @@ fn provider_accepts_reasoning_content(provider: ApiProvider) -> bool { | ApiProvider::Novita | ApiProvider::Fireworks | ApiProvider::Siliconflow + | ApiProvider::SiliconflowCn + | ApiProvider::Volcengine | ApiProvider::Arcee | ApiProvider::Sglang ) diff --git a/crates/tui/src/commands/mod.rs b/crates/tui/src/commands/mod.rs index dd10da10..54dc6280 100644 --- a/crates/tui/src/commands/mod.rs +++ b/crates/tui/src/commands/mod.rs @@ -668,8 +668,8 @@ pub fn execute(cmd: &str, app: &mut App) -> CommandResult { _ => { // Third source: skills (lowest precedence after native and user-config). // Try to run a skill whose name matches the command. - if skills::run_skill_by_name(app, command, arg).is_some() { - return skills::run_skill_by_name(app, command, arg).unwrap(); + if let Some(result) = skills::run_skill_by_name(app, command, arg) { + return result; } let suggestions = suggest_command_names(command, 3); if suggestions.is_empty() { diff --git a/crates/tui/src/tools/file.rs b/crates/tui/src/tools/file.rs index 671f1366..3600c933 100644 --- a/crates/tui/src/tools/file.rs +++ b/crates/tui/src/tools/file.rs @@ -114,7 +114,9 @@ impl ToolSpec for ReadFileTool { "start_line must be 1-based and greater than 0".to_string(), )); } - Some(v) => v as usize, + Some(v) => usize::try_from(v).map_err(|_| { + ToolError::invalid_input("start_line exceeds platform addressable range".to_string()) + })?, None => 1, }; @@ -124,7 +126,14 @@ impl ToolSpec for ReadFileTool { "max_lines must be greater than 0".to_string(), )); } - Some(v) => std::cmp::min(v as usize, HARD_MAX_READ_LINES), + Some(v) => { + let converted = usize::try_from(v).map_err(|_| { + ToolError::invalid_input( + "max_lines exceeds platform addressable range".to_string(), + ) + })?; + std::cmp::min(converted, HARD_MAX_READ_LINES) + } None => DEFAULT_READ_LINES, }; @@ -292,7 +301,9 @@ fn clean_pdf_text(raw: &str) -> String { if any_content { let start = out.find(|c: char| c != '\n').unwrap_or(0); // Walk back from end to find the last non-newline character. - let end = out.rfind(|c: char| c != '\n').map_or(out.len(), |i| i + 1); + let end = out + .rfind(|c: char| c != '\n') + .map_or(out.len(), |i| i + out[i..].chars().next().map_or(1, |c| c.len_utf8())); out[start..end].to_string() } else { String::new() diff --git a/crates/tui/src/tools/search.rs b/crates/tui/src/tools/search.rs index 221d760b..0174011a 100644 --- a/crates/tui/src/tools/search.rs +++ b/crates/tui/src/tools/search.rs @@ -100,8 +100,9 @@ impl ToolSpec for GrepFilesTool { async fn execute(&self, input: Value, context: &ToolContext) -> Result { let pattern_str = required_str(&input, "pattern")?; let path_str = optional_str(&input, "path").unwrap_or("."); - let context_lines = - usize::try_from(optional_u64(&input, "context_lines", 2)).unwrap_or(usize::MAX); + let context_lines = usize::try_from(optional_u64(&input, "context_lines", 2)) + .unwrap_or(usize::MAX) + .min(1000); let case_insensitive = optional_bool(&input, "case_insensitive", false); let max_results = usize::try_from(optional_u64(&input, "max_results", MAX_RESULTS as u64)) .unwrap_or(MAX_RESULTS);