Support symbols in ES5/3 ArrayLike. (issue 292560043 by kpreid@google.com)

7 views
Skip to first unread message

re...@codereview-hr.appspotmail.com

unread,
Apr 28, 2016, 7:08:40 PM4/28/16
to eri...@gmail.com, kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: MarkM,

Description:
This is almost identical to the corresponding change in SES
makeArrayLike, commit 1d8af47ead4a805b7805ffe8bd1eb4a0c9fad5f8; the
difference is that the existing isNumericName function is modified.
(The existing behavior is unchanged when the precondition is respected.)

Please review this at https://codereview.appspot.com/292560043/

Affected files (+17, -8 lines):
M src/com/google/caja/es53.js


Index: src/com/google/caja/es53.js
diff --git a/src/com/google/caja/es53.js b/src/com/google/caja/es53.js
index
99ce23a07ffa5544e272421e2a248b81036b9465..557e2646eb08d1182d308a76c60f2497b0bd1838
100644
--- a/src/com/google/caja/es53.js
+++ b/src/com/google/caja/es53.js
@@ -1034,10 +1034,11 @@ var ___, cajaVM, safeJSON, WeakMap, ArrayLike,
Proxy;
* Checks if {@code n} is governed by the {@code NUM___} property
descriptor.
*
* Preconditions:
- * {@code typeof n === 'number'} or {@code 'string'}
+ * {@code typeof n === 'number'} or {@code 'string'} or
{@code 'symbol'}
*/
function isNumericName(n) {
- return typeof n === 'number' || ('' + (+n)) === n;
+ var type = typeof n;
+ return type === 'number' || (type !== 'symbol' && ('' + (+n)) === n);
}

////////////////////////////////////////////////////////////////////////
@@ -4436,6 +4437,14 @@ var ___, cajaVM, safeJSON, WeakMap, ArrayLike, Proxy;
return obj;
});

+ function coerceProp(P) {
+ if (typeof P === 'symbol') {
+ return P;
+ } else {
+ return '' + P;
+ }
+ }
+
// These are the handler methods for the proxy.
var propDesc = function (P) {
var opd = ownPropDesc(P);
@@ -4448,7 +4457,7 @@ var ___, cajaVM, safeJSON, WeakMap, ArrayLike, Proxy;
var ownPropDesc = function (P) {
// If P is 'length' or a number, handle the lookup; otherwise
// pass it on to Object.prototype.
- P = '' + P;
+ P = coerceProp(P);
if (P === 'length') {
return {
get: lengthGetter,
@@ -4472,7 +4481,7 @@ var ___, cajaVM, safeJSON, WeakMap, ArrayLike, Proxy;
// Optional trap implemented for efficiency.
// If P is 'length' or a number, handle the lookup; otherwise
// pass it on to Object.prototype.
- P = '' + P;
+ P = coerceProp(P);
if (P === 'length') {
return lengthGetter.f___(O, []);
} else if (isNumericName(P)) {
@@ -4489,14 +4498,14 @@ var ___, cajaVM, safeJSON, WeakMap, ArrayLike,
Proxy;
var has = function (P) {
// The proxy has a length, numeric indices, and behaves
// as though it inherits from Object.prototype.
- P = '' + P;
+ P = coerceProp(P);
return (P === 'length') ||
isNumericName(P) ||
P in Object.prototype;
};
var hasOwn = function (P) {
// The proxy has a length and numeric indices.
- P = '' + P;
+ P = coerceProp(P);
return (P === 'length') ||
isNumericName(P);
};
@@ -4512,8 +4521,8 @@ var ___, cajaVM, safeJSON, WeakMap, ArrayLike, Proxy;
return ['length'];
};
var del = function (P) {
- P = '' + P;
- if ((P === 'length') || ('' + +P === P)) { return false; }
+ P = coerceProp(P);
+ if ((P === 'length') || isNumericName(P)) { return false; }
return true;
};



eri...@gmail.com

unread,
Apr 28, 2016, 7:12:17 PM4/28/16
to kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/292560043/diff/1/src/com/google/caja/es53.js
File src/com/google/caja/es53.js (right):

https://codereview.appspot.com/292560043/diff/1/src/com/google/caja/es53.js#newcode1041
src/com/google/caja/es53.js:1041: return type === 'number' || (type !==
'symbol' && ('' + (+n)) === n);
Why is (type !== 'symbol') better than the more intuitive (type ===
'string') ?

https://codereview.appspot.com/292560043/

re...@codereview-hr.appspotmail.com

unread,
Apr 28, 2016, 7:26:53 PM4/28/16
to eri...@gmail.com, kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
This is almost identical to the corresponding change in SES
makeArrayLike, commit 1d8af47ead4a805b7805ffe8bd1eb4a0c9fad5f8; the
difference is that the existing isNumericName function is modified.
(The existing behavior is unchanged when the precondition is respected.)

https://codereview.appspot.com/292560043/

kpr...@google.com

unread,
Apr 28, 2016, 7:27:31 PM4/28/16
to eri...@gmail.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/292560043/diff/1/src/com/google/caja/es53.js
File src/com/google/caja/es53.js (right):

https://codereview.appspot.com/292560043/diff/1/src/com/google/caja/es53.js#newcode1041
src/com/google/caja/es53.js:1041: return type === 'number' || (type !==
'symbol' && ('' + (+n)) === n);
On 2016/04/28 23:12:17, MarkM wrote:
> Why is (type !== 'symbol') better than the more intuitive (type ===
'string') ?

It isn't. Changed.

https://codereview.appspot.com/292560043/

eri...@gmail.com

unread,
Apr 28, 2016, 7:28:50 PM4/28/16
to kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reply all
Reply to author
Forward
0 new messages