Followup fix for gaping hole in patch for bug 591846 (credit to jorendorff, r=me).
authorBrendan Eich <brendan@mozilla.org>
Sat, 29 Jan 2011 01:01:54 -0800
changeset 61689 824f69dad2bc82141bd6a7572cb5829306bcd424
parent 61688 5dbc4a422c1b0b2247ab0a13c997990d65a66e27
child 61690 378cfa4ae4f53ad02167900355664110b9769213
push idunknown
push userunknown
push dateunknown
reviewersme
bugs591846
milestone2.0b11pre
Followup fix for gaping hole in patch for bug 591846 (credit to jorendorff, r=me).
js/src/jsobj.cpp
js/src/tests/js1_8_5/regress/regress-591846.js
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -2137,18 +2137,40 @@ DefinePropertyOnObject(JSContext *cx, JS
             if (desc.isDataDescriptor()) {
                 if (!shape->isDataDescriptor())
                     break;
 
                 JSBool same;
                 if (desc.hasValue) {
                     if (!SameValue(cx, desc.value, v, &same))
                         return JS_FALSE;
-                    if (!same)
+                    if (!same) {
+                        /*
+                         * Insist that a non-configurable js::PropertyOp data
+                         * property is frozen at exactly the last-got value.
+                         *
+                         * Duplicate the first part of the big conjunction that
+                         * we tested above, rather than add a local bool flag.
+                         * Likewise, don't try to keep shape->writable() in a
+                         * flag we veto from true to false for non-configurable
+                         * PropertyOp-based data properties and test before the
+                         * SameValue check later on in order to re-use that "if
+                         * (!SameValue) Reject" logic.
+                         *
+                         * This function is large and complex enough that it
+                         * seems best to repeat a small bit of code and return
+                         * Reject(...) ASAP, instead of being clever.
+                         */
+                        if (!shape->configurable() &&
+                            (!shape->hasDefaultGetter() || !shape->hasDefaultSetter()))
+                        {
+                            return Reject(cx, JSMSG_CANT_REDEFINE_PROP, throwError, desc.id, rval);
+                        }
                         break;
+                    }
                 }
                 if (desc.hasWritable && desc.writable() != shape->writable())
                     break;
             } else {
                 /* The only fields in desc will be handled below. */
                 JS_ASSERT(desc.isGenericDescriptor());
             }
         }
--- a/js/src/tests/js1_8_5/regress/regress-591846.js
+++ b/js/src/tests/js1_8_5/regress/regress-591846.js
@@ -10,23 +10,37 @@ function check(obj, name, value, readonl
 
     assertEq(pd.configurable, false, "non-configurable " + name);
     assertEq(pd.writable, !readonly, "writable " + name);
 
     try {
         // Do not allow a transition from js::PropertyOp to writable js::Value
         // data property.
         Object.defineProperty(obj, name, {writable: true});
+        assertEq(0, 1);
+    } catch (e) {
+        assertEq('' + e, "TypeError: can't redefine non-configurable property '" + name + "'");
+    }
 
-        if (!readonly) {
+    if (!readonly) {
+        try {
             // Do not allow the property denoted by name to become a true data
             // property via a descriptor that preserves the native property's
             // writable attribute.
             Object.defineProperty(obj, name, {value: value});
+            assertEq(0, 1);
+        } catch (e) {
+            assertEq('' + e, "TypeError: can't redefine non-configurable property '" + name + "'");
         }
+    }
+
+    try {
+        // Do not allow the property to be frozen at some bogus value.
+        Object.defineProperty(obj, name, {value: "bogus", writable: false});
+        assertEq(0, 1);
     } catch (e) {
         assertEq('' + e, "TypeError: can't redefine non-configurable property '" + name + "'");
     }
 
     // Now make name non-writable.
     Object.defineProperty(obj, name, {writable: false})
 
     // Assert that the right getter result "stuck".