Skip to content

Commit adccfa6

Browse files
committed
Fix bugs when parsing format specifiers with key mappings.
* Algorithm used for parsing the key from "%(key)d" format did not handle multiple parenthesis correctly. * Second problem is that they way we calculated number of consumed chars was based on str::find, making the code vulnerable to false-positives.
1 parent 6fe2c43 commit adccfa6

File tree

1 file changed

+56
-19
lines changed

1 file changed

+56
-19
lines changed

vm/src/cformat.rs

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -343,19 +343,41 @@ fn parse_literal(text: &str) -> Result<(CFormatPart, &str, usize), ParsingError>
343343
))
344344
}
345345

346+
fn parse_text_inside_parentheses(text: &str) -> Option<(String, &str)> {
347+
let mut counter = 1;
348+
let mut chars = text.chars();
349+
let mut contained_text = String::new();
350+
while counter > 0 {
351+
let c = chars.next();
352+
353+
match c {
354+
Some('(') => {
355+
counter += 1;
356+
}
357+
Some(')') => {
358+
counter -= 1;
359+
}
360+
None => {
361+
return None;
362+
}
363+
_ => (),
364+
}
365+
366+
if counter > 0 {
367+
contained_text.push(c.unwrap());
368+
}
369+
}
370+
371+
Some((contained_text, chars.as_str()))
372+
}
373+
346374
fn parse_spec_mapping_key(text: &str) -> Result<(Option<String>, &str), CFormatErrorType> {
347375
let mut chars = text.chars();
348376

349377
let next_char = chars.next();
350378
if next_char == Some('(') {
351-
// Get remaining characters after opening parentheses.
352-
let cur_text = chars.as_str();
353-
match cur_text.find(')') {
354-
Some(position) => {
355-
let (left, right) = cur_text.split_at(position);
356-
357-
Ok((Some(left.to_string()), &right[1..]))
358-
}
379+
match parse_text_inside_parentheses(chars.as_str()) {
380+
Some((key, remaining_text)) => Ok((Some(key), remaining_text)),
359381
None => Err(CFormatErrorType::UnmatchedKeyParentheses),
360382
}
361383
} else {
@@ -480,29 +502,30 @@ fn parse_format_type(text: &str) -> Result<(CFormatType, &str, char), CFormatErr
480502
}
481503
}
482504

505+
fn calc_consumed(a: &str, b: &str) -> usize {
506+
a.chars().count() - b.chars().count()
507+
}
508+
483509
impl FromStr for CFormatSpec {
484510
type Err = ParsingError;
485511

486512
fn from_str(text: &str) -> Result<Self, Self::Err> {
487-
let consumed = 0;
488513
let mut chars = text.chars();
489514
if chars.next() != Some('%') {
490-
return Err((CFormatErrorType::MissingModuloSign, consumed));
515+
return Err((CFormatErrorType::MissingModuloSign, 1));
491516
}
492-
let consumed = consumed + 1;
493517

494-
let (mapping_key, after_mapping_key) =
495-
parse_spec_mapping_key(chars.as_str()).map_err(|err| (err, consumed))?;
518+
let after_modulo_sign = chars.as_str();
519+
let (mapping_key, after_mapping_key) = parse_spec_mapping_key(after_modulo_sign)
520+
.map_err(|err| (err, calc_consumed(text, after_modulo_sign)))?;
496521
let (flags, after_flags) = parse_flags(after_mapping_key);
497522
let (width, after_width) = parse_quantity(after_flags);
498523
let (precision, after_precision) = parse_precision(after_width);
499524
// A length modifier (h, l, or L) may be present,
500525
// but is ignored as it is not necessary for Python – so e.g. %ld is identical to %d.
501526
let after_length = consume_length(after_precision);
502-
let consumed = text.find(after_length).unwrap();
503-
let (format_type, _remaining_text, format_char) =
504-
parse_format_type(after_length).map_err(|err| (err, consumed))?;
505-
let consumed = consumed + 1; // not using text.find because if the str is exausted consumed will be 0
527+
let (format_type, remaining_text, format_char) = parse_format_type(after_length)
528+
.map_err(|err| (err, calc_consumed(text, after_length)))?;
506529

507530
// apply default precision for float types
508531
let precision = match precision {
@@ -520,7 +543,7 @@ impl FromStr for CFormatSpec {
520543
precision: precision,
521544
format_type: format_type,
522545
format_char: format_char,
523-
chars_consumed: consumed,
546+
chars_consumed: calc_consumed(text, remaining_text),
524547
})
525548
}
526549
}
@@ -583,6 +606,20 @@ mod tests {
583606
flags: CConversionFlags::empty(),
584607
});
585608
assert_eq!("%(amount)d".parse::<CFormatSpec>(), expected);
609+
610+
let expected = Ok(CFormatSpec {
611+
mapping_key: Some("m((u(((l((((ti))))p)))l))e".to_string()),
612+
format_type: CFormatType::Number(CNumberType::Decimal),
613+
format_char: 'd',
614+
chars_consumed: 30,
615+
min_field_width: None,
616+
precision: None,
617+
flags: CConversionFlags::empty(),
618+
});
619+
assert_eq!(
620+
"%(m((u(((l((((ti))))p)))l))e)d".parse::<CFormatSpec>(),
621+
expected
622+
);
586623
}
587624

588625
#[test]
@@ -613,7 +650,7 @@ mod tests {
613650
"Hello %".parse::<CFormatString>(),
614651
Err(CFormatError {
615652
typ: CFormatErrorType::IncompleteFormat,
616-
index: 6
653+
index: 7
617654
})
618655
);
619656
}

0 commit comments

Comments
 (0)