From 5bb8c1836f1aa4382b7411fb9ce00ecf3a555eea Mon Sep 17 00:00:00 2001
From: David Wendt <dcrkid@yahoo.com>
Date: Tue, 23 Jun 2020 19:26:05 -0400
Subject: [PATCH] Replace `as_bool` with `coerce_to_bool`.

Functions that need to assert Boolness without coercion should either:

1. Ensure their function declaration requires a Boolean. (We don't enforce type errors on ES4 typehints yet, but we should.)
2. Check the value type themselves and raise their own errors if necessary.

As it stands the only users of `as_bool` either needed to check the type themselves or use `coerce_to_bool`. Notably, `setPropertyIsEnumerable` doesn't appear to coerce *or* throw an error: it instead fails silently if you hand it a non-`Boolean` value.
---
 core/src/avm2/activation.rs     |  4 ++--
 core/src/avm2/globals/object.rs | 19 ++++++++++---------
 core/src/avm2/value.rs          | 23 +++++++++++++++--------
 3 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/core/src/avm2/activation.rs b/core/src/avm2/activation.rs
index c224f2c48..4134b54f9 100644
--- a/core/src/avm2/activation.rs
+++ b/core/src/avm2/activation.rs
@@ -1321,7 +1321,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> {
         offset: i32,
         reader: &mut Reader<Cursor<&[u8]>>,
     ) -> Result<FrameControl<'gc>, Error> {
-        let value = self.context.avm2.pop().as_bool()?;
+        let value = self.context.avm2.pop().coerce_to_boolean();
 
         if value {
             reader.seek(offset as i64)?;
@@ -1335,7 +1335,7 @@ impl<'a, 'gc, 'gc_context> Activation<'a, 'gc, 'gc_context> {
         offset: i32,
         reader: &mut Reader<Cursor<&[u8]>>,
     ) -> Result<FrameControl<'gc>, Error> {
-        let value = self.context.avm2.pop().as_bool()?;
+        let value = self.context.avm2.pop().coerce_to_boolean();
 
         if !value {
             reader.seek(offset as i64)?;
diff --git a/core/src/avm2/globals/object.rs b/core/src/avm2/globals/object.rs
index ab6ec4785..fab2afc37 100644
--- a/core/src/avm2/globals/object.rs
+++ b/core/src/avm2/globals/object.rs
@@ -124,16 +124,17 @@ pub fn set_property_is_enumerable<'gc>(
     let this = this?;
     let name: Result<&Value<'gc>, Error> = args.get(0).ok_or_else(|| "No name specified".into());
     let name = name?.as_string()?;
-    let is_enum = args
-        .get(1)
-        .cloned()
-        .unwrap_or(Value::Bool(true))
-        .as_bool()?;
 
-    if let Some(ns) = this.resolve_any(name)? {
-        if !ns.is_private() {
-            let qname = QName::new(ns, name);
-            this.set_local_property_is_enumerable(activation.context.gc_context, &qname, is_enum)?;
+    if let Some(Value::Bool(is_enum)) = args.get(1) {
+        if let Some(ns) = this.resolve_any(name)? {
+            if !ns.is_private() {
+                let qname = QName::new(ns, name);
+                this.set_local_property_is_enumerable(
+                    activation.context.gc_context,
+                    &qname,
+                    *is_enum,
+                )?;
+            }
         }
     }
 
diff --git a/core/src/avm2/value.rs b/core/src/avm2/value.rs
index e1f516c18..d043ae4de 100644
--- a/core/src/avm2/value.rs
+++ b/core/src/avm2/value.rs
@@ -225,18 +225,25 @@ impl<'gc> Value<'gc> {
         }
     }
 
-    pub fn as_bool(&self) -> Result<bool, Error> {
-        if let Value::Bool(b) = self {
-            Ok(*b)
-        } else {
-            Err(format!("Expected Boolean, found {:?}", self).into())
-        }
-    }
-
     pub fn as_namespace(&self) -> Result<&Namespace<'gc>, Error> {
         match self {
             Value::Namespace(ns) => Ok(ns),
             _ => Err(format!("Expected Namespace, found {:?}", self).into()),
         }
     }
+
+    /// Coerce the value to a boolean.
+    ///
+    /// Boolean coercion happens according to the rules specified in the ES4
+    /// draft proposals, which appear to be identical to ECMA-262 Edition 3.
+    pub fn coerce_to_boolean(&self) -> bool {
+        match self {
+            Value::Undefined | Value::Null => false,
+            Value::Bool(b) => *b,
+            Value::Number(f) => !f.is_nan() && f.abs() != 0.0,
+            Value::String(s) => !s.is_empty(),
+            Value::Namespace(_) => true,
+            Value::Object(_) => true,
+        }
+    }
 }