avm2: Refactor readBytes & writeBytes

core/src/avm2/bytearray.rs

@@ -228,7 +228,7 @@ impl ByteArrayStorage {
     }
 
     #[inline]
-    pub fn bytes(&self) -> &Vec<u8> {
+    pub fn bytes(&self) -> &[u8] {
         &self.bytes
     }
 
@@ -252,11 +252,6 @@ impl ByteArrayStorage {
         self.position.set(pos);
     }
 
-    #[inline]
-    pub fn add_position(&self, amnt: usize) {
-        self.position.set(self.position.get() + amnt);
-    }
-
     #[inline]
     pub fn endian(&self) -> &Endian {
         &self.endian

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

@@ -82,12 +82,11 @@ pub fn write_bytes<'gc>(
     this: Option<Object<'gc>>,
     args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error> {
-    if let Some(Value::Object(second_array)) = args.get(0) {
+    if let Some(this) = this {
-        let combining_bytes = match second_array.as_bytearray() {
+        let bytearray = args
-            Some(b) => b.bytes().clone(),
+            .get(0)
-            None => return Err("ArgumentError: Parameter must be a bytearray".into()),
+            .unwrap_or(&Value::Undefined)
-        };
+            .coerce_to_object(activation)?;
-
         let offset = args
             .get(1)
             .unwrap_or(&Value::Unsigned(0))
@@ -97,19 +96,27 @@ pub fn write_bytes<'gc>(
             .unwrap_or(&Value::Unsigned(0))
             .coerce_to_u32(activation)? as usize;
 
-        // In the docs it says "If offset or length is out of range, they are clamped to the beginning and end of the bytes array."
+        let ba_read = bytearray
-        // However, in the actual flash player, it seems to just raise an error.
+            .as_bytearray()
-        if offset + length > combining_bytes.len() {
+            .ok_or("ArgumentError: Parameter must be a bytearray")?;
-            return Err("EOFError: Reached EOF".into());
+        // We need to clone the bytes that we are going to be writing because the ByteArray we are reading from
-        }
+        // could be the same ByteArray we are writing to
-        if let Some(this) = this {
+        let to_write = ba_read
-            if let Some(mut bytearray) = this.as_bytearray_mut(activation.context.gc_context) {
+            .read_at(
-                bytearray.write_bytes(if length != 0 {
+                // If length is 0, lets read the remaining bytes of ByteArray from the supplied offset
-                    &combining_bytes[offset..length + offset]
+                if length != 0 {
+                    length
                 } else {
-                    &combining_bytes[offset..]
+                    ba_read.len().saturating_sub(offset)
-                })?;
+                },
-            }
+                offset,
+            )
+            .map(|bytes| bytes.to_vec())?;
+
+        // Drop our bytearray ref so that writing to the given ByteArray will never panic
+        drop(ba_read);
+        if let Some(mut bytearray) = this.as_bytearray_mut(activation.context.gc_context) {
+            bytearray.write_bytes(&*to_write)?;
         }
     }
 
@@ -123,17 +130,6 @@ pub fn read_bytes<'gc>(
     args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error> {
     if let Some(this) = this {
-        let current_bytes = this
-            .as_bytearray_mut(activation.context.gc_context)
-            .unwrap()
-            .bytes()
-            .clone();
-        let position = this
-            .as_bytearray_mut(activation.context.gc_context)
-            .unwrap()
-            .position();
-        let mut merging_offset = 0;
-        if let Some(Value::Object(second_array)) = args.get(0) {
         let offset = args
             .get(1)
             .unwrap_or(&Value::Unsigned(0))
@@ -143,28 +139,38 @@ pub fn read_bytes<'gc>(
             .unwrap_or(&Value::Unsigned(0))
             .coerce_to_u32(activation)? as usize;
 
-            if position + length > current_bytes.len() {
+        if let Some(bytearray) = this.as_bytearray() {
-                return Err("EOFError: Reached EOF".into());
+            // We need to clone the bytes that we are going to be writing because the ByteArray we are reading from
-            }
+            // could be the same ByteArray we are writing to
-            if let Some(mut merging_storage) =
+            let to_write = bytearray
-                second_array.as_bytearray_mut(activation.context.gc_context)
+                .read_bytes(
-            {
+                    // If length is 0, lets read the remaining bytes of ByteArray
-                let to_write = if length != 0 {
+                    if length != 0 {
-                    &current_bytes[position..length + position]
+                        length
                     } else {
-                    &current_bytes[position..]
+                        bytearray.bytes_available()
-                };
+                    },
-                merging_offset = to_write.len();
+                )
-                merging_storage.write_at(to_write, offset)?;
+                .map(|bytes| bytes.to_vec())?;
-            } else {
-                return Err("ArgumentError: Parameter must be a bytearray".into());
-            }
-        }
-        this.as_bytearray_mut(activation.context.gc_context)
-            .unwrap()
-            .add_position(merging_offset);
-    }
 
+            // Drop our bytearray ref so that writing to the given ByteArray will never panic
+            drop(bytearray);
+
+            let bytearray = args
+                .get(0)
+                .unwrap_or(&Value::Undefined)
+                .coerce_to_object(activation)?;
+            let mut ba_write = bytearray
+                .as_bytearray_mut(activation.context.gc_context)
+                .ok_or("ArgumentError: Parameter must be a bytearray")?;
+
+            // Weird undocumented behavior: If offset is greater than the length of the ByteArray we are writing to, we do nothing.
+            if offset > ba_write.len() {
+                return Ok(Value::Undefined);
+            }
+            ba_write.write_at(&*to_write, offset)?;
+        }
+    }
     Ok(Value::Undefined)
 }
 pub fn write_utf<'gc>(