Skip to content

Commit

Permalink
Merge pull request #806 from schungx/master
Browse files Browse the repository at this point in the history
Fix number parsing and raising decimal to large power.
  • Loading branch information
schungx authored Jan 3, 2024
2 parents 490d796 + c0b6aac commit c5c055f
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Fixes to bugs found via fuzzing
* Indexing into an array with a negative index that is larger than the length of the array now throws an out-of-bounds error (similar to positive indices) instead of defaulting to the first element.
* Fixed edge-case crash in timestamp functions.
* Fixed crash when indenting a block doc-comment with Unicode multi-byte space characters.
* Fixed improper parsing of numbers with too many decimal points.
* Fixed exponential running time when raising a decimal number to a very large power (> 1 million) -- it now returns an overflow error.

Other bug fixes
---------------
Expand Down
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ serde_json = { version = "1.0.45", default-features = false, features = ["alloc"
unicode-xid = { version = "0.2.0", default-features = false, optional = true }
rust_decimal = { version = "1.16.0", default-features = false, features = ["maths"], optional = true }
getrandom = { version = "0.2.0", optional = true }
rustyline = { version = "12.0.0", optional = true }
rustyline = { version = "13.0.0", optional = true }
document-features = { version = "0.2.0", optional = true }
arbitrary = { version = "1.3.2", optional = true, features = ["derive"] }

Expand Down Expand Up @@ -164,4 +164,4 @@ features = ["document-features", "metadata", "serde", "internals", "decimal", "d
[patch.crates-io]
# Notice that a custom modified version of `rustyline` is used which supports bracketed paste on Windows.
# This can be moved to the official version when bracketed paste is added.
rustyline = { git = "https://github.com/schungx/rustyline", branch = "v12" }
rustyline = { git = "https://github.com/schungx/rustyline", branch = "v13" }
4 changes: 4 additions & 0 deletions src/packages/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,10 @@ pub mod decimal_functions {
}
#[rhai_fn(return_raw)]
pub fn power(x: Decimal, y: Decimal) -> RhaiResultOf<Decimal> {
// Raising to a very large power can take exponential time, so limit it to 1 million.
if std::convert::TryInto::<u32>::try_into(y.round()).map_or(true, |v| v > 1000000) {
return Err(make_err(format!("Exponential overflow: {x} ** {y}")));
}
x.checked_powd(y)
.ok_or_else(|| make_err(format!("Exponential overflow: {x} ** {y}")))
}
Expand Down
17 changes: 13 additions & 4 deletions src/tokenizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1591,6 +1591,9 @@ fn get_next_token_inner(
let mut result = SmartString::new_const();
let mut radix_base: Option<u32> = None;
let mut valid: fn(char) -> bool = is_numeric_digit;
let mut has_period = false;

Check warning on line 1594 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_time,no_function,no_float,no_position,no_inde...

unused variable: `has_period`

Check warning on line 1594 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_time,no_function,no_float,no_position,no_inde...

variable does not need to be mutable

Check warning on line 1594 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_float,serde,metadata,internals,debugging, sta...

unused variable: `has_period`

Check warning on line 1594 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_float,serde,metadata,internals,debugging, sta...

variable does not need to be mutable

Check warning on line 1594 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,sync,no_time,no_function,no_float,no_position,no...

unused variable: `has_period`

Check warning on line 1594 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,sync,no_time,no_function,no_float,no_position,no...

variable does not need to be mutable
let mut has_e = false;

Check warning on line 1595 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_time,no_function,no_float,no_position,no_inde...

unused variable: `has_e`

Check warning on line 1595 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_time,no_function,no_float,no_position,no_inde...

variable does not need to be mutable

Check warning on line 1595 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_float,serde,metadata,internals,debugging, sta...

unused variable: `has_e`

Check warning on line 1595 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_float,serde,metadata,internals,debugging, sta...

variable does not need to be mutable

Check warning on line 1595 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,sync,no_time,no_function,no_float,no_position,no...

unused variable: `has_e`

Check warning on line 1595 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,sync,no_time,no_function,no_float,no_position,no...

variable does not need to be mutable

Check warning on line 1595 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_float,decimal, stable, false)

unused variable: `has_e`

Check warning on line 1595 in src/tokenizer.rs

View workflow job for this annotation

GitHub Actions / Build (ubuntu-latest, --features testing-environ,no_float,decimal, stable, false)

variable does not need to be mutable

result.push(c);

while let Some(next_char) = stream.peek_next() {
Expand All @@ -1603,7 +1606,7 @@ fn get_next_token_inner(
stream.eat_next_and_advance(pos);
}
#[cfg(any(not(feature = "no_float"), feature = "decimal"))]
'.' => {
'.' if !has_period && radix_base.is_none() => {
stream.get_next().unwrap();

// Check if followed by digits or something that cannot start a property name
Expand All @@ -1612,6 +1615,7 @@ fn get_next_token_inner(
Some('0'..='9') => {
result.push('.');
pos.advance();
has_period = true;
}
// _ - cannot follow a decimal point
Some(NUMBER_SEPARATOR) => {
Expand All @@ -1628,6 +1632,7 @@ fn get_next_token_inner(
result.push('.');
pos.advance();
result.push('0');
has_period = true;
}
// Not a floating-point number
_ => {
Expand All @@ -1637,22 +1642,26 @@ fn get_next_token_inner(
}
}
#[cfg(not(feature = "no_float"))]
'e' => {
'e' if !has_e && radix_base.is_none() => {
stream.get_next().unwrap();

// Check if followed by digits or +/-
match stream.peek_next() {
// digits after e - accept the e
// digits after e - accept the e (no decimal points allowed)
Some('0'..='9') => {
result.push('e');
pos.advance();
has_e = true;
has_period = true;
}
// +/- after e - accept the e and the sign
// +/- after e - accept the e and the sign (no decimal points allowed)
Some('+' | '-') => {
result.push('e');
pos.advance();
result.push(stream.get_next().unwrap());
pos.advance();
has_e = true;
has_period = true;
}
// Not a floating-point number
_ => {
Expand Down
2 changes: 2 additions & 0 deletions tests/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ fn test_float_scientific() {
assert!(engine.eval::<bool>("123.456 == 1.23456e2").unwrap());
assert!(engine.eval::<bool>("123.456 == 1.23456e+2").unwrap());
assert!(engine.eval::<bool>("123.456 == 123456e-3").unwrap());

assert!(engine.compile("123.456e1.23").is_err());
}

#[test]
Expand Down
9 changes: 9 additions & 0 deletions tests/number_literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ fn test_hex_literal() {
assert_eq!(engine.eval::<INT>("let x = 0xffffffffffffffff; x").unwrap(), -1);
#[cfg(feature = "only_i32")]
assert_eq!(engine.eval::<INT>("let x = 0xffffffff; x").unwrap(), -1);

#[cfg(not(feature = "no_float"))]
assert!(engine.compile("0xabcd.123").is_err());
}

#[test]
Expand All @@ -31,6 +34,9 @@ fn test_octal_literal() {
assert_eq!(engine.eval::<INT>("let x = 0o77; x").unwrap(), 63);
assert_eq!(engine.eval::<INT>("let x = 0O77; x").unwrap(), 63);
assert_eq!(engine.eval::<INT>("let x = 0o1234; x").unwrap(), 668);

#[cfg(not(feature = "no_float"))]
assert!(engine.compile("0o77.123").is_err());
}

#[test]
Expand All @@ -45,4 +51,7 @@ fn test_binary_literal() {
assert_eq!(engine.eval::<INT>("let x = 0b11111111_11111111_11111111_11111111_11111111_11111111_11111111_11111111; x").unwrap(), -1);
#[cfg(feature = "only_i32")]
assert_eq!(engine.eval::<INT>("let x = 0b11111111_11111111_11111111_11111111; x").unwrap(), -1);

#[cfg(not(feature = "no_float"))]
assert!(engine.compile("0b101.101").is_err());
}

0 comments on commit c5c055f

Please sign in to comment.