Scripts: operators, JSON.stringify and join count a flattened string against the heap limit

Flattening a concatenated string copies its units into a new buffer. The
string built-ins reserve that copy with flat_charged, but several other
paths flattened without it, so a script could build copies the heap limit
never saw:

- the relational operators (`<`, `>`, `<=`, `>=`) on two strings, `==`
  between a string and a number, unary `+` and every other ToNumber of a
  string, and reading a character by index (`s[0]`)
- a property name made from an object whose toString returns a long
  string, and a long string used as a property name, which now reserves
  its copy before it is made instead of after
- direct `eval` of a concatenated source
- `JSON.stringify` of a string value and its `space` argument
- `Array.prototype.join`'s separator and each string element it writes

Property names and eval source reserve the copy's heap bytes without
taking fuel.

crates/js/src/builtins/array.rs

@@ -13,6 +13,7 @@ use super::{
 };
 use crate::heap::{Class, NativeKind, ObjId};
 use crate::interp::{Abrupt, Interp, JsResult, strict_eq};
+use crate::string_builtins::flat_charged;
 use crate::value::{Str, Value, to_uint32};
 
 pub(super) fn install(interp: &mut Interp) -> JsResult<()> {
@@ -278,7 +279,7 @@ fn join_with(interp: &mut Interp, o: ObjId, separator: &Value, item: fn(&mut Int
     if len == 0 {
         return Ok(Value::str(""));
     }
-    let flat = sep.flat();
+    let flat = flat_charged(interp, &sep)?;
     let sep = &flat[..];
     // A result that cannot fit is refused before any element is read.
     let least = (sep.len() as u64).saturating_mul(u64::from(len - 1));
@@ -301,7 +302,8 @@ fn join_elements(interp: &mut Interp, o: ObjId, len: u32, sep: &[u16], item: fn(
         let Some(s) = item(interp, v)? else { continue };
         out.push_repeated(interp, sep, k.saturating_sub(seps))?;
         seps = k;
-        out.push_units(interp, &s.flat())?;
+        let units = flat_charged(interp, &s)?;
+        out.push_units(interp, &units)?;
     }
     out.push_repeated(interp, sep, u64::from(len - 1).saturating_sub(seps))?;
     Ok(out.finish(interp))

crates/js/src/builtins/json.rs

@@ -13,6 +13,7 @@ use std::collections::HashSet;
 use super::{Builder, ELEMENT_BYTES, arg, data, key_of, length_of, method, new_array, new_string, to_integer};
 use crate::heap::{Class, Key, OBJECT_BYTES, OPEN, ObjId, Object, Prop, key, string_bytes};
 use crate::interp::{Desc, ErrorType, Hint, Interp, JsResult};
+use crate::string_builtins::flat_charged;
 use crate::value::{Str, Value, number_to_str};
 
 pub(super) fn install(interp: &mut Interp) -> JsResult<()> {
@@ -473,7 +474,7 @@ fn settings(interp: &mut Interp, replacer: &Value, space: &Value) -> JsResult<Se
             let n = to_integer(interp, &space)?.clamp(0.0, 10.0) as usize;
             s.gap = vec![u16::from(b' '); n];
         }
-        Value::String(t) => s.gap = t.flat().iter().take(10).copied().collect(),
+        Value::String(t) => s.gap = flat_charged(interp, &t)?.iter().take(10).copied().collect(),
         _ => {}
     }
     Ok(s)
@@ -632,7 +633,10 @@ fn write_primitive(interp: &mut Interp, out: &mut Builder, v: &Value) -> JsResul
         Value::Bool(b) => out.push_str(interp, if *b { "true" } else { "false" }),
         Value::Number(n) if n.is_finite() => out.push_units(interp, &number_to_str(*n).flat()),
         Value::Number(_) => out.push_str(interp, "null"),
-        Value::String(s) => write_quoted(interp, out, &s.flat()),
+        Value::String(s) => {
+            let units = flat_charged(interp, s)?;
+            write_quoted(interp, out, &units)
+        }
         Value::Undefined | Value::Object(_) => Ok(()),
     }
 }

crates/js/src/interp.rs

@@ -26,6 +26,7 @@ use crate::heap::{
     ArgMap, Binding, CONFIGURABLE, Class, ENUMERABLE, ENV_BYTES, Env, EnvId, ForIn, Func, HIDDEN, Heap, Key, NativeKind, OBJECT_BYTES, OPEN, ObjId,
     Object, PROP_BYTES, Prop, PropMap, ROPE_BYTES, Roots, WRITABLE, code_bytes, key, string_bytes,
 };
+use crate::string_builtins::flat_charged;
 use crate::value::{Str, Value, array_index, index_key, number_to_str, string_to_number, to_int32, to_uint32};
 use crate::{LimitKind, Limits, Options};
 
@@ -1290,7 +1291,7 @@ impl Interp {
             if let Some(i) = array_index(k)
                 && (i as usize) < s.len()
             {
-                let unit = s.flat().get(i as usize).copied().unwrap_or(0);
+                let unit = flat_charged(self, s)?.get(i as usize).copied().unwrap_or(0);
                 return Ok(Value::String(Str::from_units(vec![unit])));
             }
         }
@@ -2397,7 +2398,7 @@ impl Interp {
         };
         let strict = self.strict();
         self.check_source_units(s.len()).map_err(|e| self.compile_error_to_abrupt(e))?;
-        let units = s.flat();
+        let units = self.flat_reserved(s)?;
         let code = self.compile_source(&units, strict, CodeKind::Eval).map_err(|e| self.compile_error_to_abrupt(e))?;
         self.charge_code(&code, declaration_bytes(&code).saturating_add(ENV_BYTES))?;
         let frame = self.frame()?;
@@ -2562,7 +2563,7 @@ impl Interp {
             Value::Null => 0.0,
             Value::Bool(b) => f64::from(u8::from(*b)),
             Value::Number(n) => *n,
-            Value::String(s) => string_to_number(&s.flat()),
+            Value::String(s) => string_to_number(&flat_charged(self, s)?),
             Value::Object(_) => {
                 let p = self.to_primitive(v, Hint::Number)?;
                 return self.to_number(&p);
@@ -2596,19 +2597,27 @@ impl Interp {
     #[allow(clippy::wrong_self_convention)]
     pub(crate) fn to_key(&mut self, v: &Value) -> JsResult<Key> {
         match v {
-            Value::String(s) => {
-                let rope = s.is_rope();
-                let flat = s.flat();
-                if rope {
-                    self.heap.bytes += string_bytes(flat.len());
-                }
-                Ok(flat)
-            }
+            Value::String(s) => self.flat_reserved(s),
             Value::Number(n) if *n >= 0.0 && *n < 4_294_967_295.0 && n.fract() == 0.0 => Ok(index_key(*n as u32)),
-            _ => Ok(self.to_str(v)?.flat()),
+            _ => {
+                let s = self.to_str(v)?;
+                self.flat_reserved(&s)
+            }
         }
     }
 
+    /// The code units of `s`. Flattening a rope makes a copy of its units, so for a rope that
+    /// copy's bytes are reserved and added to the heap first, without taking fuel. Everything
+    /// the caller still needs must be reachable from the roots.
+    fn flat_reserved(&mut self, s: &Str) -> JsResult<Key> {
+        if s.is_rope() {
+            let bytes = string_bytes(s.len());
+            self.reserve_heap(bytes)?;
+            self.heap.bytes = self.heap.bytes.saturating_add(bytes);
+        }
+        Ok(s.flat())
+    }
+
     /// ToObject (section 9.9).
     #[allow(clippy::wrong_self_convention)]
     pub(crate) fn to_object(&mut self, v: &Value) -> JsResult<ObjId> {
@@ -2731,7 +2740,8 @@ impl Interp {
             (self.to_primitive(x, Hint::Number)?, py)
         };
         if let (Value::String(a), Value::String(b)) = (&px, &py) {
-            return Ok(Some(*a.flat() < *b.flat()));
+            let (a, b) = (flat_charged(self, a)?, flat_charged(self, b)?);
+            return Ok(Some(*a < *b));
         }
         let nx = self.to_number(&px)?;
         let ny = self.to_number(&py)?;
@@ -2756,7 +2766,9 @@ impl Interp {
                 | (Value::String(_), Value::String(_))
                 | (Value::Bool(_), Value::Bool(_))
                 | (Value::Object(_), Value::Object(_)) => return Ok(strict_eq(&x, &y)),
-                (Value::Number(n), Value::String(s)) | (Value::String(s), Value::Number(n)) => return Ok(*n == string_to_number(&s.flat())),
+                (Value::Number(n), Value::String(s)) | (Value::String(s), Value::Number(n)) => {
+                    return Ok(*n == string_to_number(&flat_charged(self, s)?));
+                }
                 (Value::Bool(v), _) => x = Value::Number(f64::from(u8::from(*v))),
                 (_, Value::Bool(v)) => y = Value::Number(f64::from(u8::from(*v))),
                 (Value::Number(_) | Value::String(_), Value::Object(_)) => y = self.to_primitive(&y, Hint::Default)?,

crates/js/src/text_tests.rs

@@ -309,6 +309,23 @@ fn flattening_a_rope_counts_against_the_heap() {
     }
 }
 
+#[test]
+fn operators_and_builtins_count_a_flattened_rope_against_the_heap() {
+    // Three ropes of about 524,288 units, each 1 MiB once flat, with a 3 MiB heap.
+    let build = "var s = 'x'; for (var i = 0; i < 19; i++) s = s + s; var t = s + 'y', u = s + 'z';";
+    let limits = Limits { heap_bytes: 3 << 20, ..Limits::default() };
+    let not_stopped: Vec<_> =
+        ["X < 'a'", "X == 1", "+X", "X[0]", "({})[{ toString: function () { return X; } }]", "JSON.stringify(0, null, X)", "[1].join(X)"]
+            .into_iter()
+            .filter(|read| {
+                let src = format!("{build} {}; {}; {}; 0", read.replace('X', "s"), read.replace('X', "t"), read.replace('X', "u"));
+                let result = Realm::with_options(Options::default(), limits.clone()).run(&src);
+                !matches!(result, Err(ref e) if e.kind == RunErrorKind::Limit(LimitKind::Heap))
+            })
+            .collect();
+    assert!(not_stopped.is_empty(), "not stopped by the heap limit: {not_stopped:?}");
+}
+
 #[test]
 fn compiling_a_pattern_counts_against_the_heap() {
     let src = "var s = '[\\\\s\\\\S]a*'; for (var i = 0; i < 14; i++) s = s + s; new RegExp(s).source.length";