Skip to content

Commit d509e7e

Browse files
bobzhangbobzhang
authored
Merge pull request #5358 from therecount/claire/fix-object-equality
Use Object.prototype.hasOwnProperty.call(a, b) instead of a.hasOwnProperty(b)
2 parents 718b9d5 + ca1e589 commit d509e7e

File tree

5 files changed

+56
-35
lines changed

5 files changed

+56
-35
lines changed

‎jscomp/runtime/caml_obj.ml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,16 @@ module O = struct
3737
[%raw{|function(o,foo){
3838
for (var x in o) { foo(x) }}
3939
|}]
40+
41+
(**
42+
JS objects are not guaranteed to have `Object` in their prototype
43+
chain so calling `some_obj.hasOwnProperty(key)` can sometimes throw
44+
an exception when dealing with JS interop. This mainly occurs when
45+
objects are created via `Object.create(null)`. The only safe way
46+
to call this function is directly, e.g. `Object.prototype.hasOwnProperty.call(some_obj, key)`.
47+
*)
4048
external hasOwnProperty :
41-
t -> key -> bool = "hasOwnProperty" [@@bs.send]
49+
t -> key -> bool = "call" [@@bs.scope ("Object", "prototype", "hasOwnProperty")] [@@bs.val]
4250
external get_value : Obj.t -> key -> Obj.t = ""[@@bs.get_index]
4351

4452
end

‎jscomp/test/caml_compare_test.js

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -987,77 +987,89 @@ var suites = {
987987
],
988988
tl: {
989989
hd: [
990-
"File \"caml_compare_test.ml\", line 87, characters 4-11",
990+
"eq_no_prototype",
991991
(function (param) {
992992
return {
993993
TAG: /* Eq */0,
994-
_0: Caml_obj.compare(null, {
995-
hd: 3,
996-
tl: /* [] */0
997-
}),
998-
_1: -1
994+
_0: Caml_obj.equal({x:1}, ((function(){let o = Object.create(null);o.x = 1;return o;})())),
995+
_1: true
999996
};
1000997
})
1001998
],
1002999
tl: {
10031000
hd: [
1004-
"File \"caml_compare_test.ml\", line 90, characters 4-11",
1001+
"File \"caml_compare_test.ml\", line 88, characters 4-11",
10051002
(function (param) {
10061003
return {
10071004
TAG: /* Eq */0,
1008-
_0: Caml_obj.compare({
1005+
_0: Caml_obj.compare(null, {
10091006
hd: 3,
10101007
tl: /* [] */0
1011-
}, null),
1012-
_1: 1
1008+
}),
1009+
_1: -1
10131010
};
10141011
})
10151012
],
10161013
tl: {
10171014
hd: [
1018-
"File \"caml_compare_test.ml\", line 93, characters 4-11",
1015+
"File \"caml_compare_test.ml\", line 91, characters 4-11",
10191016
(function (param) {
10201017
return {
10211018
TAG: /* Eq */0,
1022-
_0: Caml_obj.compare(null, 0),
1023-
_1: -1
1019+
_0: Caml_obj.compare({
1020+
hd: 3,
1021+
tl: /* [] */0
1022+
}, null),
1023+
_1: 1
10241024
};
10251025
})
10261026
],
10271027
tl: {
10281028
hd: [
1029-
"File \"caml_compare_test.ml\", line 96, characters 4-11",
1029+
"File \"caml_compare_test.ml\", line 94, characters 4-11",
10301030
(function (param) {
10311031
return {
10321032
TAG: /* Eq */0,
1033-
_0: Caml_obj.compare(0, null),
1034-
_1: 1
1033+
_0: Caml_obj.compare(null, 0),
1034+
_1: -1
10351035
};
10361036
})
10371037
],
10381038
tl: {
10391039
hd: [
1040-
"File \"caml_compare_test.ml\", line 99, characters 4-11",
1040+
"File \"caml_compare_test.ml\", line 97, characters 4-11",
10411041
(function (param) {
10421042
return {
10431043
TAG: /* Eq */0,
1044-
_0: Caml_obj.compare(undefined, 0),
1045-
_1: -1
1044+
_0: Caml_obj.compare(0, null),
1045+
_1: 1
10461046
};
10471047
})
10481048
],
10491049
tl: {
10501050
hd: [
1051-
"File \"caml_compare_test.ml\", line 102, characters 4-11",
1051+
"File \"caml_compare_test.ml\", line 100, characters 4-11",
10521052
(function (param) {
10531053
return {
10541054
TAG: /* Eq */0,
1055-
_0: Caml_obj.compare(0, undefined),
1056-
_1: 1
1055+
_0: Caml_obj.compare(undefined, 0),
1056+
_1: -1
10571057
};
10581058
})
10591059
],
1060-
tl: /* [] */0
1060+
tl: {
1061+
hd: [
1062+
"File \"caml_compare_test.ml\", line 103, characters 4-11",
1063+
(function (param) {
1064+
return {
1065+
TAG: /* Eq */0,
1066+
_0: Caml_obj.compare(0, undefined),
1067+
_1: 1
1068+
};
1069+
})
1070+
],
1071+
tl: /* [] */0
1072+
}
10611073
}
10621074
}
10631075
}
@@ -1119,21 +1131,21 @@ function eq(loc, x, y) {
11191131
Mt.eq_suites(test_id, suites, loc, x, y);
11201132
}
11211133

1122-
eq("File \"caml_compare_test.ml\", line 112, characters 6-13", true, Caml_obj.greaterthan(1, undefined));
1134+
eq("File \"caml_compare_test.ml\", line 113, characters 6-13", true, Caml_obj.greaterthan(1, undefined));
11231135

1124-
eq("File \"caml_compare_test.ml\", line 113, characters 6-13", true, Caml_obj.lessthan(/* [] */0, {
1136+
eq("File \"caml_compare_test.ml\", line 114, characters 6-13", true, Caml_obj.lessthan(/* [] */0, {
11251137
hd: 1,
11261138
tl: /* [] */0
11271139
}));
11281140

1129-
eq("File \"caml_compare_test.ml\", line 114, characters 6-13", false, Caml_obj.greaterthan(undefined, 1));
1141+
eq("File \"caml_compare_test.ml\", line 115, characters 6-13", false, Caml_obj.greaterthan(undefined, 1));
11301142

1131-
eq("File \"caml_compare_test.ml\", line 115, characters 6-13", false, Caml_obj.greaterthan(undefined, [
1143+
eq("File \"caml_compare_test.ml\", line 116, characters 6-13", false, Caml_obj.greaterthan(undefined, [
11321144
1,
11331145
30
11341146
]));
11351147

1136-
eq("File \"caml_compare_test.ml\", line 116, characters 6-13", false, Caml_obj.lessthan([
1148+
eq("File \"caml_compare_test.ml\", line 117, characters 6-13", false, Caml_obj.lessthan([
11371149
1,
11381150
30
11391151
], undefined));

‎jscomp/test/caml_compare_test.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ let suites = ref Mt.[
8383
"eq_in_list2", (fun _ -> Eq ([[%bs.obj {x=2}]] = [[%bs.obj {x=2}]], true));
8484
"eq_with_list", (fun _ -> Eq ([%bs.obj {x=[0]}] = [%bs.obj {x=[0]}], true));
8585
"eq_with_list2", (fun _ -> Eq ([%bs.obj {x=[0]}] = [%bs.obj {x=[1]}], false));
86+
"eq_no_prototype", (fun _ -> Eq ([%bs.raw "{x:1}"] = [%bs.raw "(function(){let o = Object.create(null);o.x = 1;return o;})()"], true));
8687

8788
__LOC__ , begin fun _ ->
8889
Eq(compare Js.null (Js.Null.return [3]), -1)

‎lib/es6/caml_obj.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ function aux_obj_compare(a, b) {
209209
var do_key = function (param, key) {
210210
var min_key = param[2];
211211
var b = param[1];
212-
if (!(!b.hasOwnProperty(key) || compare(param[0][key], b[key]) > 0)) {
212+
if (!(!Object.prototype.hasOwnProperty.call(b, key) || compare(param[0][key], b[key]) > 0)) {
213213
return ;
214214
}
215215
var mk = min_key.contents;
@@ -310,14 +310,14 @@ function equal(a, b) {
310310
contents: true
311311
};
312312
var do_key_a = function (key) {
313-
if (!b.hasOwnProperty(key)) {
313+
if (!Object.prototype.hasOwnProperty.call(b, key)) {
314314
result.contents = false;
315315
return ;
316316
}
317317

318318
};
319319
var do_key_b = function (key) {
320-
if (!a.hasOwnProperty(key) || !equal(b[key], a[key])) {
320+
if (!Object.prototype.hasOwnProperty.call(a, key) || !equal(b[key], a[key])) {
321321
result.contents = false;
322322
return ;
323323
}

‎lib/js/caml_obj.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ function aux_obj_compare(a, b) {
209209
var do_key = function (param, key) {
210210
var min_key = param[2];
211211
var b = param[1];
212-
if (!(!b.hasOwnProperty(key) || compare(param[0][key], b[key]) > 0)) {
212+
if (!(!Object.prototype.hasOwnProperty.call(b, key) || compare(param[0][key], b[key]) > 0)) {
213213
return ;
214214
}
215215
var mk = min_key.contents;
@@ -310,14 +310,14 @@ function equal(a, b) {
310310
contents: true
311311
};
312312
var do_key_a = function (key) {
313-
if (!b.hasOwnProperty(key)) {
313+
if (!Object.prototype.hasOwnProperty.call(b, key)) {
314314
result.contents = false;
315315
return ;
316316
}
317317

318318
};
319319
var do_key_b = function (key) {
320-
if (!a.hasOwnProperty(key) || !equal(b[key], a[key])) {
320+
if (!Object.prototype.hasOwnProperty.call(a, key) || !equal(b[key], a[key])) {
321321
result.contents = false;
322322
return ;
323323
}

0 commit comments

Comments
 (0)