From abcb7540088be7175b96f1b949555b51d1be9d81 Mon Sep 17 00:00:00 2001 From: Lord-McSweeney Date: Fri, 13 Dec 2024 14:41:41 -0800 Subject: [PATCH] avm2: Fix some edge cases with the hasnext2, nextname, and nextvalue ops --- core/src/avm2/activation.rs | 129 ++++++++++++++++------- core/src/avm2/amf.rs | 12 +-- core/src/avm2/globals/flash/net.rs | 6 +- core/src/avm2/globals/json.rs | 2 +- core/src/avm2/object.rs | 4 +- core/src/avm2/object/array_object.rs | 12 ++- core/src/avm2/object/namespace_object.rs | 8 +- core/src/avm2/object/proxy_object.rs | 9 +- core/src/avm2/object/qname_object.rs | 8 +- core/src/avm2/object/script_object.rs | 3 +- core/src/avm2/object/vector_object.rs | 6 +- core/src/avm2/object/xml_list_object.rs | 10 +- core/src/avm2/object/xml_object.rs | 4 +- core/src/external.rs | 12 +-- 14 files changed, 131 insertions(+), 94 deletions(-) diff --git a/core/src/avm2/activation.rs b/core/src/avm2/activation.rs index fdc94c263bc2..b41ce44821da 100644 --- a/core/src/avm2/activation.rs +++ b/core/src/avm2/activation.rs @@ -2522,20 +2522,28 @@ impl<'a, 'gc> Activation<'a, 'gc> { } fn op_has_next(&mut self) -> Result, 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) @@ -2545,58 +2553,103 @@ impl<'a, 'gc> Activation<'a, 'gc> { object_register: u32, index_register: u32, ) -> Result, 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, 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, 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) } diff --git a/core/src/avm2/amf.rs b/core/src/avm2/amf.rs index dec582e0fa42..59db88a28364 100644 --- a/core/src/avm2/amf.rs +++ b/core/src/avm2/amf.rs @@ -213,15 +213,11 @@ 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) = @@ -229,7 +225,7 @@ pub fn recursive_serialize<'gc>( { elements.push(elem); } - last_index = obj.get_next_enumerant(index, activation)?; + last_index = obj.get_next_enumerant(last_index, activation)?; } Ok(()) } diff --git a/core/src/avm2/globals/flash/net.rs b/core/src/avm2/globals/flash/net.rs index 07dd7422b396..63153bf47c45 100644 --- a/core/src/avm2/globals/flash/net.rs +++ b/core/src/avm2/globals/flash/net.rs @@ -25,9 +25,9 @@ fn object_to_index_map<'gc>( ) -> Result, 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)? @@ -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) } diff --git a/core/src/avm2/globals/json.rs b/core/src/avm2/globals/json.rs index 8c46b1a625ad..ea23fdaf1214 100644 --- a/core/src/avm2/globals/json.rs +++ b/core/src/avm2/globals/json.rs @@ -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)?; diff --git a/core/src/avm2/object.rs b/core/src/avm2/object.rs index be0f89691843..937b3bd17534 100644 --- a/core/src/avm2/object.rs +++ b/core/src/avm2/object.rs @@ -789,7 +789,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy self, last_index: u32, _activation: &mut Activation<'_, 'gc>, - ) -> Result, Error<'gc>> { + ) -> Result> { let base = self.base(); Ok(base.get_next_enumerant(last_index)) @@ -808,7 +808,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into> + Clone + Copy ) -> Result, 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. diff --git a/core/src/avm2/object/array_object.rs b/core/src/avm2/object/array_object.rs index 8fa28cfd1295..6aa60afd470d 100644 --- a/core/src/avm2/object/array_object.rs +++ b/core/src/avm2/object/array_object.rs @@ -212,14 +212,14 @@ impl<'gc> TObject<'gc> for ArrayObject<'gc> { self, mut last_index: u32, _activation: &mut Activation<'_, 'gc>, - ) -> Result, Error<'gc>> { + ) -> Result> { 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); @@ -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( diff --git a/core/src/avm2/object/namespace_object.rs b/core/src/avm2/object/namespace_object.rs index 5ab498008560..60b848cc5572 100644 --- a/core/src/avm2/object/namespace_object.rs +++ b/core/src/avm2/object/namespace_object.rs @@ -126,12 +126,8 @@ impl<'gc> TObject<'gc> for NamespaceObject<'gc> { self, last_index: u32, _activation: &mut Activation<'_, 'gc>, - ) -> Result, Error<'gc>> { - Ok(if last_index < 2 { - Some(last_index + 1) - } else { - Some(0) - }) + ) -> Result> { + Ok(if last_index < 2 { last_index + 1 } else { 0 }) } fn get_enumerant_value( diff --git a/core/src/avm2/object/proxy_object.rs b/core/src/avm2/object/proxy_object.rs index fb4f59d138df..a0cd7ad4ed14 100644 --- a/core/src/avm2/object/proxy_object.rs +++ b/core/src/avm2/object/proxy_object.rs @@ -149,12 +149,11 @@ impl<'gc> TObject<'gc> for ProxyObject<'gc> { self, last_index: u32, activation: &mut Activation<'_, 'gc>, - ) -> Result, Error<'gc>> { + ) -> Result> { 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( diff --git a/core/src/avm2/object/qname_object.rs b/core/src/avm2/object/qname_object.rs index 63982b6618d3..b9502ea9384c 100644 --- a/core/src/avm2/object/qname_object.rs +++ b/core/src/avm2/object/qname_object.rs @@ -148,12 +148,8 @@ impl<'gc> TObject<'gc> for QNameObject<'gc> { self, last_index: u32, _activation: &mut Activation<'_, 'gc>, - ) -> Result, Error<'gc>> { - Ok(if last_index < 2 { - Some(last_index + 1) - } else { - Some(0) - }) + ) -> Result> { + Ok(if last_index < 2 { last_index + 1 } else { 0 }) } fn get_enumerant_value( diff --git a/core/src/avm2/object/script_object.rs b/core/src/avm2/object/script_object.rs index 14c76b69c32a..6c37957fff7a 100644 --- a/core/src/avm2/object/script_object.rs +++ b/core/src/avm2/object/script_object.rs @@ -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 { + 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> { diff --git a/core/src/avm2/object/vector_object.rs b/core/src/avm2/object/vector_object.rs index c4af585d2a68..67d2b3f1f6c8 100644 --- a/core/src/avm2/object/vector_object.rs +++ b/core/src/avm2/object/vector_object.rs @@ -216,11 +216,11 @@ impl<'gc> TObject<'gc> for VectorObject<'gc> { self, last_index: u32, _activation: &mut Activation<'_, 'gc>, - ) -> Result, Error<'gc>> { + ) -> Result> { 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) } } diff --git a/core/src/avm2/object/xml_list_object.rs b/core/src/avm2/object/xml_list_object.rs index fb12303e1173..19a6fcd247d5 100644 --- a/core/src/avm2/object/xml_list_object.rs +++ b/core/src/avm2/object/xml_list_object.rs @@ -1031,14 +1031,12 @@ impl<'gc> TObject<'gc> for XmlListObject<'gc> { self, last_index: u32, _activation: &mut Activation<'_, 'gc>, - ) -> Result, Error<'gc>> { + ) -> Result> { 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( diff --git a/core/src/avm2/object/xml_object.rs b/core/src/avm2/object/xml_object.rs index a3c25ef793d6..07af15022d5a 100644 --- a/core/src/avm2/object/xml_object.rs +++ b/core/src/avm2/object/xml_object.rs @@ -604,8 +604,8 @@ impl<'gc> TObject<'gc> for XmlObject<'gc> { self, last_index: u32, _activation: &mut Activation<'_, 'gc>, - ) -> Result, Error<'gc>> { - Ok(Some(if last_index == 0 { 1 } else { 0 })) + ) -> Result> { + Ok(if last_index == 0 { 1 } else { 0 }) } fn get_enumerant_value( diff --git a/core/src/external.rs b/core/src/external.rs index 60ae66a14f33..a018b47a1ddf 100644 --- a/core/src/external.rs +++ b/core/src/external.rs @@ -218,19 +218,15 @@ impl Value { let mut values = BTreeMap::new(); 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)?; values.insert(name.to_string(), Value::from_avm2(activation, value)?); - last_index = obj.get_next_enumerant(index, activation)?; + last_index = obj.get_next_enumerant(last_index, activation)?; } Value::Object(values)