avm2: Correctness fixes to AMF serialization (#13537)

Fixes some cases where `amf::serialize_value` returns `None`
---------

Co-authored-by: Lord-McSweeney <Lord-McSweeney@github.com>

core/src/avm2/amf.rs

@@ -110,7 +110,7 @@ pub fn serialize_value<'gc>(
                         .iter()
                         .map(|v| {
                             serialize_value(activation, v, amf_version, object_table)
-                                .expect("Unexpected non-object value in object vector")
+                                .unwrap_or(AmfValue::Undefined)
                         })
                         .collect();
 
@@ -421,7 +421,17 @@ pub fn deserialize_value<'gc>(
             let class = alias_to_class(activation, name)?;
             let storage = VectorStorage::from_values(
                 vec.iter()
-                    .map(|v| deserialize_value(activation, v))
+                    .map(|v| {
+                        deserialize_value(activation, v).map(|value| {
+                            // There's no Vector.<void>: convert any
+                            // Undefined items in the Vector to Null.
+                            if matches!(value, Value::Undefined) {
+                                Value::Null
+                            } else {
+                                value
+                            }
+                        })
+                    })
                     .collect::<Result<Vec<_>, _>>()?,
                 *is_fixed,
                 Some(class),

core/src/avm2/globals/flash/net/socket.rs

@@ -646,24 +646,28 @@ pub fn write_object<'gc>(
             ObjectEncoding::Amf0 => AMFVersion::AMF0,
             ObjectEncoding::Amf3 => AMFVersion::AMF3,
         };
-        if let Some(amf) =
-            crate::avm2::amf::serialize_value(activation, obj, amf_version, &mut Default::default())
-        {
-            let element = Element::new("", Rc::new(amf));
-            let mut lso = flash_lso::types::Lso::new(vec![element], "", amf_version);
-            let bytes = flash_lso::write::write_to_bytes(&mut lso)
-                .map_err(|_| "Failed to serialize object")?;
-            // This is kind of hacky: We need to strip out the header and any padding so that we only write
-            // the value. In the future, there should be a method to do this in the flash_lso crate.
-            let element_padding = match amf_version {
-                AMFVersion::AMF0 => 8,
-                AMFVersion::AMF3 => 7,
-            };
-            socket.write_bytes(
-                &bytes[flash_lso::write::header_length(&lso.header) + element_padding
-                    ..bytes.len() - 1],
-            );
-        }
+
+        let amf = crate::avm2::amf::serialize_value(
+            activation,
+            obj,
+            amf_version,
+            &mut Default::default(),
+        )
+        .unwrap_or(flash_lso::types::Value::Undefined);
+
+        let element = Element::new("", Rc::new(amf));
+        let mut lso = flash_lso::types::Lso::new(vec![element], "", amf_version);
+        let bytes =
+            flash_lso::write::write_to_bytes(&mut lso).map_err(|_| "Failed to serialize object")?;
+        // This is kind of hacky: We need to strip out the header and any padding so that we only write
+        // the value. In the future, there should be a method to do this in the flash_lso crate.
+        let element_padding = match amf_version {
+            AMFVersion::AMF0 => 8,
+            AMFVersion::AMF3 => 7,
+        };
+        socket.write_bytes(
+            &bytes[flash_lso::write::header_length(&lso.header) + element_padding..bytes.len() - 1],
+        );
     }
 
     Ok(Value::Undefined)

core/src/avm2/globals/flash/utils/byte_array.rs

@@ -758,6 +758,7 @@ pub fn read_object<'gc>(
         let bytes = bytearray
             .read_at(bytearray.bytes_available(), bytearray.position())
             .map_err(|e| e.to_avm(activation))?;
+
         let (bytes_left, value) = match bytearray.object_encoding() {
             ObjectEncoding::Amf0 => {
                 let mut decoder = AMF0Decoder::default();
@@ -799,26 +800,31 @@ pub fn write_object<'gc>(
             ObjectEncoding::Amf0 => AMFVersion::AMF0,
             ObjectEncoding::Amf3 => AMFVersion::AMF3,
         };
-        if let Some(amf) =
-            crate::avm2::amf::serialize_value(activation, obj, amf_version, &mut Default::default())
-        {
-            let element = Element::new("", Rc::new(amf));
-            let mut lso = flash_lso::types::Lso::new(vec![element], "", amf_version);
-            let bytes = flash_lso::write::write_to_bytes(&mut lso)
-                .map_err(|_| "Failed to serialize object")?;
-            // This is kind of hacky: We need to strip out the header and any padding so that we only write
-            // the value. In the future, there should be a method to do this in the flash_lso crate.
-            let element_padding = match amf_version {
-                AMFVersion::AMF0 => 8,
-                AMFVersion::AMF3 => 7,
-            };
-            bytearray
-                .write_bytes(
-                    &bytes[flash_lso::write::header_length(&lso.header) + element_padding
-                        ..bytes.len() - 1],
-                )
-                .map_err(|e| e.to_avm(activation))?;
-        }
+
+        let amf = crate::avm2::amf::serialize_value(
+            activation,
+            obj,
+            amf_version,
+            &mut Default::default(),
+        )
+        .unwrap_or(flash_lso::types::Value::Undefined);
+
+        let element = Element::new("", Rc::new(amf));
+        let mut lso = flash_lso::types::Lso::new(vec![element], "", amf_version);
+        let bytes =
+            flash_lso::write::write_to_bytes(&mut lso).map_err(|_| "Failed to serialize object")?;
+        // This is kind of hacky: We need to strip out the header and any padding so that we only write
+        // the value. In the future, there should be a method to do this in the flash_lso crate.
+        let element_padding = match amf_version {
+            AMFVersion::AMF0 => 8,
+            AMFVersion::AMF3 => 7,
+        };
+        bytearray
+            .write_bytes(
+                &bytes[flash_lso::write::header_length(&lso.header) + element_padding
+                    ..bytes.len() - 1],
+            )
+            .map_err(|e| e.to_avm(activation))?;
     }
 
     Ok(Value::Undefined)

tests/tests/swfs/avm2/amf_function/Test.as

@@ -0,0 +1,66 @@
+package {
+   import flash.display.MovieClip;
+   import flash.utils.*;
+
+   public class Test extends MovieClip {
+      public function Test() {
+         var t1:Function = new Function();
+         var t2:Vector.<Function> = Vector.<Function>([new Function(),new Function(),new Function()]);
+         var t3:Vector.<Function> = Vector.<Function>([null,new Function()]);
+         var t4:Vector.<Function> = Vector.<Function>([]);
+         var t5:Array = [new Function(),new Function()];
+         runTest("Just function",0,t1);
+         runTest("Just function",3,t1);
+         runTest("Function vector",3,t2);
+         runTest("Function vector with null element",3,t3);
+         runTest("Empty function vector",3,t4);
+         runTest("Array with two function elements",3,t5);
+      }
+      
+      public function printByteArray(name:String, array:ByteArray):void {
+         trace("Printing ByteArray:");
+         var str:* = "";
+         var i:* = 0;
+         while(i < array.length)
+         {
+            str += array[i];
+            if(i != array.length - 1)
+            {
+               str += ", ";
+            }
+            i++;
+         }
+         trace(str);
+      }
+      
+      public function runTest(name:String, amfversion:*, obj:*):void {
+         trace("Running test \"" + name + "\" with AMF" + amfversion);
+         var bytearray:* = new ByteArray();
+         bytearray.objectEncoding = amfversion;
+         bytearray.writeObject(obj);
+         printByteArray(name,bytearray);
+         bytearray.position = 0;
+         var read:* = bytearray.readObject();
+         trace("read back: " + getQualifiedClassName(read));
+         if(read && read instanceof Vector.<*>)
+         {
+            trace("Was vector, length " + read.length + ". Elements:");
+            for(var i in read)
+            {
+               trace(i + "th element: " + read[i]);
+            }
+         }
+         if(read && read instanceof Array)
+         {
+            trace("Was array, length " + read.length + ". Elements:");
+            for(i in read)
+            {
+               trace(i + "th element: " + read[i]);
+            }
+         }
+         trace(read);
+         trace("Done with test!");
+      }
+   }
+}
+

tests/tests/swfs/avm2/amf_function/output.txt

@@ -0,0 +1,46 @@
+Running test "Just function" with AMF0
+Printing ByteArray:
+6
+read back: void
+undefined
+Done with test!
+Running test "Just function" with AMF3
+Printing ByteArray:
+0
+read back: void
+undefined
+Done with test!
+Running test "Function vector" with AMF3
+Printing ByteArray:
+16, 7, 0, 1, 0, 0, 0
+read back: __AS3__.vec::Vector.<Object>
+Was vector, length 3. Elements:
+0th element: null
+1th element: null
+2th element: null
+null,null,null
+Done with test!
+Running test "Function vector with null element" with AMF3
+Printing ByteArray:
+16, 5, 0, 1, 1, 0
+read back: __AS3__.vec::Vector.<Object>
+Was vector, length 2. Elements:
+0th element: null
+1th element: null
+null,null
+Done with test!
+Running test "Empty function vector" with AMF3
+Printing ByteArray:
+16, 1, 0, 1
+read back: __AS3__.vec::Vector.<Object>
+Was vector, length 0. Elements:
+
+Done with test!
+Running test "Array with two function elements" with AMF3
+Printing ByteArray:
+9, 1, 1
+read back: Array
+Was array, length 0. Elements:
+
+Done with test!
+should be undefined: undefined

tests/tests/swfs/avm2/amf_function/test.toml

@@ -0,0 +1 @@
+num_frames = 1