From fb155d8df5670e7a98f4d1e7e212b4fd74d7d233 Mon Sep 17 00:00:00 2001
From: Mike Gerwitz <mike@mikegerwitz.com>
Date: Tue, 14 Jun 2011 18:24:18 -0400
Subject: [PATCH] Getters/setters can no longer override properties

---
 lib/member_builder.js                    | 17 ++++++++++----
 test/test-member_builder-gettersetter.js | 30 ++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/lib/member_builder.js b/lib/member_builder.js
index cab1729..5b8571f 100644
--- a/lib/member_builder.js
+++ b/lib/member_builder.js
@@ -361,10 +361,14 @@ function validateGetterSetter( members, keywords, name, base )
 
     if ( prev )
     {
-        if ( typeof prev === 'function' )
+        // To speed up the system we'll simply check for a getter/setter, rather
+        // than checking separately for methods/properties. This is at the
+        // expense of more detailed error messages. They'll live.
+        if ( !( prev_data.get || prev_data.set ) )
         {
             throw TypeError(
-                "Cannot override method '" + name + "' with getter/setter"
+                "Cannot override method or property '" + name +
+                    "' with getter/setter"
             );
         }
     }
@@ -430,10 +434,15 @@ function scanMembers( members, name, base )
     // locate requested member by scanning each level of visibility
     while ( i-- )
     {
-        if ( member = members[ visibility[ i ] ][ name ] )
+        var visobj = members[ visibility[ i ] ];
+
+        // in order to support getters/setters, we must go off of the descriptor
+        if ( member = util.getPropertyDescriptor( visobj, name ) )
         {
             return {
-                member:     member,
+                get:        member.get,
+                set:        member.set,
+                member:     member.value,
                 visibility: ( ( fallback ) ? 0 : i ),
             };
         }
diff --git a/test/test-member_builder-gettersetter.js b/test/test-member_builder-gettersetter.js
index 4708d27..d17d461 100644
--- a/test/test-member_builder-gettersetter.js
+++ b/test/test-member_builder-gettersetter.js
@@ -231,3 +231,33 @@ testEach( function testCannotOverrideMethodWithGetterOrSetter( type, build )
     assert.fail( type + " should not be able to override methods");
 } );
 
+
+/**
+ * Getters/setters should not be able to override properties. While, at first,
+ * this concept may seem odd, keep in mind that the parent would likely not
+ * expect a subtype to be able to override property assignments. This could open
+ * up holes to exploit the parent class.
+ */
+testEach( function testCannotOverridePropertiesWithGetterOrSetter( type, build )
+{
+    setUp();
+
+    // declare a property
+    members[ 'public' ][ name ] = 'foo';
+
+    try
+    {
+        // attempt to override property with getter/setter (should fail)
+        build( { 'public': true }, null, true );
+    }
+    catch ( e )
+    {
+        assert.ok( e.message.search( name ) !== -1,
+            "Property override error message should contain getter/setter name"
+        );
+        return;
+    }
+
+    assert.fail( type + " should not be able to override properties" );
+} );
+