Skip to content

Commit

Permalink
avm2: Fix some edge cases with the hasnext2, nextname, and nextvalue ops
Browse files Browse the repository at this point in the history
  • Loading branch information
Lord-McSweeney authored and Lord-McSweeney committed Dec 13, 2024
1 parent 9f95699 commit abcb754
Show file tree
Hide file tree
Showing 14 changed files with 131 additions and 94 deletions.
129 changes: 91 additions & 38 deletions core/src/avm2/activation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2522,20 +2522,28 @@ impl<'a, 'gc> Activation<'a, 'gc> {
}

fn op_has_next(&mut self) -> Result<FrameControl<'gc>, Error<'gc>> {
let cur_index = self.pop_stack().coerce_to_u32(self)?;
let cur_index = self.pop_stack().coerce_to_i32(self)?;

if cur_index < 0 {
self.push_raw(false);

return Ok(FrameControl::Continue);
}

let value = self.pop_stack();

let object = match value {
Value::Undefined | Value::Null => None,
Value::Object(obj) => Some(obj),
_ => value.proto(self),
};

if let Some(object) = object {
let next_index = object.get_next_enumerant(cur_index as u32, self)?;

let object = self.pop_stack();
if matches!(object, Value::Undefined | Value::Null) {
self.push_raw(0.0);
} else if let Some(next_index) = object
.as_object()
.map(|o| o.get_next_enumerant(cur_index, self))
.transpose()?
.flatten()
{
self.push_raw(next_index);
} else {
self.push_raw(0.0);
self.push_raw(0);
}

Ok(FrameControl::Continue)
Expand All @@ -2545,58 +2553,103 @@ impl<'a, 'gc> Activation<'a, 'gc> {
object_register: u32,
index_register: u32,
) -> Result<FrameControl<'gc>, Error<'gc>> {
let mut cur_index = self.local_register(index_register).coerce_to_u32(self)?;
let cur_index = self.local_register(index_register).coerce_to_i32(self)?;

let object = self.local_register(object_register);
if cur_index < 0 {
self.push_raw(false);

let mut object = if matches!(object, Value::Undefined | Value::Null) {
None
} else {
Some(object.coerce_to_object(self)?)
};
return Ok(FrameControl::Continue);
}

while let Some(cur_object) = object {
if let Some(index) = cur_object.get_next_enumerant(cur_index, self)? {
cur_index = index;
break;
} else {
let mut cur_index = cur_index as u32;

let object_reg = self.local_register(object_register);
let mut result_value = object_reg;
let mut object = None;

match object_reg {
Value::Undefined | Value::Null => {
cur_index = 0;
object = cur_object.proto();
}
Value::Object(obj) => {
object = obj.proto();
cur_index = obj.get_next_enumerant(cur_index, self)?;
}
value => {
let proto = value.proto(self);
if let Some(proto) = proto {
object = proto.proto();
cur_index = proto.get_next_enumerant(cur_index, self)?;
}
}
};

while let (Some(cur_object), 0) = (object, cur_index) {
cur_index = cur_object.get_next_enumerant(cur_index, self)?;
result_value = cur_object.into();

object = cur_object.proto();
}

if object.is_none() {
cur_index = 0;
if cur_index == 0 {
result_value = Value::Null;
}

self.push_raw(cur_index != 0);
self.set_local_register(index_register, cur_index);
self.set_local_register(
object_register,
object.map(|v| v.into()).unwrap_or(Value::Null),
);
self.set_local_register(object_register, result_value);

Ok(FrameControl::Continue)
}

fn op_next_name(&mut self) -> Result<FrameControl<'gc>, Error<'gc>> {
let cur_index = self.pop_stack().coerce_to_number(self)?;
let object = self.pop_stack().coerce_to_object_or_typeerror(self, None)?;
let cur_index = self.pop_stack().coerce_to_i32(self)?;

let name = object.get_enumerant_name(cur_index as u32, self)?;
if cur_index <= 0 {
self.push_stack(Value::Null);

self.push_stack(name);
return Ok(FrameControl::Continue);
}

let value = self.pop_stack();
let object = match value.null_check(self, None)? {
Value::Object(obj) => Some(obj),
value => value.proto(self),
};

if let Some(object) = object {
let name = object.get_enumerant_name(cur_index as u32, self)?;

self.push_stack(name);
} else {
self.push_stack(Value::Undefined);
}

Ok(FrameControl::Continue)
}

fn op_next_value(&mut self) -> Result<FrameControl<'gc>, Error<'gc>> {
let cur_index = self.pop_stack().coerce_to_number(self)?;
let object = self.pop_stack().coerce_to_object_or_typeerror(self, None)?;
let cur_index = self.pop_stack().coerce_to_i32(self)?;

let value = object.get_enumerant_value(cur_index as u32, self)?;
if cur_index <= 0 {
self.push_stack(Value::Undefined);

self.push_stack(value);
return Ok(FrameControl::Continue);
}

let value = self.pop_stack();
let object = match value.null_check(self, None)? {
Value::Object(obj) => Some(obj),
value => value.proto(self),
};

if let Some(object) = object {
let value = object.get_enumerant_value(cur_index as u32, self)?;

self.push_stack(value);
} else {
self.push_stack(Value::Undefined);
}

Ok(FrameControl::Continue)
}
Expand Down
12 changes: 4 additions & 8 deletions core/src/avm2/amf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,23 +213,19 @@ pub fn recursive_serialize<'gc>(

// FIXME: Flash only seems to use this enumeration for dynamic classes.
let mut last_index = obj.get_next_enumerant(0, activation)?;
while let Some(index) = last_index {
if index == 0 {
break;
}

while last_index != 0 {
let name = obj
.get_enumerant_name(index, activation)?
.get_enumerant_name(last_index, activation)?
.coerce_to_string(activation)?;
let value = obj.get_enumerant_value(index, activation)?;
let value = obj.get_enumerant_value(last_index, activation)?;

let name = name.to_utf8_lossy().to_string();
if let Some(elem) =
get_or_create_element(activation, name.clone(), value, object_table, amf_version)
{
elements.push(elem);
}
last_index = obj.get_next_enumerant(index, activation)?;
last_index = obj.get_next_enumerant(last_index, activation)?;
}
Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions core/src/avm2/globals/flash/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ fn object_to_index_map<'gc>(
) -> Result<IndexMap<String, String>, Error<'gc>> {
let mut map = IndexMap::new();
let mut last_index = obj.get_next_enumerant(0, activation)?;
while let Some(index) = last_index {
while last_index != 0 {
let name = obj
.get_enumerant_name(index, activation)?
.get_enumerant_name(last_index, activation)?
.coerce_to_string(activation)?;
let value = obj
.get_public_property(name, activation)?
Expand All @@ -37,7 +37,7 @@ fn object_to_index_map<'gc>(

let name = name.to_utf8_lossy().to_string();
map.insert(name, value);
last_index = obj.get_next_enumerant(index, activation)?;
last_index = obj.get_next_enumerant(last_index, activation)?;
}
Ok(map)
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm2/globals/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl<'gc> AvmSerializer<'gc> {
}
for i in 1.. {
match obj.get_enumerant_name(i, activation)? {
Value::Undefined => break,
Value::Null => break,
name_val => {
let name = name_val.coerce_to_string(activation)?;
let value = obj.get_public_property(name, activation)?;
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
self,
last_index: u32,
_activation: &mut Activation<'_, 'gc>,
) -> Result<Option<u32>, Error<'gc>> {
) -> Result<u32, Error<'gc>> {
let base = self.base();

Ok(base.get_next_enumerant(last_index))
Expand All @@ -808,7 +808,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
) -> Result<Value<'gc>, Error<'gc>> {
let base = self.base();

Ok(base.get_enumerant_name(index).unwrap_or(Value::Undefined))
Ok(base.get_enumerant_name(index).unwrap_or(Value::Null))
}

/// Retrieve a given enumerable value by index.
Expand Down
12 changes: 7 additions & 5 deletions core/src/avm2/object/array_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,14 @@ impl<'gc> TObject<'gc> for ArrayObject<'gc> {
self,
mut last_index: u32,
_activation: &mut Activation<'_, 'gc>,
) -> Result<Option<u32>, Error<'gc>> {
) -> Result<u32, Error<'gc>> {
let array = self.0.array.borrow();

let array_length = array.length() as u32;

// Array enumeration skips over holes.
if let Some(index) = array.get_next_enumerant(last_index as usize) {
return Ok(Some(index as u32));
return Ok(index as u32);
}

last_index = std::cmp::max(last_index, array_length);
Expand All @@ -229,10 +229,12 @@ impl<'gc> TObject<'gc> for ArrayObject<'gc> {
// After enumerating all of the 'normal' array entries,
// we enumerate all of the local properties stored on the
// ScriptObject.
if let Some(index) = self.base().get_next_enumerant(last_index - array_length) {
return Ok(Some(index + array_length));
let index = self.base().get_next_enumerant(last_index - array_length);
if index != 0 {
return Ok(index + array_length);
}
Ok(None)

Ok(0)
}

fn get_enumerant_name(
Expand Down
8 changes: 2 additions & 6 deletions core/src/avm2/object/namespace_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,8 @@ impl<'gc> TObject<'gc> for NamespaceObject<'gc> {
self,
last_index: u32,
_activation: &mut Activation<'_, 'gc>,
) -> Result<Option<u32>, Error<'gc>> {
Ok(if last_index < 2 {
Some(last_index + 1)
} else {
Some(0)
})
) -> Result<u32, Error<'gc>> {
Ok(if last_index < 2 { last_index + 1 } else { 0 })
}

fn get_enumerant_value(
Expand Down
9 changes: 4 additions & 5 deletions core/src/avm2/object/proxy_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,11 @@ impl<'gc> TObject<'gc> for ProxyObject<'gc> {
self,
last_index: u32,
activation: &mut Activation<'_, 'gc>,
) -> Result<Option<u32>, Error<'gc>> {
) -> Result<u32, Error<'gc>> {
let prop = Multiname::new(activation.avm2().namespaces.proxy, "nextNameIndex");
Ok(Some(
self.call_property(&prop, &[last_index.into()], activation)?
.coerce_to_u32(activation)?,
))
Ok(self
.call_property(&prop, &[last_index.into()], activation)?
.coerce_to_u32(activation)?)
}

fn get_enumerant_name(
Expand Down
8 changes: 2 additions & 6 deletions core/src/avm2/object/qname_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,8 @@ impl<'gc> TObject<'gc> for QNameObject<'gc> {
self,
last_index: u32,
_activation: &mut Activation<'_, 'gc>,
) -> Result<Option<u32>, Error<'gc>> {
Ok(if last_index < 2 {
Some(last_index + 1)
} else {
Some(0)
})
) -> Result<u32, Error<'gc>> {
Ok(if last_index < 2 { last_index + 1 } else { 0 })
}

fn get_enumerant_value(
Expand Down
3 changes: 2 additions & 1 deletion core/src/avm2/object/script_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,11 @@ impl<'gc> ScriptObjectWrapper<'gc> {
unlock!(Gc::write(mc, self.0), ScriptObjectData, proto).set(Some(proto));
}

pub fn get_next_enumerant(&self, last_index: u32) -> Option<u32> {
pub fn get_next_enumerant(&self, last_index: u32) -> u32 {
self.values()
.next(last_index as usize)
.map(|val| val as u32)
.unwrap_or(0)
}

pub fn get_enumerant_name(&self, index: u32) -> Option<Value<'gc>> {
Expand Down
6 changes: 3 additions & 3 deletions core/src/avm2/object/vector_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,11 +216,11 @@ impl<'gc> TObject<'gc> for VectorObject<'gc> {
self,
last_index: u32,
_activation: &mut Activation<'_, 'gc>,
) -> Result<Option<u32>, Error<'gc>> {
) -> Result<u32, Error<'gc>> {
if last_index < self.0.vector.borrow().length() as u32 {
Ok(Some(last_index.saturating_add(1)))
Ok(last_index.saturating_add(1))
} else {
Ok(None)
Ok(0)
}
}

Expand Down
10 changes: 4 additions & 6 deletions core/src/avm2/object/xml_list_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1031,14 +1031,12 @@ impl<'gc> TObject<'gc> for XmlListObject<'gc> {
self,
last_index: u32,
_activation: &mut Activation<'_, 'gc>,
) -> Result<Option<u32>, Error<'gc>> {
) -> Result<u32, Error<'gc>> {
if (last_index as usize) < self.0.children.borrow().len() {
return Ok(Some(last_index + 1));
return Ok(last_index + 1);
}
// Return `Some(0)` instead of `None`, as we do *not* want to
// fall back to the prototype chain. XMLList is special, and enumeration
// *only* ever considers the XML children.
Ok(Some(0))

Ok(0)
}

fn get_enumerant_value(
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm2/object/xml_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -604,8 +604,8 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> {
self,
last_index: u32,
_activation: &mut Activation<'_, 'gc>,
) -> Result<Option<u32>, Error<'gc>> {
Ok(Some(if last_index == 0 { 1 } else { 0 }))
) -> Result<u32, Error<'gc>> {
Ok(if last_index == 0 { 1 } else { 0 })
}

fn get_enumerant_value(
Expand Down
Loading

0 comments on commit abcb754

Please sign in to comment.