Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions lib/tests/tests/issues/obe_10727_float_nan_arithmetic.vrl
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# issue: OBE-10727
# Overflowing multiplication produces a legitimate `inf` float, and multiplying that by zero
# yields NaN. `NotNan`'s `Mul` impl panicked on that, taking the process down.
# result: can't multiply type float by float

x = 1000000.0
x = x * x
x = x * x
x = x * x
x = x * x
x = x * x
x = x * x
x * 0.0
6 changes: 6 additions & 0 deletions lib/tests/tests/issues/obe_10731_xml_comment_only_child.vrl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# issue: OBE-10731
# An element whose only child is a comment took the single-child "flatten" path and handed the
# comment node to `process_node`, hitting `unreachable!("shouldn't be other XML nodes")`.
# result: { "a": {} }

parse_xml!("<a><!-- comment --></a>")
7 changes: 7 additions & 0 deletions lib/tests/tests/issues/obe_10734_escaped_closing_brace.vrl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# issue: OBE-10734
# The lexer accepted `\}` as an escape code, but `unescape_string_literal` had no matching arm
# and fell through to `unimplemented!("invalid escape")` — a one-character program crashed the
# compiler.
# result: "}"

"\}"
12 changes: 12 additions & 0 deletions lib/tests/tests/issues/obe_10735_array_index_cap.vrl
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# issue: OBE-10735

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should leave this change out - there could be a valid use case for indexing beyond 32769.

# Assigning to a large array index padded the array with `Value::Null` up to that index with no
# cap, so an event-controlled index could exhaust memory. Indices beyond ±32768 are now ignored.
# result: [0, 32769]

capped = []
capped[40000] = 1

allowed = []
allowed[32768] = 1

[length(capped), length(allowed)]
82 changes: 77 additions & 5 deletions src/compiler/value/arithmetic.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#![deny(clippy::arithmetic_side_effects)]

use std::ops::{Add, Mul, Rem};

use crate::compiler::{
value::{Kind, VrlValueConvert},
ExpressionError,
Expand Down Expand Up @@ -68,6 +66,33 @@ fn safe_sub(lhv: f64, rhv: f64) -> Option<Value> {
}
}

fn safe_add(lhv: f64, rhv: f64) -> Option<Value> {
let result = lhv + rhv;
if result.is_nan() {
None
} else {
Some(Value::from_f64_or_zero(result))
}
}

fn safe_mul(lhv: f64, rhv: f64) -> Option<Value> {
let result = lhv * rhv;
if result.is_nan() {
None
} else {
Some(Value::from_f64_or_zero(result))
}
}

fn safe_rem(lhv: f64, rhv: f64) -> Option<Value> {
let result = lhv % rhv;
if result.is_nan() {
None
} else {
Some(Value::from_f64_or_zero(result))
}
}

impl VrlValueArithmetic for Value {
/// Similar to [`std::ops::Mul`], but fallible (e.g. `TryMul`).
fn try_mul(self, rhs: Self) -> Result<Self, ValueError> {
Expand All @@ -90,7 +115,7 @@ impl VrlValueArithmetic for Value {
}
Value::Float(lhv) => {
let rhs = rhs.try_into_f64().map_err(|_| err())?;
lhv.mul(rhs).into()
safe_mul(*lhv, rhs).ok_or_else(err)?
}
Value::Bytes(lhv) if rhs.is_integer() => {
Bytes::from(lhv.repeat(as_usize(rhs.try_integer()?))).into()
Expand Down Expand Up @@ -134,7 +159,7 @@ impl VrlValueArithmetic for Value {
let rhs = rhs
.try_into_f64()
.map_err(|_| ValueError::Add(Kind::float(), rhs.kind()))?;
lhs.add(rhs).into()
safe_add(*lhs, rhs).ok_or(ValueError::Add(Kind::float(), Kind::float()))?
}
(lhs @ Value::Bytes(_), Value::Null) => lhs,
(Value::Bytes(lhs), Value::Bytes(rhs)) => {
Expand Down Expand Up @@ -230,7 +255,7 @@ impl VrlValueArithmetic for Value {
}
Value::Float(lhv) => {
let rhv = rhs.try_into_f64().map_err(|_| err())?;
lhv.rem(rhv).into()
safe_rem(*lhv, rhv).ok_or_else(err)?
}
_ => return Err(err()),
};
Expand Down Expand Up @@ -342,3 +367,50 @@ impl VrlValueArithmetic for Value {
}
}
}

#[cfg(test)]
mod tests {
use ordered_float::NotNan;

use super::*;

// `NotNan` permits infinities, so a float operand can legitimately be ±∞ (e.g. produced by
// an overflowing multiplication, or by `parse_json` on an out-of-range literal). Operations
// whose result is NaN used to reach `NotNan`'s `Add`/`Mul`/`Rem` impls, which panic.
// See OBE-10727.
fn float(v: f64) -> Value {
Value::Float(NotNan::new(v).expect("test operand is not NaN"))
}

#[test]
fn multiplying_infinity_by_zero_returns_an_error_instead_of_panicking() {
assert!(float(f64::INFINITY).try_mul(float(0.0)).is_err());
assert!(float(0.0).try_mul(float(f64::NEG_INFINITY)).is_err());
}

#[test]
fn adding_opposite_infinities_returns_an_error_instead_of_panicking() {
assert!(float(f64::INFINITY)
.try_add(float(f64::NEG_INFINITY))
.is_err());
}

#[test]
fn taking_the_remainder_of_infinity_returns_an_error_instead_of_panicking() {
assert!(float(f64::INFINITY).try_rem(float(f64::INFINITY)).is_err());
assert!(float(f64::INFINITY).try_rem(float(2.0)).is_err());
}

#[test]
fn an_infinite_result_is_still_a_valid_value() {
// Only NaN results are rejected — overflow to ±∞ must keep working.
assert_eq!(
float(f64::MAX).try_mul(float(10.0)),
Ok(float(f64::INFINITY))
);
assert_eq!(
float(f64::INFINITY).try_add(float(1.0)),
Ok(float(f64::INFINITY))
);
}
}
9 changes: 9 additions & 0 deletions src/parser/lex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,7 @@ fn unescape_string_literal(mut s: &str) -> String {
b't' => '\t',
b'0' => '\0',
b'{' => '{',
b'}' => '}',
_ => unimplemented!("invalid escape"),
};

Expand Down Expand Up @@ -2146,6 +2147,14 @@ mod test {
);
}

// OBE-10734: the lexer's `escape_code` accepted `\}`, but `unescape_string_literal` had no
// matching arm and fell through to `unimplemented!("invalid escape")`.
#[test]
fn unescape_escaped_closing_brace() {
assert_eq!("}", StringLiteralToken(r"\}").unescape());
assert_eq!("{}", StringLiteralToken(r"\{\}").unescape());
}

#[test]
fn function_closure_no_arg() {
test(
Expand Down
33 changes: 17 additions & 16 deletions src/parsing/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,22 +162,23 @@ fn process_node(node: Node, config: &ParseXmlConfig) -> Value {
_ => match node.children().count() {
// For a single node, 'flatten' the object if necessary.
1 => {
// Expect a single element.
let node = node.children().next().expect("expected 1 XML node");

// If the node is an element, treat it as an object.
if node.is_element() {
let mut map = BTreeMap::new();

map.insert(
node.tag_name().name().to_string().into(),
process_node(node, config),
);

Value::Object(map)
} else {
// Otherwise, 'flatten' the object by continuing processing.
process_node(node, config)
// Skip non-element/non-text nodes (e.g. comments, PIs) to prevent
// passing them to process_node which cannot handle them.
let child = node.children().find(|n| n.is_element() || n.is_text());
match child {
Some(node) if node.is_element() => {
let mut map = BTreeMap::new();

map.insert(
node.tag_name().name().to_string().into(),
process_node(node, config),
);

Value::Object(map)
}
Some(node) => process_node(node, config),
// Only child is a comment or PI — treat as empty element.
None => Value::Object(recurse(node)),
}
}
// For 2+ nodes, expand.
Expand Down
24 changes: 23 additions & 1 deletion src/stdlib/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::compiler::prelude::*;
#[allow(clippy::cast_possible_wrap)]
fn find(value: Value, pattern: Value, from: Option<Value>) -> Resolved {
let from = match from {
Some(value) => value.try_integer()?,
Some(value) => value.try_integer()?.max(0),
None => 0,
} as usize;

Expand Down Expand Up @@ -75,6 +75,9 @@ struct FindFn {

impl FindFn {
fn find_regex_in_str(value: &str, regex: ValueRegex, offset: usize) -> Option<usize> {
if offset > value.len() {
return None;
}
regex.find_at(value, offset).map(|found| found.start())
}

Expand Down Expand Up @@ -178,5 +181,24 @@ mod tests {
want: Err("expected string or regex, got integer"),
tdef: TypeDef::integer().infallible(),
}

// OBE-10722: negative from wraps to huge usize, panics in regex::find_at
negative_from_string {
args: func_args![value: "foobar", pattern: "bar", from: -1_i64],
want: Ok(value!(3)),
tdef: TypeDef::integer().infallible(),
}

negative_from_regex {
args: func_args![value: "foobar", pattern: Value::Regex(Regex::new("bar").unwrap().into()), from: -10_i64],
want: Ok(value!(3)),
tdef: TypeDef::integer().infallible(),
}

from_past_end {
args: func_args![value: "foobar", pattern: Value::Regex(Regex::new("bar").unwrap().into()), from: 100_i64],
want: Ok(value!(-1)),
tdef: TypeDef::integer().infallible(),
}
];
}
Loading