Skip to content

Commit

Permalink
Fix detection of indexed properties in Object.defineProperty()
Browse files Browse the repository at this point in the history
When defining an indexed property on an Array object, the object's
length property should (perhaps) be updated.  This was done for any
property for which

  ToUInt32(name) == ToNumber(name)

was true, meaning any property name that, when converted to a number,
was an integer in the range [0, 2^32).  The detection should be more
strict; an indexed property is one for which

  ToString(ToUInt32(name)) == name

is true only.

Review URL: https://codereview.chromium.org/13914003

Patch from Jens Lindström <jl@opera.com>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14242 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
  • Loading branch information
mstarzinger@chromium.org committed Apr 12, 2013
1 parent 66f5c75 commit 75c388e
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/v8natives.js
Expand Up @@ -932,7 +932,7 @@ function DefineArrayProperty(obj, p, desc, should_throw) {

// Step 4 - Special handling for array index.
var index = ToUint32(p);
if (index == ToNumber(p) && index != 4294967295) {
if (ToString(index) == p && index != 4294967295) {
var length = obj.length;
var length_desc = GetOwnProperty(obj, "length");
if ((index >= length && !length_desc.isWritable()) ||
Expand Down
5 changes: 5 additions & 0 deletions test/mjsunit/object-define-property.js
Expand Up @@ -918,6 +918,11 @@ assertFalse(desc.writable);
assertFalse(desc.enumerable);
assertFalse(desc.configurable);

// Define non-array property, check that .length is unaffected.
assertEquals(16, arr.length);
Object.defineProperty(arr, '0x20', descElement);
assertEquals(16, arr.length);

// See issue 968: http://code.google.com/p/v8/issues/detail?id=968
var o = { x : 42 };
Object.defineProperty(o, "x", { writable: false });
Expand Down

0 comments on commit 75c388e

Please sign in to comment.