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
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -100,8 +100,9 @@ impl ToolSpec for GrepFilesTool {
|
||||
async fn execute(&self, input: Value, context: &ToolContext) -> Result<ToolResult, ToolError> {
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user