fix: avoid panic when parsing with a multi-byte delimiter
Ini::parse split the value using delimiter + 1, assuming the delimiter occupies a single byte. With a multi-byte UTF-8 delimiter this offset lands inside the delimiter character, and slicing the string there panics. Advance by the delimiter's actual UTF-8 length instead.
This commit is contained in:
1 parent
eca6be7adc
commit
71e7f66418
3 files changed
+105
-1
No files matched your search
@@ -0,0 +1,81 @@
|
|||||||
|
# Fix panic when parsing with a multi-byte delimiter
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
Parsing input panics when a configured delimiter is a multi-byte UTF-8
|
||||||
|
character and a line uses that delimiter:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
use configparser::ini::{Ini, IniDefault};
|
||||||
|
|
||||||
|
let mut defaults = IniDefault::default();
|
||||||
|
defaults.delimiters = vec!['\u{00a7}']; // section sign, 2 bytes in UTF-8
|
||||||
|
|
||||||
|
let mut config = Ini::new_from_defaults(defaults);
|
||||||
|
config.read(String::from("[s]\nkey\u{00a7}value\n")).unwrap();
|
||||||
|
// thread panicked: start byte index 4 is not a char boundary;
|
||||||
|
// it is inside '§' (bytes 3..5 of string)
|
||||||
|
```
|
||||||
|
|
||||||
|
`delimiters` is a public, user-configurable field, so this is reachable through
|
||||||
|
the public API with safe input.
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
|
||||||
|
In `Ini::parse` (`src/ini.rs`), the key/value split computes the delimiter
|
||||||
|
position with `str::find`, which returns a byte offset, and then slices the
|
||||||
|
value starting one byte later:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
let value = trimmed[delimiter + 1..].trim().to_owned();
|
||||||
|
```
|
||||||
|
|
||||||
|
`delimiter + 1` assumes the delimiter occupies a single byte. When the
|
||||||
|
delimiter is a multi-byte character, `delimiter + 1` lands inside that
|
||||||
|
character, and slicing a `str` on a non-char-boundary index panics. ASCII
|
||||||
|
delimiters such as the default `=` and `:` happen to be one byte, which is why
|
||||||
|
this went unnoticed.
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
|
||||||
|
Advance past the delimiter by its actual UTF-8 length instead of a hard-coded
|
||||||
|
`1`:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
let delimiter_len = trimmed[delimiter..]
|
||||||
|
.chars()
|
||||||
|
.next()
|
||||||
|
.map_or(1, char::len_utf8);
|
||||||
|
let value = trimmed[delimiter + delimiter_len..].trim().to_owned();
|
||||||
|
```
|
||||||
|
|
||||||
|
For single-byte delimiters this is identical to the previous behaviour.
|
||||||
|
|
||||||
|
## Test
|
||||||
|
|
||||||
|
Added `multibyte_delimiter_does_not_panic` to `tests/test.rs`, which configures
|
||||||
|
a two-byte delimiter (`\u{00a7}`), parses `key\u{00a7}value`, and asserts the
|
||||||
|
value is read back correctly instead of panicking.
|
||||||
|
|
||||||
|
## Gate
|
||||||
|
|
||||||
|
- `cargo test --all-features` -> ok (all suites pass, exit 0)
|
||||||
|
- `cargo build --all-features` -> ok (exit 0)
|
||||||
|
- `cargo fmt --check` -> ok (exit 0)
|
||||||
|
|
||||||
|
## Counterfactual
|
||||||
|
|
||||||
|
Reverting to `trimmed[delimiter + 1..]` makes the new test panic:
|
||||||
|
|
||||||
|
```
|
||||||
|
thread 'multibyte_delimiter_does_not_panic' panicked at src/ini.rs:1020:48:
|
||||||
|
start byte index 4 is not a char boundary; it is inside '§' (bytes 3..5 of string)
|
||||||
|
```
|
||||||
|
|
||||||
|
## Flags for the account owner
|
||||||
|
|
||||||
|
`cargo clippy --all-targets --all-features -- -D warnings` currently fails, but
|
||||||
|
this is pre-existing and unrelated to this change: on an unmodified checkout the
|
||||||
|
same lint (`redundant reference in format! argument`, emitted by a newer Clippy
|
||||||
|
than the code was written against) already produces the same errors, none of
|
||||||
|
which are in the code touched here.
|
||||||
+8
-1
@@ -1017,7 +1017,14 @@ impl Ini {
|
|||||||
current_key = Some(key.clone());
|
current_key = Some(key.clone());
|
||||||
}
|
}
|
||||||
|
|
||||||
let value = trimmed[delimiter + 1..].trim().to_owned();
|
// `delimiter` is a byte offset, and the delimiter
|
||||||
|
// itself may be a multi-byte character, so advance
|
||||||
|
// by its UTF-8 length rather than assuming one byte.
|
||||||
|
let delimiter_len = trimmed[delimiter..]
|
||||||
|
.chars()
|
||||||
|
.next()
|
||||||
|
.map_or(1, char::len_utf8);
|
||||||
|
let value = trimmed[delimiter + delimiter_len..].trim().to_owned();
|
||||||
|
|
||||||
valmap.insert(key, Some(value));
|
valmap.insert(key, Some(value));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -973,3 +973,19 @@ fn serde_case_sensitive_roundtrip() -> Result<(), Box<dyn Error>> {
|
|||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn multibyte_delimiter_does_not_panic() -> Result<(), Box<dyn Error>> {
|
||||||
|
use configparser::ini::IniDefault;
|
||||||
|
|
||||||
|
// A multi-byte delimiter must not cause a panic when splitting the
|
||||||
|
// key from the value on a non-char-boundary byte offset.
|
||||||
|
let mut defaults = IniDefault::default();
|
||||||
|
defaults.delimiters = vec!['\u{00a7}']; // section sign, 2 bytes in UTF-8
|
||||||
|
|
||||||
|
let mut config = Ini::new_from_defaults(defaults);
|
||||||
|
config.read(String::from("[s]\nkey\u{00a7}value\n"))?;
|
||||||
|
|
||||||
|
assert_eq!(config.get("s", "key"), Some(String::from("value")));
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
Reference in new issue
Block a user