Skip to content

Commit a6e3877

Browse files
committed
fix: harden shipping-code unwrap() flags at source (Hypatia code_safety)
Resolve the unwrap_without_check findings in shipping crates at source rather than exempting them (#34 burn-down): - stdlib.rs char_at: real bug — `idx < s.len()` is a BYTE guard but indexing used `chars().nth(idx)` (char index), so a multi-byte UTF-8 string could reach None and panic (CWE-754). Now char-indexed with a clean IndexOutOfBounds and a regression test (tests/integration_test.rs). - string.rs int_to_string_radix: `char::from_digit` is provably total here (radix validated 2..=36, digit = num % radix < radix); document with `.expect`. - ai.rs: reword a comment that contained a literal `.unwrap()` token and use `.expect(reason)` in a unit test (both were scanner false positives). https://claude.ai/code/session_014uKLLhZiAGNayhLjdxGXUx
1 parent f304aa3 commit a6e3877

5 files changed

Lines changed: 46 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ this project aims to follow [Semantic Versioning](https://semver.org/spec/v2.0.0
3434

3535
### Fixed
3636

37+
- fix(stdlib): `char_at` indexes by character, not byte — the old `idx < s.len()` byte guard could panic on multi-byte UTF-8 (CWE-754); now returns a clean `IndexOutOfBounds` + regression test
38+
- fix(string): document the `char::from_digit` totality invariant with `.expect` instead of `.unwrap()` in `int_to_string_radix`
3739
- fix(parser): accept comma/trailing-comma separators in `ai_model` attribute blocks, matching struct-field syntax (#84)
3840
- fix(my-fmt): borrow-after-move in `fs::write` output path (crate did not compile) (#84)
3941
- fix(my-lsp): non-exhaustive `CheckError` match — handle `ExpressionTooDeep` (crate did not compile) (#84)

crates/my-lang/lib/common/string.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,10 @@ pub fn int_to_string_radix(n: i64, radix: u32) -> Option<String> {
213213

214214
while num > 0 {
215215
let digit = (num % radix as u64) as u32;
216-
let c = char::from_digit(digit, radix).unwrap();
216+
// Total: `radix` is validated to 2..=36 above and `digit = num % radix`
217+
// is therefore `< radix <= 36`, so `from_digit` always returns `Some`.
218+
let c = char::from_digit(digit, radix)
219+
.expect("digit = num % radix < radix <= 36, so from_digit is total");
217220
result.insert(0, c);
218221
num /= radix as u64;
219222
}

crates/my-lang/lib/mylang/ai.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl AiModelType {
2626
// Deliberately not implementing `std::str::FromStr`: this classifier is
2727
// infallible (every string maps to *some* variant -- unknown names fall
2828
// through to `Local`), so the trait's `Result<Self, Self::Err>` shape
29-
// would just force callers to `.unwrap()` an `Infallible` everywhere.
29+
// would just force callers to discharge an `Infallible` result everywhere.
3030
#[allow(clippy::should_implement_trait)]
3131
pub fn from_str(s: &str) -> Self {
3232
if s.starts_with("gpt-") || s.starts_with("o1") {
@@ -381,7 +381,12 @@ mod tests {
381381
conv.add_assistant("Hi there!");
382382

383383
assert_eq!(conv.message_count(), 3);
384-
assert_eq!(conv.last_message().unwrap().content, "Hi there!");
384+
assert_eq!(
385+
conv.last_message()
386+
.expect("conversation has messages after the adds above")
387+
.content,
388+
"Hi there!"
389+
);
385390
}
386391

387392
#[test]

crates/my-lang/src/stdlib.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -374,14 +374,17 @@ fn register_string_functions(define: &mut impl FnMut(String, Value)) {
374374
arity: 2,
375375
func: |args| match (&args[0], &args[1]) {
376376
(Value::String(s), Value::Int(idx)) => {
377-
let idx = *idx as usize;
378-
if idx < s.len() {
379-
Ok(Value::String(s.chars().nth(idx).unwrap().to_string()))
380-
} else {
381-
Err(RuntimeError::IndexOutOfBounds {
382-
index: idx as i64,
383-
length: s.len(),
384-
})
377+
// Index by *character*, not byte: `s.len()` is the byte
378+
// length, so the old `idx < s.len()` guard let a multi-byte
379+
// UTF-8 string reach `chars().nth(idx) == None` and panic.
380+
// `try_from` also rejects negative indices. Length is
381+
// reported as the char count to match char-indexing.
382+
match usize::try_from(*idx).ok().and_then(|i| s.chars().nth(i)) {
383+
Some(c) => Ok(Value::String(c.to_string())),
384+
None => Err(RuntimeError::IndexOutOfBounds {
385+
index: *idx,
386+
length: s.chars().count(),
387+
}),
385388
}
386389
}
387390
_ => Err(RuntimeError::TypeError {

tests/integration_test.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,3 +351,25 @@ cache = true
351351
assert!(manifest.dependencies.contains_key("std"));
352352
assert_eq!(manifest.ai.default_model, Some("claude-3-opus".to_string()));
353353
}
354+
355+
// Regression: `char_at` must index by character, not byte. "aé" is 2 chars but
356+
// 3 bytes; the old `idx < s.len()` (byte) guard let an in-byte-range / out-of-
357+
// char-range index reach `chars().nth(idx) == None` and panic (CWE-754). Now it
358+
// returns the correct char in range and a clean runtime error out of range.
359+
#[test]
360+
fn test_eval_char_at_is_char_indexed_not_byte() {
361+
// In range: char index 1 of "aé" is the multi-byte 'é', not a byte split.
362+
let in_range = r#"fn main() -> String { return char_at("aé", 1); }"#;
363+
match eval(in_range) {
364+
Ok(Value::String(s)) => assert_eq!(s, "é"),
365+
other => panic!("expected \"é\", got {:?}", other),
366+
}
367+
368+
// Out of char range but within byte length (2 chars, 3 bytes): index 2 must
369+
// be a recoverable error, NOT a panic.
370+
let oob = r#"fn main() -> String { return char_at("aé", 2); }"#;
371+
assert!(
372+
eval(oob).is_err(),
373+
"char_at past the char count must error, not panic"
374+
);
375+
}

0 commit comments

Comments
 (0)