Make SES makeArrayLike tolerate Symbols. (issue 294430043 by kpreid@google.com)

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

Reviewers: MarkM,

Description:
This is needed because Firefox currently has both Symbols and
Proxy.create, and ('' + P) throws when P is a Symbol.

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

Affected files (+50, -13 lines):
M src/com/google/caja/ses/startSES.js
M tests/com/google/caja/plugin/test-language-guest.html


Index: src/com/google/caja/ses/startSES.js
diff --git a/src/com/google/caja/ses/startSES.js
b/src/com/google/caja/ses/startSES.js
index
12a4d80522897480e93a81d0bc7f22fcd65a53d7..858029103cd356067580bc189c6177715eb5ce1b
100644
--- a/src/com/google/caja/ses/startSES.js
+++ b/src/com/google/caja/ses/startSES.js
@@ -1303,6 +1303,17 @@ ses.startSES = function(global,
})();
if (nativeProxies) {
(function () {
+ function coerceProp(P) {
+ if (typeof P === 'symbol') {
+ return P;
+ } else {
+ return '' + P;
+ }
+ }
+ function isNumericString(P) {
+ return typeof P === 'string' && P == '' + (+P);
+ }
+
function ArrayLike(proto, getItem, getLength) {
if (typeof proto !== 'object') {
throw new TypeError('Expected proto to be an object.');
@@ -1317,14 +1328,14 @@ ses.startSES = function(global,
}

function ownPropDesc(P) {
- P = '' + P;
+ P = coerceProp(P);
if (P === 'length') {
return {
get: lengthGetter,
enumerable: false,
configurable: true // required proxy invariant
};
- } else if (typeof P === 'number' || P === '' + (+P)) {
+ } else if (isNumericString(P)) {
return {
get: constFunc(function() {
var getter = itemMap.get(this);
@@ -1337,6 +1348,7 @@ ses.startSES = function(global,
return void 0;
}
function propDesc(P) {
+ P = coerceProp(P);
var opd = ownPropDesc(P);
if (opd) {
return opd;
@@ -1345,10 +1357,10 @@ ses.startSES = function(global,
}
}
function get(O, P) {
- P = '' + P;
+ P = coerceProp(P);
if (P === 'length') {
return lengthGetter.call(O);
- } else if (typeof P === 'number' || P === '' + (+P)) {
+ } else if (isNumericString(P)) {
var getter = itemMap.get(O);
return getter ? getter(+P) : void 0;
} else {
@@ -1358,17 +1370,14 @@ ses.startSES = function(global,
}
}
function has(P) {
- P = '' + P;
+ P = coerceProp(P);
return (P === 'length') ||
- (typeof P === 'number') ||
- (P === '' + +P) ||
+ isNumericString(P) ||
(P in Object.prototype);
}
function hasOwn(P) {
- P = '' + P;
- return (P === 'length') ||
- (typeof P === 'number') ||
- (P === '' + +P);
+ P = coerceProp(P);
+ return P === 'length' || isNumericString(P);
}
function getPN() {
var result = getOwnPN ();
@@ -1383,8 +1392,12 @@ ses.startSES = function(global,
return ['length'];
};
function del(P) {
- P = '' + P;
- if ((P === 'length') || ('' + +P === P)) { return false; }
+ P = coerceProp(P);
+ if (P === 'length' || isNumericString(P)) {
+ // Non-deletable property.
+ return false;
+ }
+ // Nonexistent property.
return true;
}

Index: tests/com/google/caja/plugin/test-language-guest.html
diff --git a/tests/com/google/caja/plugin/test-language-guest.html
b/tests/com/google/caja/plugin/test-language-guest.html
index
828ac4e538216018f9fbe502b7014e47e61c3162..891a8001b4129554f6db9bee4b3fd4787c556942
100644
--- a/tests/com/google/caja/plugin/test-language-guest.html
+++ b/tests/com/google/caja/plugin/test-language-guest.html
@@ -214,6 +214,30 @@
});
</script>

+<p id="testMakeArrayLikeWithSymbol" class="testcontainer">
+ testMakeArrayLikeWithSymbol
+</p>
+<script type="text/javascript">
+ jsunitRegister('testMakeArrayLikeWithSymbol',
+ function testMakeArrayLikeWithSymbol() {
+ // Symbol not yet whitelisted.
+ var Symbol = directAccess.evalInGuestFrame('Symbol');
+
+ var ArrayLike = cajaVM.makeArrayLike(1);
+ var instance = ArrayLike(Object.create(ArrayLike.prototype),
+ function getLength() { return 0; },
+ function getItem(i) { return i; });
+
+ assertEquals(undefined, instance[Symbol.iterator]);
+ assertEquals(false, Symbol.iterator in instance);
+ delete instance[Symbol.iterator];
+ assertEquals(undefined,
+ Object.getOwnPropertyDescriptor(instance, Symbol.iterator));
+
+ pass();
+ });
+</script>
+
<p id="testRegExec" class="testcontainer">
Test regexp.exec return value
</p>

[eri...@gmail.com]

LGTM

(Note that my understanding of the scanning/testing framework is
shallow.)

https://codereview.appspot.com/294430043/