Skip to content

Commit

Permalink
fix(boa): fixes concat limit
Browse files Browse the repository at this point in the history
- adds limit checks to array concat
- still fails
test/built-ins/Array/prototype/concat/arg-length-exceeding-integer-limit.js
- no longer runs forever

Closes #1306

add `is_concat_spreadable` utility

add array check

make suggested changes

perf

add docs & propagate error

add `is_concat_spreadable` utility

add array check

add `is_concat_spreadable` utility

add array check

make suggested changes

fix(boa): fixes concat limit

- adds limit checks to array concat
- still fails
test/built-ins/Array/prototype/concat/arg-length-exceeding-integer-limit.js
- no longer runs forever

Closes #1306

add `is_concat_spreadable` utility

add array check

make suggested changes

perf

add docs & propagate error

add docs & propagate error
  • Loading branch information
neeldug committed Jul 22, 2021
1 parent fafba6b commit 9b7314f
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 4 deletions.
48 changes: 45 additions & 3 deletions boa/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub mod array_iterator;
#[cfg(test)]
mod tests;

use crate::property::PropertyDescriptor;
use crate::{
builtins::array::array_iterator::{ArrayIterationKind, ArrayIterator},
builtins::BuiltIn,
Expand Down Expand Up @@ -280,6 +281,32 @@ impl Array {
Ok(array_obj_ptr)
}

/// Utility function for concatenating array objects.
///
/// Returns a Boolean valued property that if `true` indicates that
/// an object should be flattened to its array elements
/// by `Array.prototype.concat`.
pub(crate) fn is_concat_spreadable(this: &Value, context: &mut Context) -> Result<bool> {
// 1. If Type(O) is not Object, return false.
if !this.is_object() {
return Ok(false);
}
// 2. Let spreadable be ? Get(O, @@isConcatSpreadable).
let spreadable = this
.get_field(WellKnownSymbols::is_concat_spreadable(), context)?;

// 3. If spreadable is not undefined, return ! ToBoolean(spreadable).
if !spreadable.is_undefined() {
return Ok(spreadable.to_boolean());
}

// 4. Return ? IsArray(O).
match this.as_object() {
Some(obj) => Ok(obj.is_array()),
_ => Ok(false),
}
}

/// `get Array [ @@species ]`
///
/// The `Array [ @@species ]` accessor property returns the Array constructor.
Expand Down Expand Up @@ -432,19 +459,34 @@ impl Array {

// Make a new array (using this object as the prototype basis for the new
// one)
let mut new_values: Vec<Value> = Vec::new();
let mut new_values = Vec::new();

let this_length = this.get_field("length", context)?.to_length(context)?;
for n in 0..this_length {
new_values.push(this.get_field(n, context)?);
}

let mut n = this_length;

for concat_array in args {
let concat_length = concat_array
.get_field("length", context)?
.to_length(context)?;
for n in 0..concat_length {
new_values.push(concat_array.get_field(n, context)?);
let spreadable = Self::is_concat_spreadable(concat_array, context)?;
if spreadable {
if n + concat_length > Number::MAX_SAFE_INTEGER as usize {
return context.throw_type_error("Invalid array length");
}
for k in 0..concat_length {
new_values.push(concat_array.get_field(k, context)?);
n += 1;
}
} else {
if n >= Number::MAX_SAFE_INTEGER as usize {
return context.throw_type_error("Invalid array length");
}
new_values.push(concat_array.get_field(0, context)?);
n += 1;
}
}

Expand Down
1 change: 0 additions & 1 deletion test_ignore.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ feature:json-modules
//feature:class

// These seem to run forever:
arg-length-exceeding-integer-limit
15.4.4.19-8-c-ii-1

// These generate a stack overflow
Expand Down

0 comments on commit 9b7314f

Please sign in to comment.